Android: Move scheduling logic to CompositorImpl

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 220 matching lines @@
 }
 
 CompositorImpl::CompositorImpl(CompositorClient* client,
                                gfx::NativeWindow root_window)
     : root_layer_(cc::Layer::Create()),
       has_transparent_background_(false),
       device_scale_factor_(1),
       window_(NULL),
       surface_id_(0),
       client_(client),
-      root_window_(root_window) {
+      root_window_(root_window),
+      did_post_swapbuffers_(false),
+      ignore_schedule_composite_(false),
+      needs_composite_(false),
+      should_composite_on_vsync_(false),
+      did_composite_this_frame_(false),
+      pending_swapbuffers_(0U),
+      weak_factory_(this) {
   DCHECK(client);
   DCHECK(root_window);
   ImageTransportFactoryAndroid::AddObserver(this);
   root_window->AttachCompositor(this);
 }
 
 CompositorImpl::~CompositorImpl() {
   root_window_->DetachCompositor();
   ImageTransportFactoryAndroid::RemoveObserver(this);
   // Clean-up any surface references.
   SetSurface(NULL);
 }
 
-void CompositorImpl::Composite() {
-  if (host_)
-    host_->Composite(gfx::FrameTime::Now());
+void CompositorImpl::PostComposite(base::TimeDelta delay) {
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&CompositorImpl::Composite,
+                 weak_factory_.GetWeakPtr(),
+                 COMPOSITE_IMMEDIATELY),
+      delay);
+}
+
+void CompositorImpl::Composite(CompositingTrigger trigger) {
+  if (!host_)
+    return;
+
+  if (!needs_composite_)
+    return;
+
+  if (trigger != COMPOSITE_ON_VSYNC && should_composite_on_vsync_) {
+    TRACE_EVENT0("compositor", "CompositorImpl_DeferCompositeToVSync");
+    root_window_->RequestVSyncUpdate();
+    return;
+  }
+
+  // Don't Composite more than once in between vsync ticks.
+  if (did_composite_this_frame_) {
+    TRACE_EVENT0("compositor", "CompositorImpl_ThrottleComposite");
+    if (should_composite_on_vsync_)
+      root_window_->RequestVSyncUpdate();
+    else
+      PostComposite(vsync_period_);

[brianderson, 2014/05/10 00:24:40]

Why PostComposite a vsync period away here instead of just transitioning to
vsync scheduling?

[no sievers, 2014/05/12 19:49:41]

It would transition to vsync scheduling immediately if SetNeedsComposite() was
called (should_composite_on_vsync_).

The else is for when there was another tree modification, which should not
happen in normal operation. In that case I wanted to make sure that it does not
get lost, but we also don't unnecessarily change the phase.

+    return;
+  }
+
+  const unsigned int kMaxSwapBuffers = 2U;
+  DCHECK_LE(pending_swapbuffers_, kMaxSwapBuffers);
+  if (pending_swapbuffers_ == kMaxSwapBuffers) {
+    TRACE_EVENT0("compositor", "CompositorImpl_SwapLimit");
+    if (should_composite_on_vsync_)
+      root_window_->RequestVSyncUpdate();
+    else
+      PostComposite(vsync_period_);
+    return;
+  }
+
+  // Reset state before Layout+Composite since that might create more
+  // requests to Composite that we need to respect.
+  needs_composite_ = false;
+  should_composite_on_vsync_ = false;
+
+  // Ignore ScheduleComposite() from layer tree changes during Layout.

[brianderson, 2014/05/10 00:24:40]

What happens if you don't ignore?

[no sievers, 2014/05/12 19:49:41]

It would post a task to Composite() unnecessarily.
It would also reset needs_composite_ to true.

I'm worried that it might composite unnecessarily for a while, as long as it
happens so that Chrome does end up touching one of the layers. On the other
hand, if the animation was really still going on it would force Composite()
through SetNeedsComposite().

+  ignore_schedule_composite_ = true;
+  client_->Layout();
+  ignore_schedule_composite_ = false;
+
+  did_post_swapbuffers_ = false;
+  host_->Composite(gfx::FrameTime::Now());
+  if (did_post_swapbuffers_)
+    pending_swapbuffers_++;
+
+  if (trigger != COMPOSITE_ON_VSYNC) {
+    // Need to track vsync to avoid compositing more than once per frame.
+    root_window_->RequestVSyncUpdate();
+  }
+  did_composite_this_frame_ = true;
 }
 
 void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
   root_layer_->RemoveAllChildren();
   if (root_layer)
     root_layer_->AddChild(root_layer);
 }
 
 void CompositorImpl::SetWindowSurface(ANativeWindow* window) {
   GpuSurfaceTracker* tracker = GpuSurfaceTracker::Get();
@@ skipping 41 matching lines @@
     content::RegisterViewSurface(surface_id_, j_surface.obj());
   }
 }
 
 void CompositorImpl::SetVisible(bool visible) {
   if (!visible) {
     ui_resource_map_.clear();
     host_.reset();
     client_->UIResourcesAreInvalid();
   } else if (!host_) {
+    needs_composite_ = false;
+    did_composite_this_frame_ = false;
+    should_composite_on_vsync_ = false;
+    pending_swapbuffers_ = 0;

[brianderson, 2014/05/10 00:24:40]

Do you not get pending acks when you go invisible?

[no sievers, 2014/05/12 19:49:41]

Yes, I was running into accounting problems and it would stop updating after
becoming visible again.

I didn't fully investigate but am guessing that the callbacks can get dropped
when we synchronously tear down the LTH and context (and everything that belongs
to it like CommandBufferProxyImpl etc.) above.

We also had a reset logic in the previous code in ContentViewRenderView, but
interestingly only when the surface got recreated.

     cc::LayerTreeSettings settings;
     settings.refresh_rate = 60.0;
     settings.impl_side_painting = false;
     settings.allow_antialiasing = false;
     settings.calculate_top_controls_position = false;
     settings.top_controls_height = 0.f;
     settings.highp_threshold_min = 2048;
 
     CommandLine* command_line = CommandLine::ForCurrentProcess();
     settings.initial_debug_state.SetRecordRenderingStats(
@@ skipping 38 matching lines @@
     host_->set_has_transparent_background(flag);
 }
 
 bool CompositorImpl::CompositeAndReadback(void *pixels, const gfx::Rect& rect) {
   if (host_)
     return host_->CompositeAndReadback(pixels, rect);
   else
     return false;
 }
 
+void CompositorImpl::SetNeedsComposite() {

[brianderson, 2014/05/10 00:24:40]

Is this the signal we get when the tab stack wants an update?

[no sievers, 2014/05/12 19:49:41]

Yea exactly. It's needed because we don't get a call to ScheduleComposite()
because nothing gets changed in the layer tree.

It currently works the opposite way, more sort of like SetNeedsAnimate() with
the difference that it's not only changing properties on a layer but might
switch out entire layers (hence using Layout() callback before Composite()).

+  if (!host_.get() || needs_composite_)
+    return;
+
+  needs_composite_ = true;
+
+  // For explicit requests we try to composite regularly on vsync.
+  should_composite_on_vsync_ = true;
+  root_window_->RequestVSyncUpdate();
+}
+
 cc::UIResourceId CompositorImpl::GenerateUIResourceFromUIResourceBitmap(
     const cc::UIResourceBitmap& bitmap,
     bool is_transient) {
   if (!host_)
     return 0;
 
   cc::UIResourceId id = 0;
   scoped_ptr<cc::UIResourceClient> resource;
   if (is_transient) {
     scoped_ptr<TransientUIResource> transient_resource =
@@ skipping 74 matching lines @@
   return make_scoped_ptr(
       new WebGraphicsContext3DCommandBufferImpl(surface_id,
                                                 url,
                                                 gpu_channel_host.get(),
                                                 attributes,
                                                 lose_context_when_out_of_memory,
                                                 limits,
                                                 NULL));
 }
 
+void CompositorImpl::Layout() {
+  // TODO: If we get this callback from the SingleThreadProxy, we need
+  // to stop calling it ourselves in CompositorImpl::Composite().
+  NOTREACHED();
+  client_->Layout();
+}
+
 scoped_ptr<cc::OutputSurface> CompositorImpl::CreateOutputSurface(
     bool fallback) {
   blink::WebGraphicsContext3D::Attributes attrs;
   attrs.shareResources = true;
   attrs.noAutomaticFlushes = true;
 
   DCHECK(window_);
   DCHECK(surface_id_);
 
   scoped_refptr<ContextProviderCommandBuffer> context_provider =
       ContextProviderCommandBuffer::Create(
           CreateGpuProcessViewContext(attrs, surface_id_), "BrowserCompositor");
   if (!context_provider.get()) {
     LOG(ERROR) << "Failed to create 3D context for compositor.";
     return scoped_ptr<cc::OutputSurface>();
   }
 
   return scoped_ptr<cc::OutputSurface>(
       new OutputSurfaceWithoutParent(context_provider));
 }
 
 void CompositorImpl::OnLostResources() {
   client_->DidLoseResources();
 }
 
-void CompositorImpl::DidCompleteSwapBuffers() {
-  client_->OnSwapBuffersCompleted();
-}
+void CompositorImpl::ScheduleComposite() {
+  if (needs_composite_ || ignore_schedule_composite_)
+    return;
 
-void CompositorImpl::ScheduleComposite() {
-  client_->ScheduleComposite();
+  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(base::TimeDelta());
 }
 
 void CompositorImpl::ScheduleAnimation() {
   ScheduleComposite();
 }
 
 void CompositorImpl::DidPostSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidPostSwapBuffers");
-  client_->OnSwapBuffersPosted();
+  did_post_swapbuffers_ = true;
+}
+
+void CompositorImpl::DidCompleteSwapBuffers() {
+  TRACE_EVENT0("compositor", "CompositorImpl::DidCompleteSwapBuffers");
+  DCHECK_GT(pending_swapbuffers_, 0U);
+  client_->OnSwapBuffersCompleted(pending_swapbuffers_--);

[brianderson, 2014/05/10 00:24:40]

Can you decrement after the function call?

[no sievers, 2014/05/12 19:49:41]

Isn't that the same?

 }
 
 void CompositorImpl::DidAbortSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidAbortSwapBuffers");
-  client_->OnSwapBuffersCompleted();
+  DCHECK_GT(pending_swapbuffers_, 0U);
+  client_->OnSwapBuffersCompleted(pending_swapbuffers_--);
 }
 
 void CompositorImpl::DidCommit() {
   root_window_->OnCompositingDidCommit();
 }
 
 void CompositorImpl::AttachLayerForReadback(scoped_refptr<cc::Layer> layer) {
   root_layer_->AddChild(layer);
 }
 
+void CompositorImpl::OnVSync(base::TimeTicks frame_time,
+                             base::TimeDelta vsync_period) {
+  vsync_period_ = vsync_period;
+  did_composite_this_frame_ = false;
+
+  if (should_composite_on_vsync_)
+    Composite(COMPOSITE_ON_VSYNC);
+}
+
 }  // namespace content