cc: Throttle swaps in Scheduler instead of OutputSurface

cc/output/output_surface.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/output/output_surface.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 28 matching lines @@
 const size_t kGpuLatencyHistorySize = 60;
 const double kGpuLatencyEstimationPercentile = 100.0;
 
 }
 
 namespace cc {
 
 OutputSurface::OutputSurface(scoped_refptr<ContextProvider> context_provider)
     : context_provider_(context_provider),
       device_scale_factor_(-1),
-      max_frames_pending_(0),
-      pending_swap_buffers_(0),
+      begin_frame_interval_(BeginFrameArgs::DefaultInterval()),
+      throttle_frame_production_(true),
       needs_begin_frame_(false),
       client_ready_for_begin_frame_(true),
       client_(NULL),
+      is_lost_(false),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false),
       weak_ptr_factory_(this),
       gpu_latency_history_(kGpuLatencyHistorySize) {}
 
 OutputSurface::OutputSurface(scoped_ptr<SoftwareOutputDevice> software_device)
     : software_device_(software_device.Pass()),
       device_scale_factor_(-1),
-      max_frames_pending_(0),
-      pending_swap_buffers_(0),
+      begin_frame_interval_(BeginFrameArgs::DefaultInterval()),
+      throttle_frame_production_(true),
       needs_begin_frame_(false),
       client_ready_for_begin_frame_(true),
       client_(NULL),
+      is_lost_(false),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false),
       weak_ptr_factory_(this),
       gpu_latency_history_(kGpuLatencyHistorySize) {}
 
 OutputSurface::OutputSurface(scoped_refptr<ContextProvider> context_provider,
                              scoped_ptr<SoftwareOutputDevice> software_device)
     : context_provider_(context_provider),
       software_device_(software_device.Pass()),
       device_scale_factor_(-1),
-      max_frames_pending_(0),
-      pending_swap_buffers_(0),
+      begin_frame_interval_(BeginFrameArgs::DefaultInterval()),
+      throttle_frame_production_(true),
       needs_begin_frame_(false),
       client_ready_for_begin_frame_(true),
       client_(NULL),
+      is_lost_(false),
       check_for_retroactive_begin_frame_pending_(false),
       external_stencil_test_enabled_(false),
       weak_ptr_factory_(this),
       gpu_latency_history_(kGpuLatencyHistorySize) {}
 
+void OutputSurface::SetThrottleFrameProduction(bool enable) {
+  DCHECK(!frame_rate_controller_);
+  throttle_frame_production_ = enable;
+}
+
 void OutputSurface::InitializeBeginFrameEmulation(
     base::SingleThreadTaskRunner* task_runner,
-    bool throttle_frame_production,
     base::TimeDelta interval) {
-  if (throttle_frame_production) {
-    scoped_refptr<DelayBasedTimeSource> time_source;
-    if (gfx::FrameTime::TimestampsAreHighRes())
-      time_source = DelayBasedTimeSourceHighRes::Create(interval, task_runner);
-    else
-      time_source = DelayBasedTimeSource::Create(interval, task_runner);
-    frame_rate_controller_.reset(new FrameRateController(time_source));
-  } else {
-    frame_rate_controller_.reset(new FrameRateController(task_runner));
-  }
+  DCHECK(throttle_frame_production_);
+  scoped_refptr<DelayBasedTimeSource> time_source;
+  if (gfx::FrameTime::TimestampsAreHighRes())
+    time_source = DelayBasedTimeSourceHighRes::Create(interval, task_runner);
+  else
+    time_source = DelayBasedTimeSource::Create(interval, task_runner);
+  frame_rate_controller_.reset(new FrameRateController(time_source));
 
   frame_rate_controller_->SetClient(this);
-  frame_rate_controller_->SetMaxSwapsPending(max_frames_pending_);
   frame_rate_controller_->SetDeadlineAdjustment(
       capabilities_.adjust_deadline_for_parent ?
           BeginFrameArgs::DefaultDeadlineAdjustment() : base::TimeDelta());
-
-  // The new frame rate controller will consume the swap acks of the old
-  // frame rate controller, so we set that expectation up here.
-  for (int i = 0; i < pending_swap_buffers_; i++)
-    frame_rate_controller_->DidSwapBuffers();
-}
-
-void OutputSurface::SetMaxFramesPending(int max_frames_pending) {
-  if (frame_rate_controller_)
-    frame_rate_controller_->SetMaxSwapsPending(max_frames_pending);
-  max_frames_pending_ = max_frames_pending;
 }
 
 void OutputSurface::CommitVSyncParameters(base::TimeTicks timebase,
                                           base::TimeDelta interval) {
   TRACE_EVENT2("cc",
                "OutputSurface::CommitVSyncParameters",
                "timebase",
                (timebase - base::TimeTicks()).InSecondsF(),
                "interval",
                interval.InSecondsF());
+  begin_frame_interval_ = interval;
   if (frame_rate_controller_)
     frame_rate_controller_->SetTimebaseAndInterval(timebase, interval);
 }
 
