android: Remove custom browser compositor scheduling logic

content/browser/renderer_host/compositor_impl_android.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/compositor_impl_android.h"
 
 #include <android/bitmap.h>
 #include <android/native_window_jni.h>
 
 #include "base/android/jni_android.h"
@@ skipping 124 matching lines @@
     DCHECK(command_buffer_proxy);
     return command_buffer_proxy;
   }
 
   void OnSwapBuffersCompleted(const std::vector<ui::LatencyInfo>& latency_info,
                               gfx::SwapResult result) {
     RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
     OutputSurface::OnSwapBuffersComplete();
   }
 
-  void OnUpdateVSyncParameters(base::TimeTicks timebase,
-                               base::TimeDelta interval) override {
+  void OnVSync(base::TimeTicks timebase, base::TimeDelta interval) override {
     CommitVSyncParameters(timebase, interval);
   }
 
   CompositorImpl* compositor_;
   base::Callback<void(gpu::Capabilities)> populate_gpu_capabilities_callback_;
   base::CancelableCallback<void(const std::vector<ui::LatencyInfo>&,
                                 gfx::SwapResult)>
       swap_buffers_completion_callback_;
   scoped_ptr<cc::OverlayCandidateValidator> overlay_candidate_validator_;
 };
 
+class ExternalBeginFrameSource : public cc::BeginFrameSourceBase,
+                                 public CompositorImpl::VSyncObserver {
+ public:
+  ExternalBeginFrameSource(CompositorImpl* compositor)
+      : compositor_(compositor) {
+    compositor_->AddObserver(this);
+  }
+
+  ~ExternalBeginFrameSource() {
+    compositor_->RemoveObserver(this);
+  }
+
+  // cc::BeginFrameSourceBase implementation:
+  void OnNeedsBeginFramesChange(
+      bool needs_begin_frames) override {
+    compositor_->OnNeedsBeginFramesChange(needs_begin_frames);
+  }
+
+  // CompositorImpl::VSyncObserver implementation:
+  void OnVSync(base::TimeTicks frame_time,
+               base::TimeDelta vsync_period) override {
+    if (NeedsBeginFrames()) {

[brianderson, 2015/12/12 01:41:46]

This check shouldn't be needed. The scheduler should be immune to spurious
BeginFrames, and actually wants them in case it suddenly decides it wants
BeginFrames.

[no sievers, 2015/12/17 00:52:32]

Done.

+      CallOnBeginFrame(cc::BeginFrameArgs::Create(
+          BEGINFRAME_FROM_HERE, frame_time, base::TimeTicks::Now(),
+          vsync_period, cc::BeginFrameArgs::NORMAL));
+    }
+  }
+
+ private:
+  CompositorImpl* compositor_;
+};
+
 static bool g_initialized = false;
 
 bool g_use_surface_manager = false;
 base::LazyInstance<cc::SurfaceManager> g_surface_manager =
     LAZY_INSTANCE_INITIALIZER;
 
 int g_surface_id_namespace = 0;
 
 class SingleThreadTaskGraphRunner : public cc::SingleThreadTaskGraphRunner {
  public:
@@ skipping 62 matching lines @@
     : root_layer_(cc::Layer::Create(Compositor::LayerSettings())),
       resource_manager_(root_window),
       surface_id_allocator_(GetSurfaceManager() ? CreateSurfaceIdAllocator()
                                                 : nullptr),
       has_transparent_background_(false),
       device_scale_factor_(1),
       window_(NULL),
       surface_id_(0),
       client_(client),
       root_window_(root_window),
-      did_post_swapbuffers_(false),
-      ignore_schedule_composite_(false),
-      needs_composite_(false),
       needs_animate_(false),
-      will_composite_immediately_(false),
-      composite_on_vsync_trigger_(DO_NOT_COMPOSITE),
       pending_swapbuffers_(0U),
       num_successive_context_creation_failures_(0),
       output_surface_request_pending_(false),
+      needs_begin_frames_(false),
       weak_factory_(this) {
   DCHECK(client);
   DCHECK(root_window);
   root_window->AttachCompositor(this);
   CreateLayerTreeHost();
   resource_manager_.Init(host_.get());
 }
 
 CompositorImpl::~CompositorImpl() {
   root_window_->DetachCompositor();
   // Clean-up any surface references.
   SetSurface(NULL);
 }
 
-void CompositorImpl::PostComposite(CompositingTrigger trigger) {
-  DCHECK(host_->visible());
-  DCHECK(needs_composite_);
-  DCHECK(trigger == COMPOSITE_IMMEDIATELY || trigger == COMPOSITE_EVENTUALLY);
-
-  if (will_composite_immediately_ ||
-      (trigger == COMPOSITE_EVENTUALLY && WillComposite())) {
-    // We will already composite soon enough.
-    DCHECK(WillComposite());
-    return;
-  }
-
-  if (DidCompositeThisFrame()) {
-    DCHECK(!WillCompositeThisFrame());
-    if (composite_on_vsync_trigger_ != COMPOSITE_IMMEDIATELY) {
-      composite_on_vsync_trigger_ = trigger;
-      root_window_->RequestVSyncUpdate();
-    }
-    DCHECK(WillComposite());
-    return;
-  }
-
-  base::TimeDelta delay;
-  if (trigger == COMPOSITE_IMMEDIATELY) {
-    will_composite_immediately_ = true;
-    composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
-  } else {
-    DCHECK(!WillComposite());
-    const base::TimeDelta estimated_composite_time = vsync_period_ / 4;
-    const base::TimeTicks now = base::TimeTicks::Now();
-
-    if (!last_vsync_.is_null() && (now - last_vsync_) < vsync_period_) {
-      base::TimeTicks next_composite =
-          last_vsync_ + vsync_period_ - estimated_composite_time;
-      if (next_composite < now) {
-        // It's too late, we will reschedule composite as needed on the next
-        // vsync.
-        composite_on_vsync_trigger_ = COMPOSITE_EVENTUALLY;
-        root_window_->RequestVSyncUpdate();
-        DCHECK(WillComposite());
-        return;
-      }
-
-      delay = next_composite - now;
-    }
-  }
-  TRACE_EVENT2("cc,benchmark", "CompositorImpl::PostComposite",
-               "trigger", trigger,
-               "delay", delay.InMillisecondsF());
-
-  DCHECK(composite_on_vsync_trigger_ == DO_NOT_COMPOSITE);
-  if (current_composite_task_)
-    current_composite_task_->Cancel();
-
-  // Unretained because we cancel the task on shutdown.
-  current_composite_task_.reset(new base::CancelableClosure(
-      base::Bind(&CompositorImpl::Composite, base::Unretained(this), trigger)));
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, current_composite_task_->callback(), delay);
-}
-
-void CompositorImpl::Composite(CompositingTrigger trigger) {
-  if (trigger == COMPOSITE_IMMEDIATELY)
-    will_composite_immediately_ = false;
-
-  DCHECK(host_->visible());
-  DCHECK(trigger == COMPOSITE_IMMEDIATELY || trigger == COMPOSITE_EVENTUALLY);
-  DCHECK(needs_composite_);
-  DCHECK(!DidCompositeThisFrame());
-
-  DCHECK_LE(pending_swapbuffers_, kMaxUiSwapBuffers);
-  // Swap Ack accounting is unreliable if the OutputSurface was lost.
-  // In that case still attempt to composite, which will cause creation of a
-  // new OutputSurface and reset pending_swapbuffers_.
-  if (pending_swapbuffers_ == kMaxUiSwapBuffers &&
-      !host_->output_surface_lost()) {
-    TRACE_EVENT0("compositor", "CompositorImpl_SwapLimit");
-    return;
-  }
-
-  // Reset state before Layout+Composite since that might create more
-  // requests to Composite that we need to respect.
-  needs_composite_ = false;
-
-  // Only allow compositing once per vsync.
-  current_composite_task_->Cancel();
-  DCHECK(DidCompositeThisFrame() && !WillComposite());
-
-  const base::TimeTicks frame_time = base::TimeTicks::Now();
-  if (needs_animate_) {
-    base::AutoReset<bool> auto_reset_ignore_schedule(
-        &ignore_schedule_composite_, true);
-    needs_animate_ = false;
-    root_window_->Animate(frame_time);
-  }
-
-  did_post_swapbuffers_ = false;
-  host_->Composite(frame_time);
-  if (did_post_swapbuffers_)
-    pending_swapbuffers_++;
-
-  // Need to track vsync to avoid compositing more than once per frame.
-  root_window_->RequestVSyncUpdate();
-}
-
 ui::UIResourceProvider& CompositorImpl::GetUIResourceProvider() {
   return *this;
 }
 
 ui::ResourceManager& CompositorImpl::GetResourceManager() {
   return resource_manager_;
 }
 
 void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
   if (subroot_layer_.get()) {
@@ skipping 40 matching lines @@
         gfx::GLSurfaceHandle(surface_id_, gfx::NATIVE_DIRECT));
     // Register first, SetVisible() might create an OutputSurface.
     RegisterViewSurface(surface_id_, j_surface.obj());
     SetVisible(true);
     ANativeWindow_release(window);
   }
 }
 
 void CompositorImpl::CreateLayerTreeHost() {
   DCHECK(!host_);
-  DCHECK(!WillCompositeThisFrame());
-
-  // Just in case, since we immediately hide the LTH in this function,
-  // and we do not want to end up with a pending Composite task when the
-  // host is hidden.
-  base::AutoReset<bool> auto_reset_ignore_schedule(&ignore_schedule_composite_,
-                                                   true);
 
   cc::LayerTreeSettings settings;
   settings.renderer_settings.refresh_rate = 60.0;
   settings.renderer_settings.allow_antialiasing = false;
   settings.renderer_settings.highp_threshold_min = 2048;
   settings.use_zero_copy = true;
+  settings.use_external_begin_frame_source = true;
 
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   settings.initial_debug_state.SetRecordRenderingStats(
       command_line->HasSwitch(cc::switches::kEnableGpuBenchmarking));
   if (command_line->HasSwitch(cc::switches::kDisableCompositorPropertyTrees))
     settings.use_property_trees = false;
-  // TODO(enne): Update this this compositor to use the scheduler.
-  settings.single_thread_proxy_scheduler = false;
+  settings.single_thread_proxy_scheduler = true;
 
   settings.use_compositor_animation_timelines = !command_line->HasSwitch(
       switches::kDisableAndroidCompositorAnimationTimelines);
 
   cc::LayerTreeHost::InitParams params;
   params.client = this;
   params.shared_bitmap_manager = HostSharedBitmapManager::current();
   params.gpu_memory_buffer_manager = BrowserGpuMemoryBufferManager::current();
   params.task_graph_runner = g_task_graph_runner.Pointer();
   params.main_task_runner = base::ThreadTaskRunnerHandle::Get();
   params.settings = &settings;
+  params.external_begin_frame_source.reset(new ExternalBeginFrameSource(this));
   host_ = cc::LayerTreeHost::CreateSingleThreaded(this, &params);
   DCHECK(!host_->visible());
   host_->SetRootLayer(root_layer_);
   host_->SetViewportSize(size_);
   host_->set_has_transparent_background(has_transparent_background_);
   host_->SetDeviceScaleFactor(device_scale_factor_);
 
   if (needs_animate_)
     host_->SetNeedsAnimate();
 }
 
 void CompositorImpl::SetVisible(bool visible) {
   TRACE_EVENT1("cc", "CompositorImpl::SetVisible", "visible", visible);
   if (!visible) {
     DCHECK(host_->visible());
-    // Look for any layers that were attached to the root for readback
-    // and are waiting for Composite() to happen.
-    bool readback_pending = false;
-    for (size_t i = 0; i < root_layer_->children().size(); ++i) {
-      if (root_layer_->children()[i]->HasCopyRequest()) {
-        readback_pending = true;
-        break;
-      }
-    }
-    if (readback_pending) {

[brianderson, 2015/12/12 01:41:46]

Does this break readbacks at all?

[no sievers, 2015/12/17 00:52:32]

The pending requests will still signal completion (but not success) when layers
are torn down. I'm not so worried about this now, since I only added it
speculatively a while back when there was talk about a use case for doing
readbacks while hiding the activity (which made pending readbacks here likely in
theory), but we actually didn't end up making that change.

-      base::AutoReset<bool> auto_reset_ignore_schedule(
-          &ignore_schedule_composite_, true);
-      host_->Composite(base::TimeTicks::Now());
-    }
-    if (WillComposite())
-      CancelComposite();
     host_->SetVisible(false);
     if (!host_->output_surface_lost())
       host_->ReleaseOutputSurface();
     pending_swapbuffers_ = 0;
-    needs_composite_ = false;
-    composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
     establish_gpu_channel_timeout_.Stop();
     display_client_.reset();
-    if (current_composite_task_) {
-      current_composite_task_->Cancel();
-      current_composite_task_.reset();
-    }
   } else {
     host_->SetVisible(true);
     if (output_surface_request_pending_)
       RequestNewOutputSurface();
-    SetNeedsComposite();
+    host_->SetNeedsCommit();

[brianderson, 2015/12/12 01:41:46]

Is this needed? I think the impl side requests it if resources have been
discarded or if the output surface is lost.

[no sievers, 2015/12/17 00:52:32]

You're right, not needed. Removed.

   }
 }
 
 void CompositorImpl::setDeviceScaleFactor(float factor) {
   device_scale_factor_ = factor;
   if (host_)
     host_->SetDeviceScaleFactor(factor);
 }
 
 void CompositorImpl::SetWindowBounds(const gfx::Size& size) {
@@ skipping 10 matching lines @@
 
 void CompositorImpl::SetHasTransparentBackground(bool flag) {
   has_transparent_background_ = flag;
   if (host_)
     host_->set_has_transparent_background(flag);
 }
 
 void CompositorImpl::SetNeedsComposite() {
   if (!host_->visible())
     return;
-  DCHECK(!needs_composite_ || WillComposite());
-
-  needs_composite_ = true;
-  PostComposite(COMPOSITE_IMMEDIATELY);
+  host_->SetNeedsCommit();

[brianderson, 2015/12/12 01:41:46]

Was this why the SetNeedsCommit was needed above? Maybe remove the visibility
check?

[no sievers, 2015/12/17 00:52:32]

This is actually what the browser will call if it wants to draw (let's say for a
tab stack animation or what not). I had to change it to SetNeedsAnimate(), which
is actually the right thing, since SetNeedsCommit() is ignored during
UpdateLTH().
What the caller is trying to say here is 'call me again next frame when I update
my animation (i.e. I want to update layer properties in the next frame instead
of doing any invalidation now)'.

 }
 
 static scoped_ptr<WebGraphicsContext3DCommandBufferImpl>
 CreateGpuProcessViewContext(
     const scoped_refptr<GpuChannelHost>& gpu_channel_host,
     const blink::WebGraphicsContext3D::Attributes attributes,
     int surface_id) {
   GURL url("chrome://gpu/Compositor::createContext3D");
   static const size_t kBytesPerPixel = 4;
   gfx::DeviceDisplayInfo display_info;
@@ skipping 13 matching lines @@
       new WebGraphicsContext3DCommandBufferImpl(surface_id,
                                                 url,
                                                 gpu_channel_host.get(),
                                                 attributes,
                                                 lose_context_when_out_of_memory,
                                                 limits,
                                                 NULL));
 }
 
 void CompositorImpl::UpdateLayerTreeHost() {
-  base::AutoReset<bool> auto_reset_ignore_schedule(&ignore_schedule_composite_,
-                                                   true);
   client_->UpdateLayerTreeHost();
+  if (needs_animate_) {
+    needs_animate_ = false;
+    root_window_->Animate(base::TimeTicks::Now());
+  }
 }
 
 void CompositorImpl::OnGpuChannelEstablished() {
   establish_gpu_channel_timeout_.Stop();
   CreateOutputSurface();
 }
 
 void CompositorImpl::OnGpuChannelTimeout() {
   LOG(FATAL) << "Timed out waiting for GPU channel.";
 }
