Skip to content

Commit

Permalink
viz: Use SetFrameRate API on android if supported.
Browse files Browse the repository at this point in the history
Android R has a new NDK API which allows tagging a surface with any
frame rate. This allows better decision making in the platform based on
the frame rate for all content. So its preferable to use that instead
of making a decision in Chrome when possible.

TBR=sunnyps@chromium.org
R=dalecurtis@chromium.org

Change-Id: I1b597ca487ed320982eb034cf3870afe3c63ab9f
Bug: 1039988
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2168030
Commit-Queue: Khushal <khushalsagar@chromium.org>
Auto-Submit: Khushal <khushalsagar@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768034}
  • Loading branch information
khushalsagar authored and Commit Bot committed May 12, 2020
1 parent 38b31e9 commit a472c01
Show file tree
Hide file tree
Showing 26 changed files with 217 additions and 22 deletions.
2 changes: 1 addition & 1 deletion components/viz/common/frame_sinks/begin_frame_args.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct VIZ_COMMON_EXPORT BeginFrameArgs {
// This is the preferred interval to use when the producer can animate at the
// max interval supported by the Display.
static constexpr base::TimeDelta MinInterval() {
return base::TimeDelta::Min();
return base::TimeDelta::FromSeconds(0);
}

// This is a hard-coded deadline adjustment used by the display compositor.
Expand Down
22 changes: 21 additions & 1 deletion components/viz/service/display/display.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "components/viz/service/display/surface_aggregator.h"
#include "components/viz/service/surfaces/surface.h"
#include "components/viz/service/surfaces/surface_manager.h"
#include "gpu/command_buffer/client/context_support.h"
#include "gpu/command_buffer/client/gles2_interface.h"
#include "gpu/ipc/scheduler_sequence.h"
#include "services/viz/public/mojom/compositing/compositor_frame_sink.mojom.h"
Expand All @@ -47,6 +48,9 @@
#include "ui/gfx/presentation_feedback.h"
#include "ui/gfx/swap_result.h"

#if defined(OS_ANDROID)
#include "ui/gl/android/android_surface_control_compat.h"
#endif
namespace viz {

namespace {
Expand Down Expand Up @@ -208,6 +212,14 @@ bool ReduceComplexity(const cc::Region& region,
return true;
}

bool SupportsSetFrameRate(const OutputSurface* output_surface) {
#if defined(OS_ANDROID)
return output_surface->capabilities().supports_surfaceless &&
gl::SurfaceControl::SupportsSetFrameRate();
#endif
return false;
}

} // namespace

constexpr base::TimeDelta Display::kDrawToSwapMin;
Expand Down Expand Up @@ -330,7 +342,8 @@ void Display::Initialize(DisplayClient* client,
output_surface_->software_device()->BindToClient(this);

frame_rate_decider_ = std::make_unique<FrameRateDecider>(
surface_manager_, this, using_synthetic_bfs);
surface_manager_, this, using_synthetic_bfs,
SupportsSetFrameRate(output_surface_.get()));

InitializeRenderer(enable_shared_images);

Expand Down Expand Up @@ -1144,6 +1157,13 @@ void Display::RemoveOverdrawQuads(CompositorFrame* frame) {
}

void Display::SetPreferredFrameInterval(base::TimeDelta interval) {
if (frame_rate_decider_->supports_set_frame_rate()) {
float frame_rate =
interval.InSecondsF() == 0 ? 0 : (1 / interval.InSecondsF());
output_surface_->SetFrameRate(frame_rate);
return;
}

client_->SetPreferredFrameInterval(interval);
}

Expand Down
60 changes: 45 additions & 15 deletions components/viz/service/display/frame_rate_decider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
#include "components/viz/service/surfaces/surface_manager.h"

namespace viz {
namespace {

bool AreAlmostEqual(base::TimeDelta a, base::TimeDelta b) {
if (a.is_min() || b.is_min() || a.is_max() || b.is_max())
return a == b;

constexpr auto kMaxDelta = base::TimeDelta::FromMillisecondsD(0.5);
return (a - b).magnitude() < kMaxDelta;
}

} // namespace

FrameRateDecider::ScopedAggregate::ScopedAggregate(FrameRateDecider* decider)
: decider_(decider) {
Expand All @@ -21,11 +32,13 @@ FrameRateDecider::ScopedAggregate::~ScopedAggregate() {

FrameRateDecider::FrameRateDecider(SurfaceManager* surface_manager,
Client* client,
bool using_synthetic_bfs)
bool using_synthetic_bfs,
bool supports_set_frame_rate)
: supported_intervals_{BeginFrameArgs::DefaultInterval()},
surface_manager_(surface_manager),
client_(client),
using_synthetic_bfs_(using_synthetic_bfs) {
using_synthetic_bfs_(using_synthetic_bfs),
supports_set_frame_rate_(supports_set_frame_rate) {
surface_manager_->AddObserver(this);
}

Expand Down Expand Up @@ -147,29 +160,45 @@ void FrameRateDecider::UpdatePreferredFrameIntervalIfNeeded() {
// animating. This ensures that, for instance, if we're currently displaying
// a video while the rest of the page is static, we choose the frame interval
// optimal for the video.
base::TimeDelta min_frame_sink_interval =
frame_sinks_updated_in_previous_frame_.empty()
? BeginFrameArgs::MinInterval()
: base::TimeDelta::Max();
base::Optional<base::TimeDelta> min_frame_sink_interval;
bool all_frame_sinks_have_same_interval = true;
for (const auto& frame_sink_id : frame_sinks_updated_in_previous_frame_) {
min_frame_sink_interval = std::min(
min_frame_sink_interval,
client_->GetPreferredFrameIntervalForFrameSinkId(frame_sink_id));
auto interval =
client_->GetPreferredFrameIntervalForFrameSinkId(frame_sink_id);
if (!min_frame_sink_interval) {
min_frame_sink_interval = interval;
continue;
}

if (!AreAlmostEqual(*min_frame_sink_interval, interval))
all_frame_sinks_have_same_interval = false;
min_frame_sink_interval = std::min(*min_frame_sink_interval, interval);
}
if (!min_frame_sink_interval)
min_frame_sink_interval = BeginFrameArgs::MinInterval();

TRACE_EVENT_INSTANT1("viz",
"FrameRateDecider::UpdatePreferredFrameIntervalIfNeeded",
TRACE_EVENT_SCOPE_THREAD, "min_frame_sink_interval",
min_frame_sink_interval.InMillisecondsF());
min_frame_sink_interval->InMillisecondsF());

// If only one frame sink is being updated and its frame rate can be directly
// forwarded to the system, then prefer that over choosing one of the refresh
// rates advertised by the system.
if (all_frame_sinks_have_same_interval && supports_set_frame_rate_) {
SetPreferredInterval(*min_frame_sink_interval);
return;
}

// If we don't have an explicit preference from the active frame sinks, then
// we use a 0 value for preferred frame interval to let the framework pick the
// ideal refresh rate.
base::TimeDelta new_preferred_interval = UnspecifiedFrameInterval();
if (min_frame_sink_interval != BeginFrameArgs::MinInterval()) {
if (*min_frame_sink_interval != BeginFrameArgs::MinInterval()) {
for (auto supported_interval : supported_intervals_) {
// Pick the display interval which is closest to the preferred interval.
if ((min_frame_sink_interval - supported_interval).magnitude() <
(min_frame_sink_interval - new_preferred_interval).magnitude()) {
if ((*min_frame_sink_interval - supported_interval).magnitude() <
(*min_frame_sink_interval - new_preferred_interval).magnitude()) {
new_preferred_interval = supported_interval;
}
}
Expand All @@ -184,14 +213,15 @@ void FrameRateDecider::SetPreferredInterval(
TRACE_EVENT_SCOPE_THREAD, "new_preferred_interval",
new_preferred_interval.InMillisecondsF());

if (new_preferred_interval == last_computed_preferred_frame_interval_) {
if (AreAlmostEqual(new_preferred_interval,
last_computed_preferred_frame_interval_)) {
num_of_frames_since_preferred_interval_changed_++;
} else {
num_of_frames_since_preferred_interval_changed_ = 0u;
}
last_computed_preferred_frame_interval_ = new_preferred_interval;

if (current_preferred_frame_interval_ == new_preferred_interval)
if (AreAlmostEqual(current_preferred_frame_interval_, new_preferred_interval))
return;

// The min num of frames heuristic is to ensure we see a constant pattern
Expand Down
5 changes: 4 additions & 1 deletion components/viz/service/display/frame_rate_decider.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {

FrameRateDecider(SurfaceManager* surface_manager,
Client* client,
bool using_synthetic_bfs);
bool using_synthetic_bfs,
bool supports_set_frame_rate);
~FrameRateDecider() override;

void SetSupportedFrameIntervals(
std::vector<base::TimeDelta> supported_intervals);
bool supports_set_frame_rate() const { return supports_set_frame_rate_; }

void set_min_num_of_frames_to_toggle_interval_for_testing(size_t num) {
min_num_of_frames_to_toggle_interval_ = num;
Expand Down Expand Up @@ -95,6 +97,7 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {
SurfaceManager* const surface_manager_;
Client* const client_;
const bool using_synthetic_bfs_;
const bool supports_set_frame_rate_;
};

} // namespace viz
Expand Down
36 changes: 32 additions & 4 deletions components/viz/service/display/frame_rate_decider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class FrameRateDeciderTest : public testing::Test,

void SetUp() override {
surface_manager_ = std::make_unique<SurfaceManager>(this, base::nullopt);
frame_rate_decider_ =
std::make_unique<FrameRateDecider>(surface_manager_.get(), this, false);
frame_rate_decider_ = std::make_unique<FrameRateDecider>(
surface_manager_.get(), this, false, false);
frame_rate_decider_->set_min_num_of_frames_to_toggle_interval_for_testing(
0u);
}
Expand Down Expand Up @@ -319,8 +319,8 @@ TEST_F(FrameRateDeciderTest, TogglesAfterMinNumOfFrames) {
}

TEST_F(FrameRateDeciderTest, TogglesWithSyntheticBFS) {
frame_rate_decider_ =
std::make_unique<FrameRateDecider>(surface_manager_.get(), this, true);
frame_rate_decider_ = std::make_unique<FrameRateDecider>(
surface_manager_.get(), this, true, false);
frame_rate_decider_->set_min_num_of_frames_to_toggle_interval_for_testing(0u);
base::TimeDelta min_supported_interval = base::TimeDelta::FromSeconds(1);
const std::vector<base::TimeDelta> supported_intervals = {
Expand Down Expand Up @@ -366,5 +366,33 @@ TEST_F(FrameRateDeciderTest, TogglesWithSyntheticBFS) {
EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());
}

TEST_F(FrameRateDeciderTest, ManySinksWithMinInterval) {
base::TimeDelta min_supported_interval = base::TimeDelta::FromSeconds(1);
const std::vector<base::TimeDelta> supported_intervals = {
min_supported_interval * 3, min_supported_interval * 2,
min_supported_interval};
frame_rate_decider_->SetSupportedFrameIntervals(supported_intervals);
EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());

Surface* surfaces[3];
for (int i = 0; i < 3; ++i) {
FrameSinkId frame_sink_id(1u, i);
if (i == 0)
preferred_intervals_[frame_sink_id] = min_supported_interval;
else
preferred_intervals_[frame_sink_id] = BeginFrameArgs::MinInterval();
surfaces[i] = CreateAndDrawSurface(frame_sink_id);
}

for (int i = 0; i < 3; ++i)
UpdateFrame(surfaces[i]);
{
FrameRateDecider::ScopedAggregate scope(frame_rate_decider_.get());
for (int i = 0; i < 3; ++i)
frame_rate_decider_->OnSurfaceWillBeDrawn(surfaces[i]);
}
EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());
}

} // namespace
} // namespace viz
3 changes: 3 additions & 0 deletions components/viz/service/display/output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ class VIZ_SERVICE_EXPORT OutputSurface {
GetGpuTaskSchedulerHelper() = 0;
virtual gpu::MemoryTracker* GetMemoryTracker() = 0;

// Notifies the OutputSurface of rate of content updates in frames per second.
virtual void SetFrameRate(float frame_rate) {}

protected:
struct OutputSurface::Capabilities capabilities_;
scoped_refptr<ContextProvider> context_provider_;
Expand Down
5 changes: 5 additions & 0 deletions components/viz/service/display_embedder/gl_output_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -254,4 +254,9 @@ GLOutputSurface::GetGpuTaskSchedulerHelper() {
gpu::MemoryTracker* GLOutputSurface::GetMemoryTracker() {
return viz_context_provider_->GetMemoryTracker();
}

void GLOutputSurface::SetFrameRate(float frame_rate) {
viz_context_provider_->ContextSupport()->SetFrameRate(frame_rate);
}

} // namespace viz
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class GLOutputSurface : public OutputSurface {
scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
override;
gpu::MemoryTracker* GetMemoryTracker() override;
void SetFrameRate(float frame_rate) override;

protected:
OutputSurfaceClient* client() const { return client_; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@ gpu::MemoryTracker* SkiaOutputSurfaceImpl::GetMemoryTracker() {
return impl_on_gpu_->GetMemoryTracker();
}

void SkiaOutputSurfaceImpl::SetFrameRate(float frame_rate) {
auto task = base::BindOnce(&SkiaOutputSurfaceImplOnGpu::SetFrameRate,
base::Unretained(impl_on_gpu_.get()), frame_rate);
ScheduleGpuTask(std::move(task), {});
}

void SkiaOutputSurfaceImpl::SetCapabilitiesForTesting(
gfx::SurfaceOrigin output_surface_origin) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
scoped_refptr<gpu::GpuTaskSchedulerHelper> GetGpuTaskSchedulerHelper()
override;
gfx::Rect GetCurrentFramebufferDamage() const override;
void SetFrameRate(float frame_rate) override;

// SkiaOutputSurface implementation:
SkCanvas* BeginPaintCurrentFrame() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,11 @@ void SkiaOutputSurfaceImplOnGpu::SetGpuVSyncEnabled(bool enabled) {
output_device_->SetGpuVSyncEnabled(enabled);
}

void SkiaOutputSurfaceImplOnGpu::SetFrameRate(float frame_rate) {
if (gl_surface_)
gl_surface_->SetFrameRate(frame_rate);
}

void SkiaOutputSurfaceImplOnGpu::SetCapabilitiesForTesting(
const OutputSurface::Capabilities& capabilities) {
MakeCurrent(false /* need_fbo0 */);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate {
#endif
void SetGpuVSyncEnabled(bool enabled);

void SetFrameRate(float frame_rate);
bool was_context_lost() { return context_state_->context_lost(); }

void SetCapabilitiesForTesting(
Expand Down
1 change: 1 addition & 0 deletions components/viz/test/test_context_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class TestContextSupport : public gpu::ContextSupport {
void WillCallGLFromSkia() override;
void DidCallGLFromSkia() override;
void SetDisplayTransform(gfx::OverlayTransform transform) override {}
void SetFrameRate(float frame_rate) override {}

void CallAllSyncPointCallbacks();

Expand Down
4 changes: 4 additions & 0 deletions gpu/command_buffer/client/context_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ class ContextSupport {
// from the client.
virtual void SetDisplayTransform(gfx::OverlayTransform transform) = 0;

// Notifies the onscreen surface of the rate at which content is being
// updated.
virtual void SetFrameRate(float frame_rate) = 0;

protected:
ContextSupport() = default;
virtual ~ContextSupport() = default;
Expand Down
4 changes: 4 additions & 0 deletions gpu/command_buffer/client/gpu_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ class GPU_EXPORT GpuControl {
// from the client.
virtual void SetDisplayTransform(gfx::OverlayTransform transform) = 0;

// Notifies the surface of the ideal frame rate that the content is updated
// at. This can be used to tune the hardware refresh rate.
virtual void SetFrameRate(float frame_rate) {}

private:
DISALLOW_COPY_AND_ASSIGN(GpuControl);
};
Expand Down
5 changes: 5 additions & 0 deletions gpu/command_buffer/client/implementation_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,4 +420,9 @@ void ImplementationBase::SetDisplayTransform(gfx::OverlayTransform transform) {
gpu_control_->SetDisplayTransform(transform);
}

void ImplementationBase::SetFrameRate(float frame_rate) {
helper_->Flush();
gpu_control_->SetFrameRate(frame_rate);
}

} // namespace gpu
1 change: 1 addition & 0 deletions gpu/command_buffer/client/implementation_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class GLES2_IMPL_EXPORT ImplementationBase
void WillCallGLFromSkia() override;
void DidCallGLFromSkia() override;
void SetDisplayTransform(gfx::OverlayTransform transform) override;
void SetFrameRate(float frame_rate) override;

// base::trace_event::MemoryDumpProvider implementation.
bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ class ContextSupportStub : public ContextSupport {
void WillCallGLFromSkia() override {}
void DidCallGLFromSkia() override {}
void SetDisplayTransform(gfx::OverlayTransform transform) override {}
void SetFrameRate(float frame_rate) override {}

private:
std::unique_ptr<char[]> mapped_transfer_cache_entry_;
Expand Down
11 changes: 11 additions & 0 deletions gpu/ipc/in_process_command_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,17 @@ void InProcessCommandBuffer::SetDisplayTransformOnGpuThread(
surface_->SetDisplayTransform(transform);
}

void InProcessCommandBuffer::SetFrameRate(float frame_rate) {
ScheduleGpuTask(
base::BindOnce(&InProcessCommandBuffer::SetFrameRateOnGpuThread,
gpu_thread_weak_ptr_factory_.GetWeakPtr(), frame_rate));
}

void InProcessCommandBuffer::SetFrameRateOnGpuThread(float frame_rate) {
DCHECK_CALLED_ON_VALID_SEQUENCE(gpu_sequence_checker_);
surface_->SetFrameRate(frame_rate);
}

#if defined(OS_WIN)
void InProcessCommandBuffer::DidCreateAcceleratedSurfaceChildWindow(
SurfaceHandle parent_window,
Expand Down
Loading

0 comments on commit a472c01

Please sign in to comment.