diff options
author | brianderson <brianderson@chromium.org> | 2015-11-17 19:41:15 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-18 03:41:57 +0000 |
commit | 0055b7eaee4ca249cb83ac8ddc5200cbf7a12069 (patch) | |
tree | 28c8175c71eb6c6831bc2e7903679233db2ec8bd /cc | |
parent | 27b48cde7e6243c777ff7b44c9fe7d341c0af2d2 (diff) | |
download | chromium_src-0055b7eaee4ca249cb83ac8ddc5200cbf7a12069.zip chromium_src-0055b7eaee4ca249cb83ac8ddc5200cbf7a12069.tar.gz chromium_src-0055b7eaee4ca249cb83ac8ddc5200cbf7a12069.tar.bz2 |
cc: Clean up max frames/swaps pending usage.
All output surfaces used by the Renderer and UI should
only need a max swaps pending of 1, especially considering
Surfaces and CompositorTimingHistory don't support
multiple queued frames.
The output suface used by the cc::Display, however, may
have more than 1 swap pending - especially on platforms like
CrOS where the SwapAck is deferred until the buffer is actually
displayed.
This patch:
1) Changes the default max pending frames/swaps from 2 to 1.
2) Changes Blimp to have only 1 pending frame.
3) DCHECKS that all Renderers and UIs have a max swaps pending of 1.
4) Sets the default value in the constructor of
OutpuSurface::Capabilities, rather than through
an extra check for zero + init.
BUG=525756
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Review URL: https://codereview.chromium.org/1435133004
Cr-Commit-Position: refs/heads/master@{#360283}
Diffstat (limited to 'cc')
-rw-r--r-- | cc/output/output_surface.h | 6 | ||||
-rw-r--r-- | cc/scheduler/scheduler.cc | 4 | ||||
-rw-r--r-- | cc/scheduler/scheduler.h | 1 | ||||
-rw-r--r-- | cc/scheduler/scheduler_state_machine.cc | 16 | ||||
-rw-r--r-- | cc/scheduler/scheduler_state_machine.h | 3 | ||||
-rw-r--r-- | cc/scheduler/scheduler_unittest.cc | 8 | ||||
-rw-r--r-- | cc/surfaces/onscreen_display_client.cc | 10 | ||||
-rw-r--r-- | cc/surfaces/surface_display_output_surface.cc | 1 | ||||
-rw-r--r-- | cc/surfaces/surface_display_output_surface_unittest.cc | 1 | ||||
-rw-r--r-- | cc/test/fake_layer_tree_host_impl_client.h | 1 | ||||
-rw-r--r-- | cc/test/fake_output_surface.cc | 20 | ||||
-rw-r--r-- | cc/test/fake_output_surface.h | 4 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl.cc | 5 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl.h | 1 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl_unittest.cc | 1 | ||||
-rw-r--r-- | cc/trees/single_thread_proxy.cc | 5 | ||||
-rw-r--r-- | cc/trees/single_thread_proxy.h | 1 | ||||
-rw-r--r-- | cc/trees/thread_proxy.cc | 4 | ||||
-rw-r--r-- | cc/trees/thread_proxy.h | 1 |
19 files changed, 22 insertions, 71 deletions
diff --git a/cc/output/output_surface.h b/cc/output/output_surface.h index 26c2ffe..2176455 100644 --- a/cc/output/output_surface.h +++ b/cc/output/output_surface.h @@ -45,10 +45,6 @@ class OutputSurfaceClient; // surface (on the compositor thread) and go back to step 1. class CC_EXPORT OutputSurface : public base::trace_event::MemoryDumpProvider { public: - enum { - DEFAULT_MAX_FRAMES_PENDING = 2 - }; - OutputSurface(const scoped_refptr<ContextProvider>& context_provider, const scoped_refptr<ContextProvider>& worker_context_provider, scoped_ptr<SoftwareOutputDevice> software_device); @@ -67,7 +63,7 @@ class CC_EXPORT OutputSurface : public base::trace_event::MemoryDumpProvider { struct Capabilities { Capabilities() : delegated_rendering(false), - max_frames_pending(0), + max_frames_pending(1), adjust_deadline_for_parent(true), uses_default_gl_framebuffer(true), flipped_output_surface(false), diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc index 072086f..eb2c3e1 100644 --- a/cc/scheduler/scheduler.cc +++ b/cc/scheduler/scheduler.cc @@ -193,10 +193,6 @@ void Scheduler::SetNeedsPrepareTiles() { ProcessScheduledActions(); } -void Scheduler::SetMaxSwapsPending(int max) { - state_machine_.SetMaxSwapsPending(max); -} - void Scheduler::DidSwapBuffers() { state_machine_.DidSwapBuffers(); diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h index 5773bc4..6f8ba1e 100644 --- a/cc/scheduler/scheduler.h +++ b/cc/scheduler/scheduler.h @@ -95,7 +95,6 @@ class CC_EXPORT Scheduler : public BeginFrameObserverBase { void SetNeedsPrepareTiles(); - void SetMaxSwapsPending(int max); void DidSwapBuffers(); void DidSwapBuffersComplete(); diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc index 01ac8cb..d770689 100644 --- a/cc/scheduler/scheduler_state_machine.cc +++ b/cc/scheduler/scheduler_state_machine.cc @@ -13,6 +13,11 @@ namespace cc { +namespace { +// Surfaces and CompositorTimingHistory don't support more than 1 pending swap. +const int kMaxPendingSwaps = 1; +} // namespace + SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings) : settings_(settings), output_surface_state_(OUTPUT_SURFACE_NONE), @@ -32,7 +37,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings) invalidate_output_surface_funnel_(false), prepare_tiles_funnel_(0), consecutive_checkerboard_animations_(0), - max_pending_swaps_(1), pending_swaps_(0), swaps_with_current_output_surface_(0), needs_redraw_(false), @@ -217,7 +221,6 @@ void SchedulerStateMachine::AsValueInto( invalidate_output_surface_funnel_); state->SetInteger("consecutive_checkerboard_animations", consecutive_checkerboard_animations_); - state->SetInteger("max_pending_swaps_", max_pending_swaps_); state->SetInteger("pending_swaps_", pending_swaps_); state->SetInteger("swaps_with_current_output_surface", swaps_with_current_output_surface_); @@ -912,7 +915,7 @@ bool SchedulerStateMachine::main_thread_missed_last_deadline() const { } bool SchedulerStateMachine::SwapThrottled() const { - return pending_swaps_ >= max_pending_swaps_; + return pending_swaps_ >= kMaxPendingSwaps; } void SchedulerStateMachine::SetVisible(bool visible) { @@ -957,17 +960,12 @@ void SchedulerStateMachine::SetNeedsPrepareTiles() { needs_prepare_tiles_ = true; } } - -void SchedulerStateMachine::SetMaxSwapsPending(int max) { - max_pending_swaps_ = max; -} - void SchedulerStateMachine::DidSwapBuffers() { TRACE_EVENT_ASYNC_BEGIN0("cc", "Scheduler:pending_swaps", this); pending_swaps_++; swaps_with_current_output_surface_++; - DCHECK_LE(pending_swaps_, max_pending_swaps_); + DCHECK_LE(pending_swaps_, kMaxPendingSwaps); did_perform_swap_in_last_draw_ = true; last_frame_number_swap_performed_ = current_frame_number_; diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h index 17fdf98..3c32dc6 100644 --- a/cc/scheduler/scheduler_state_machine.h +++ b/cc/scheduler/scheduler_state_machine.h @@ -174,9 +174,6 @@ class CC_EXPORT SchedulerStateMachine { // PrepareTiles will occur shortly (even if no redraw is required). void SetNeedsPrepareTiles(); - // Sets how many swaps can be pending to the OutputSurface. - void SetMaxSwapsPending(int max); - // If the scheduler attempted to draw and swap, this provides feedback // regarding whether or not the swap actually occured. We might skip the // swap when there is not damage, for example. diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc index 6bdf5dd..362946f 100644 --- a/cc/scheduler/scheduler_unittest.cc +++ b/cc/scheduler/scheduler_unittest.cc @@ -1471,7 +1471,6 @@ TEST_F(SchedulerTest, MainFrameNotSkippedAfterLateCommit_DrawEstimateTooLong) { void SchedulerTest::ImplFrameSkippedAfterLateSwapAck( bool swap_ack_before_deadline) { // To get into a high latency state, this test disables automatic swap acks. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Draw and swap for first BeginFrame @@ -1595,7 +1594,6 @@ TEST_F(SchedulerTest, SetUpScheduler(true); // To get into a high latency state, this test disables automatic swap acks. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Even if every estimate related to the main thread is slow, we should @@ -1659,7 +1657,6 @@ TEST_F(SchedulerTest, void SchedulerTest::ImplFrameIsNotSkippedAfterLateSwapAck() { // To get into a high latency state, this test disables automatic swap acks. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Draw and swap for first BeginFrame @@ -1770,7 +1767,6 @@ TEST_F(SchedulerTest, fake_compositor_timing_history_->SetAllEstimatesTo(slow_duration); // To get into a high latency state, this test disables automatic swap acks. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Impl thread hits deadline before commit finishes to make @@ -1899,7 +1895,6 @@ TEST_F( // Disables automatic swap acks so this test can force swap ack throttling // to simulate a blocked Browser ui thread. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Get a new active tree in main-thread high latency mode and put us @@ -1970,7 +1965,6 @@ TEST_F(SchedulerTest, // Disables automatic swap acks so this test can force swap ack throttling // to simulate a blocked Browser ui thread. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Start a new commit in main-thread high latency mode and hold off on @@ -2051,7 +2045,6 @@ TEST_F( // Disables automatic swap acks so this test can force swap ack throttling // to simulate a blocked Browser ui thread. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // Start a new commit in main-thread high latency mode and hold off on @@ -2447,7 +2440,6 @@ void SchedulerTest::BeginFramesNotFromClient_SwapThrottled( fake_compositor_timing_history_->SetDrawDurationEstimate(base::TimeDelta()); // To test swap ack throttling, this test disables automatic swap acks. - scheduler_->SetMaxSwapsPending(1); client_->SetAutomaticSwapAck(false); // SetNeedsBeginMainFrame should begin the frame on the next BeginImplFrame. diff --git a/cc/surfaces/onscreen_display_client.cc b/cc/surfaces/onscreen_display_client.cc index 067f443..06828b0 100644 --- a/cc/surfaces/onscreen_display_client.cc +++ b/cc/surfaces/onscreen_display_client.cc @@ -35,10 +35,7 @@ OnscreenDisplayClient::~OnscreenDisplayClient() { } bool OnscreenDisplayClient::Initialize() { - int max_frames_pending = - output_surface_ ? output_surface_->capabilities().max_frames_pending : 0; - if (max_frames_pending <= 0) - max_frames_pending = OutputSurface::DEFAULT_MAX_FRAMES_PENDING; + DCHECK(output_surface_); BeginFrameSource* frame_source; if (disable_display_vsync_) { @@ -51,8 +48,9 @@ bool OnscreenDisplayClient::Initialize() { frame_source = synthetic_frame_source_.get(); } - scheduler_.reset(new DisplayScheduler( - display_.get(), frame_source, task_runner_.get(), max_frames_pending)); + scheduler_.reset( + new DisplayScheduler(display_.get(), frame_source, task_runner_.get(), + output_surface_->capabilities().max_frames_pending)); return display_->Initialize(output_surface_.Pass(), scheduler_.get()); } diff --git a/cc/surfaces/surface_display_output_surface.cc b/cc/surfaces/surface_display_output_surface.cc index 31a8543..c887ca4 100644 --- a/cc/surfaces/surface_display_output_surface.cc +++ b/cc/surfaces/surface_display_output_surface.cc @@ -25,7 +25,6 @@ SurfaceDisplayOutputSurface::SurfaceDisplayOutputSurface( allocator_(allocator) { factory_.set_needs_sync_points(false); capabilities_.delegated_rendering = true; - capabilities_.max_frames_pending = 1; capabilities_.adjust_deadline_for_parent = true; capabilities_.can_force_reclaim_resources = true; // Display and SurfaceDisplayOutputSurface share a GL context, so sync diff --git a/cc/surfaces/surface_display_output_surface_unittest.cc b/cc/surfaces/surface_display_output_surface_unittest.cc index daabafe..2fedf5a 100644 --- a/cc/surfaces/surface_display_output_surface_unittest.cc +++ b/cc/surfaces/surface_display_output_surface_unittest.cc @@ -36,6 +36,7 @@ class FakeOnscreenDisplayClient : public OnscreenDisplayClient { // to it now for future reference. fake_output_surface_ = static_cast<FakeOutputSurface*>(output_surface_.get()); + fake_output_surface_->set_max_frames_pending(2); } FakeOutputSurface* output_surface() { return fake_output_surface_; } diff --git a/cc/test/fake_layer_tree_host_impl_client.h b/cc/test/fake_layer_tree_host_impl_client.h index d05c11e..961f0f2 100644 --- a/cc/test/fake_layer_tree_host_impl_client.h +++ b/cc/test/fake_layer_tree_host_impl_client.h @@ -19,7 +19,6 @@ class FakeLayerTreeHostImplClient : public LayerTreeHostImplClient { void CommitVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval) override {} void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override {} - void SetMaxSwapsPendingOnImplThread(int max) override {} void DidSwapBuffersOnImplThread() override {} void DidSwapBuffersCompleteOnImplThread() override {} void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override {} diff --git a/cc/test/fake_output_surface.cc b/cc/test/fake_output_surface.cc index e30806e..4855f30 100644 --- a/cc/test/fake_output_surface.cc +++ b/cc/test/fake_output_surface.cc @@ -25,10 +25,7 @@ FakeOutputSurface::FakeOutputSurface( suspended_for_recycle_(false), framebuffer_(0), overlay_candidate_validator_(nullptr) { - if (delegated_rendering) { - capabilities_.delegated_rendering = true; - capabilities_.max_frames_pending = 1; - } + capabilities_.delegated_rendering = delegated_rendering; } FakeOutputSurface::FakeOutputSurface( @@ -41,10 +38,7 @@ FakeOutputSurface::FakeOutputSurface( suspended_for_recycle_(false), framebuffer_(0), overlay_candidate_validator_(nullptr) { - if (delegated_rendering) { - capabilities_.delegated_rendering = true; - capabilities_.max_frames_pending = 1; - } + capabilities_.delegated_rendering = delegated_rendering; } FakeOutputSurface::FakeOutputSurface( @@ -57,10 +51,7 @@ FakeOutputSurface::FakeOutputSurface( suspended_for_recycle_(false), framebuffer_(0), overlay_candidate_validator_(nullptr) { - if (delegated_rendering) { - capabilities_.delegated_rendering = true; - capabilities_.max_frames_pending = 1; - } + capabilities_.delegated_rendering = delegated_rendering; } FakeOutputSurface::FakeOutputSurface( @@ -74,10 +65,7 @@ FakeOutputSurface::FakeOutputSurface( suspended_for_recycle_(false), framebuffer_(0), overlay_candidate_validator_(nullptr) { - if (delegated_rendering) { - capabilities_.delegated_rendering = true; - capabilities_.max_frames_pending = 1; - } + capabilities_.delegated_rendering = delegated_rendering; } FakeOutputSurface::~FakeOutputSurface() {} diff --git a/cc/test/fake_output_surface.h b/cc/test/fake_output_surface.h index 34274d8..14ec0f2 100644 --- a/cc/test/fake_output_surface.h +++ b/cc/test/fake_output_surface.h @@ -94,6 +94,10 @@ class FakeOutputSurface : public OutputSurface { return surface.Pass(); } + void set_max_frames_pending(int max) { + capabilities_.max_frames_pending = max; + } + CompositorFrame& last_sent_frame() { return last_sent_frame_; } size_t num_sent_frames() { return num_sent_frames_; } diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc index fcc98b5..a203fd8 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -2299,10 +2299,7 @@ bool LayerTreeHostImpl::InitializeRenderer(OutputSurface* output_surface) { : base::TimeDelta(); client_->SetEstimatedParentDrawTime(parent_draw_time); - int max_frames_pending = output_surface_->capabilities().max_frames_pending; - if (max_frames_pending <= 0) - max_frames_pending = OutputSurface::DEFAULT_MAX_FRAMES_PENDING; - client_->SetMaxSwapsPendingOnImplThread(max_frames_pending); + DCHECK_EQ(1, output_surface_->capabilities().max_frames_pending); client_->OnCanDrawStateChanged(CanDraw()); // There will not be anything to draw here, so set high res diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h index e8d981a..af4cb813 100644 --- a/cc/trees/layer_tree_host_impl.h +++ b/cc/trees/layer_tree_host_impl.h @@ -93,7 +93,6 @@ class LayerTreeHostImplClient { virtual void CommitVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval) = 0; virtual void SetEstimatedParentDrawTime(base::TimeDelta draw_time) = 0; - virtual void SetMaxSwapsPendingOnImplThread(int max) = 0; virtual void DidSwapBuffersOnImplThread() = 0; virtual void DidSwapBuffersCompleteOnImplThread() = 0; virtual void OnResourcelessSoftareDrawStateChanged( diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index 849b566..8c46e62 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -114,7 +114,6 @@ class LayerTreeHostImplTest : public testing::Test, void CommitVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval) override {} void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override {} - void SetMaxSwapsPendingOnImplThread(int max) override {} void DidSwapBuffersOnImplThread() override {} void DidSwapBuffersCompleteOnImplThread() override {} void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override {} diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc index 55f2c31..3bcf018 100644 --- a/cc/trees/single_thread_proxy.cc +++ b/cc/trees/single_thread_proxy.cc @@ -482,11 +482,6 @@ void SingleThreadProxy::SetEstimatedParentDrawTime(base::TimeDelta draw_time) { scheduler_on_impl_thread_->SetEstimatedParentDrawTime(draw_time); } -void SingleThreadProxy::SetMaxSwapsPendingOnImplThread(int max) { - if (scheduler_on_impl_thread_) - scheduler_on_impl_thread_->SetMaxSwapsPending(max); -} - void SingleThreadProxy::DidSwapBuffersOnImplThread() { TRACE_EVENT0("cc", "SingleThreadProxy::DidSwapBuffersOnImplThread"); if (scheduler_on_impl_thread_) diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h index 591e0c1..edc522f3 100644 --- a/cc/trees/single_thread_proxy.h +++ b/cc/trees/single_thread_proxy.h @@ -85,7 +85,6 @@ class CC_EXPORT SingleThreadProxy : public Proxy, void CommitVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval) override; void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override; - void SetMaxSwapsPendingOnImplThread(int max) override; void DidSwapBuffersOnImplThread() override; void DidSwapBuffersCompleteOnImplThread() override; void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override; diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc index 60d13688..a192840 100644 --- a/cc/trees/thread_proxy.cc +++ b/cc/trees/thread_proxy.cc @@ -308,10 +308,6 @@ void ThreadProxy::SetEstimatedParentDrawTime(base::TimeDelta draw_time) { impl().scheduler->SetEstimatedParentDrawTime(draw_time); } -void ThreadProxy::SetMaxSwapsPendingOnImplThread(int max) { - impl().scheduler->SetMaxSwapsPending(max); -} - void ThreadProxy::DidSwapBuffersOnImplThread() { impl().scheduler->DidSwapBuffers(); } diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h index 7be549e..771ada2 100644 --- a/cc/trees/thread_proxy.h +++ b/cc/trees/thread_proxy.h @@ -187,7 +187,6 @@ class CC_EXPORT ThreadProxy : public Proxy, void CommitVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval) override; void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override; - void SetMaxSwapsPendingOnImplThread(int max) override; void DidSwapBuffersOnImplThread() override; void DidSwapBuffersCompleteOnImplThread() override; void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override; |