@@ skipping 109 matching lines @@
 }
 
 void CompositorImpl::DeleteUIResource(cc::UIResourceId resource_id) {
   host_->DeleteUIResource(resource_id);
 }
 
 bool CompositorImpl::SupportsETC1NonPowerOfTwo() const {
   return gpu_capabilities_.texture_format_etc1_npot;
 }
 
-void CompositorImpl::ScheduleComposite() {
-  if (ignore_schedule_composite_ || !host_->visible())
-    return;
-
-  DCHECK(!needs_composite_ || WillComposite());
-  needs_composite_ = true;
-  // We currently expect layer tree invalidations at most once per frame
-  // during normal operation and therefore try to composite immediately
-  // to minimize latency.
-  PostComposite(COMPOSITE_IMMEDIATELY);
-}
-
-void CompositorImpl::ScheduleAnimation() {
-  needs_animate_ = true;
-
-  if (!host_->visible())
-    return;
-
-  if (needs_composite_) {
-    DCHECK(WillComposite());
-    return;
-  }
-
-  TRACE_EVENT0("cc", "CompositorImpl::ScheduleAnimation");
-  needs_composite_ = true;
-  PostComposite(COMPOSITE_EVENTUALLY);
-}
-
 void CompositorImpl::DidPostSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidPostSwapBuffers");
