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_outside_vsync_(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(

[Sami, 2014/05/09 13:21:14]

Is there a chance we could post this task several times?

[no sievers, 2014/05/09 22:53:44]

ScheduleComposite() has an early-out if needs_composite_ is already true.
So only in some more rare cases when we are throttling, we'd post multiple
tasks, but we should still not end up actually compositing too often.

+      FROM_HERE,
+      base::Bind(&CompositorImpl::Composite, weak_factory_.GetWeakPtr(), false),
+      delay);
+}
+
+void CompositorImpl::Composite(bool is_vsync) {
+  if (!host_)
+    return;
+
+  if (!needs_composite_)
+    return;
+
+  if (!is_vsync && should_composite_on_vsync_) {
+    TRACE_EVENT0("compositor", "CompositorImpl_DeferCompositeToVSync");
+    root_window_->RequestVSyncUpdate();
+    return;
+  }
+
+  // Don't Composite immediately more than once in between vsync ticks.

[Sami, 2014/05/09 13:21:14]

Maybe add something like this as a safeguard: DCHECK(!(is_vsync &&
did_composite_outside_vsync));

[no sievers, 2014/05/09 22:53:44]

Actually obsolete now with your other comment addressed
(did_composite_outside_vsync_ -> did_composite_this_frame_).
Now both can happen when we transition from one way of scheduling to the other.

+  if (did_composite_outside_vsync_) {
+    TRACE_EVENT0("compositor", "CompositorImpl_ThrottleComposite");
+    PostComposite(vsync_period_);

[Sami, 2014/05/09 13:21:14]

Just thinking out loud here: we generally composite immediately if there's a new
frame from the renderer. We can't tell in advance when that happens. If that
happens twice per frame, then it seems like there's something wrong with
BeginFrame scheduling and we should be hitting this case regularly.

[no sievers, 2014/05/09 22:53:44]

Yea I was wondering too and logged when it happens, but fortunately I don't see
it happen too often (probably glow effect and things can cause it).

+    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_);

[Sami, 2014/05/09 13:21:14]

Should we instead trigger the composite as soon as the swap limit clears?
Otherwise this can add some latency if the GPU is backed up.

[no sievers, 2014/05/09 22:53:44]

I was actually wondering if this is better for latency, i.e. to skip a frame
instead of keeping up as much backpressure as possible. wdyt?

Again, seems like with a deadline this would be way easier :/

+    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.
+  ignore_schedule_composite_ = true;
+  client_->Layout();
+  ignore_schedule_composite_ = false;
+
+  did_post_swapbuffers_ = false;
+  host_->Composite(gfx::FrameTime::Now());

[Sami, 2014/05/09 13:21:14]

This should probably be derived from the most recent vsync time we have.

[no sievers, 2014/05/09 22:53:44]

Hmm if we composite on vsync, now is the vsync time.
Otherwise would we really want to use vsync time? AURA seems to be using now()
also.

+  if (did_post_swapbuffers_)
+    pending_swapbuffers_++;
+
+  if (!is_vsync) {
+    // Need to track vsync to avoid compositing more than once per frame.
+    root_window_->RequestVSyncUpdate();
+    did_composite_outside_vsync_ = 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_outside_vsync_ = false;
+    should_composite_on_vsync_ = false;
+    pending_swapbuffers_ = 0;
     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() {
+  if (!host_.get())
+    return;
+
+  needs_composite_ = true;
+  should_composite_on_vsync_ = true;

[Sami, 2014/05/09 13:21:14]

Doesn't this have the effect of re-scheduling any pending (immediate) composite
operation to the next vsync? Not sure if that happens in practice, but maybe we
should early out here if needs_composite_ == true.

[no sievers, 2014/05/09 22:53:44]

Done.

+  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 Composite() below.

[Sami, 2014/05/09 13:21:14]

Composite() is above, not below :)

[no sievers, 2014/05/09 22:53:44]

Done :)

+  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;
+  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_--);
 }
 
 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_outside_vsync_ = false;
+
+  if (should_composite_on_vsync_)
+    Composite(true);
+}
+
 }  // namespace content