-void OutputSurface::FrameRateControllerTick(bool throttled,
-                                            const BeginFrameArgs& args) {
+void OutputSurface::FrameRateControllerTick(const BeginFrameArgs& args) {
   DCHECK(frame_rate_controller_);
-  if (throttled)
-    skipped_begin_frame_args_ = args;
-  else
-    BeginFrame(args);
+  BeginFrame(args);
 }
 
 // Forwarded to OutputSurfaceClient
 void OutputSurface::SetNeedsRedrawRect(const gfx::Rect& damage_rect) {
   TRACE_EVENT0("cc", "OutputSurface::SetNeedsRedrawRect");
   client_->SetNeedsRedrawRect(damage_rect);
 }
 
 void OutputSurface::SetNeedsBeginFrame(bool enable) {
   TRACE_EVENT1("cc", "OutputSurface::SetNeedsBeginFrame", "enable", enable);
   needs_begin_frame_ = enable;
   client_ready_for_begin_frame_ = true;
-  if (frame_rate_controller_) {
+  if (!throttle_frame_production_) {
+    if (enable) {
+      base::TimeTicks frame_time = gfx::FrameTime::Now();
+      base::TimeTicks deadline = frame_time + begin_frame_interval_;
+      skipped_begin_frame_args_ =
+          BeginFrameArgs::Create(frame_time, deadline, begin_frame_interval_);
+    }
+  } else if (frame_rate_controller_) {
     BeginFrameArgs skipped = frame_rate_controller_->SetActive(enable);
     if (skipped.IsValid())
       skipped_begin_frame_args_ = skipped;
   }
+
   if (needs_begin_frame_)
     PostCheckForRetroactiveBeginFrame();
 }
 
 void OutputSurface::BeginFrame(const BeginFrameArgs& args) {
-  TRACE_EVENT2("cc",
+  TRACE_EVENT1("cc",
                "OutputSurface::BeginFrame",
                "client_ready_for_begin_frame_",
-               client_ready_for_begin_frame_,
-               "pending_swap_buffers_",
-               pending_swap_buffers_);
-  if (!needs_begin_frame_ || !client_ready_for_begin_frame_ ||
-      (pending_swap_buffers_ >= max_frames_pending_ &&
-       max_frames_pending_ > 0)) {
+               client_ready_for_begin_frame_);
+  if (!needs_begin_frame_ || !client_ready_for_begin_frame_) {
     skipped_begin_frame_args_ = args;
   } else {
     client_ready_for_begin_frame_ = false;
     client_->BeginFrame(args);
     // args might be an alias for skipped_begin_frame_args_.
     // Do not reset it before calling BeginFrame!
     skipped_begin_frame_args_ = BeginFrameArgs();
   }
 }
 
@@ skipping 14 matching lines @@
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&OutputSurface::CheckForRetroactiveBeginFrame,
                  weak_ptr_factory_.GetWeakPtr()));
   check_for_retroactive_begin_frame_pending_ = true;
 }
 
 void OutputSurface::CheckForRetroactiveBeginFrame() {
   TRACE_EVENT0("cc", "OutputSurface::CheckForRetroactiveBeginFrame");
   check_for_retroactive_begin_frame_pending_ = false;
-  if (gfx::FrameTime::Now() < RetroactiveBeginFrameDeadline())
+  if (!throttle_frame_production_ ||
+      gfx::FrameTime::Now() < RetroactiveBeginFrameDeadline())
     BeginFrame(skipped_begin_frame_args_);
 }
 
 void OutputSurface::DidSwapBuffers() {
-  pending_swap_buffers_++;
-  TRACE_EVENT1("cc", "OutputSurface::DidSwapBuffers",
-               "pending_swap_buffers_", pending_swap_buffers_);
+  if (is_lost_)

[danakj, 2014/04/11 15:12:28]

Why do you need these is_lost_ checks here?

If needed, can you put the TRACE above them, and include the is_lost_ in the
trace event?

[brianderson, 2014/04/11 20:49:40]

The is_lost_ check should probably go into each SwapBuffers implementation
instead of the DidSwapBuffers notification, like
https://codereview.chromium.org/221833009 does.

It would be even better if we didn't swap if we lose the output surface during a
draw, which happens in unit tests. I don't think we lose the context
synchronously during a draw in normal operation though.

[danakj, 2014/04/14 18:14:50]

The single thread implementation checks for this before swapping. Should the
thread proxy do that also?

+    return;
+  TRACE_EVENT0("cc", "OutputSurface::DidSwapBuffers");
   client_->DidSwapBuffers();
-  if (frame_rate_controller_)
-    frame_rate_controller_->DidSwapBuffers();
-  PostCheckForRetroactiveBeginFrame();
 }
 
 void OutputSurface::OnSwapBuffersComplete() {
-  pending_swap_buffers_--;
-  TRACE_EVENT1("cc", "OutputSurface::OnSwapBuffersComplete",
-               "pending_swap_buffers_", pending_swap_buffers_);
-  client_->OnSwapBuffersComplete();
-  if (frame_rate_controller_)
-    frame_rate_controller_->DidSwapBuffersComplete();
-  PostCheckForRetroactiveBeginFrame();
+  if (is_lost_)

[danakj, 2014/04/14 18:14:50]

Since this is an ACK from the browser IPC it makes sense that losing the output
surface would drop future ACKs here instead of having them call back to the
client.

Maybe the client pointer should be unset entire when the OutputSurface is lost?

+    return;
+  TRACE_EVENT0("cc", "OutputSurface::OnSwapBuffersComplete");
+  client_->DidSwapBuffersComplete();
 }
 
 void OutputSurface::ReclaimResources(const CompositorFrameAck* ack) {
   client_->ReclaimResources(ack);
 }
 
 void OutputSurface::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "OutputSurface::DidLoseOutputSurface");
   client_ready_for_begin_frame_ = true;
-  pending_swap_buffers_ = 0;
   skipped_begin_frame_args_ = BeginFrameArgs();
   if (frame_rate_controller_)
     frame_rate_controller_->SetActive(false);
   pending_gpu_latency_query_ids_.clear();
   available_gpu_latency_query_ids_.clear();
   client_->DidLoseOutputSurface();
+  is_lost_ = true;
 }
 
 void OutputSurface::SetExternalStencilTest(bool enabled) {
   external_stencil_test_enabled_ = enabled;
 }
 
 void OutputSurface::SetExternalDrawConstraints(const gfx::Transform& transform,
                                                const gfx::Rect& viewport,
                                                const gfx::Rect& clip,
                                                bool valid_for_tile_management) {
@@ skipping 236 matching lines @@
   TRACE_EVENT1("cc", "OutputSurface::SetMemoryPolicy",
                "bytes_limit_when_visible", policy.bytes_limit_when_visible);
   // Just ignore the memory manager when it says to set the limit to zero
   // bytes. This will happen when the memory manager thinks that the renderer
   // is not visible (which the renderer knows better).
   if (policy.bytes_limit_when_visible)
     client_->SetMemoryPolicy(policy);
 }
 
 }  // namespace cc