-  did_post_swapbuffers_ = true;
+  pending_swapbuffers_++;
 }
 
 void CompositorImpl::DidCompleteSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidCompleteSwapBuffers");
   DCHECK_GT(pending_swapbuffers_, 0U);
-  if (pending_swapbuffers_-- == kMaxUiSwapBuffers && needs_composite_)
-    PostComposite(COMPOSITE_IMMEDIATELY);
+  pending_swapbuffers_--;
   client_->OnSwapBuffersCompleted(pending_swapbuffers_);

[brianderson, 2015/12/12 01:41:46]

It doesn't look like any implementers look at pending_swapbuffers_. Can you
delete the pending_swapbuffers_ member variable?

Also, can this be removed all together? I don't understand what needs to be tied
to when a swap occurs. What happens if there's no damage and we don't actually
swap?

[no sievers, 2015/12/17 00:52:32]

Unfortunately this is used in CompositorViewHolder.java, see
onSwapBuffersCompleted() where |pendingSwapBuffersCount == 0| is used to detect
when we go idle here in order to flush pending invalidations for Java Views
(IIRC).

[brianderson, 2015/12/17 06:14:53]

Ok. I see where it's used now and am okay keeping the code as-is for this patch.
It seems like an odd signal to rely on though. I wonder if it makes more sense
to pipe the BeginImplFrameNotExpectedSoon event up (as a content-is-idle signal)
to flush invalidations.

 }
 
 void CompositorImpl::DidAbortSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidAbortSwapBuffers");
   // This really gets called only once from
   // SingleThreadProxy::DidLoseOutputSurfaceOnImplThread() when the
   // context was lost.
   ScheduleComposite();
   client_->OnSwapBuffersCompleted(0);
 }
 
 void CompositorImpl::DidCommit() {
   root_window_->OnCompositingDidCommit();
 }
 
 void CompositorImpl::AttachLayerForReadback(scoped_refptr<cc::Layer> layer) {
   root_layer_->AddChild(layer);
 }
 
 void CompositorImpl::RequestCopyOfOutputOnRootLayer(
     scoped_ptr<cc::CopyOutputRequest> request) {
   root_layer_->RequestCopyOfOutput(request.Pass());
 }
 
 void CompositorImpl::OnVSync(base::TimeTicks frame_time,
                              base::TimeDelta vsync_period) {
-  vsync_period_ = vsync_period;
-  last_vsync_ = frame_time;
+  FOR_EACH_OBSERVER(VSyncObserver, observer_list_,
+                    OnVSync(frame_time, vsync_period));
+  if (needs_begin_frames_)
+    root_window_->RequestVSyncUpdate();
+}
 
-  if (WillCompositeThisFrame()) {
-    // We somehow missed the last vsync interval, so reschedule for deadline.
-    // We cannot schedule immediately, or will get us out-of-phase with new
-    // renderer frames.
-    CancelComposite();
-    composite_on_vsync_trigger_ = COMPOSITE_EVENTUALLY;
-  } else {
-    current_composite_task_.reset();
-  }
+void CompositorImpl::OnNeedsBeginFramesChange(bool needs_begin_frames) {
+  if (needs_begin_frames_ == needs_begin_frames)
+    return;
 
-  DCHECK(!DidCompositeThisFrame() && !WillCompositeThisFrame());
-  if (composite_on_vsync_trigger_ != DO_NOT_COMPOSITE) {
-    CompositingTrigger trigger = composite_on_vsync_trigger_;
-    composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
-    PostComposite(trigger);
-  }
-
-  FOR_EACH_OBSERVER(VSyncObserver, observer_list_,
-                    OnUpdateVSyncParameters(frame_time, vsync_period));
+  needs_begin_frames_ = needs_begin_frames;
+  if (needs_begin_frames_)
+    root_window_->RequestVSyncUpdate();
 }
 
 void CompositorImpl::SetNeedsAnimate() {
   needs_animate_ = true;
   if (!host_->visible())
     return;
 
   host_->SetNeedsAnimate();
 }
 
 }  // namespace content