Plumb overlay processing into DirectRenderer.

cc/output/gl_renderer.cc

Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 89b1e00554a01a3e2eed1ac300305bae578e7be0..4dc877f53d5658824030d1ed874c8b86f01c7f3b 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -201,6 +201,7 @@ GLRenderer::GLRenderer(RendererClient* client,
                        TextureMailboxDeleter* texture_mailbox_deleter,
                        int highp_threshold_min)
     : DirectRenderer(client, settings, output_surface, resource_provider),
+      overlay_child_(0),
       offscreen_framebuffer_id_(0),
       shared_geometry_quad_(gfx::RectF(-0.5f, -0.5f, 1.0f, 1.0f)),
       gl_(output_surface->context_provider()->ContextGL()),
@@ -2000,6 +2001,8 @@ void GLRenderer::FinishDrawingFrame(DrawingFrame* frame) {
 
   GLC(gl_, gl_->Disable(GL_BLEND));
   blend_shadow_ = false;
+
+  ScheduleOverlays(frame);
 }
 
 void GLRenderer::FinishDrawingQuadList() { FlushTextureQuadCache(); }
@@ -2171,6 +2174,22 @@ void GLRenderer::SwapBuffers(const CompositorFrameMetadata& metadata) {
   }
   output_surface_->SwapBuffers(&compositor_frame);
 
+  // Release previously used overlay resources and hold onto the pending ones
+  // until the next swap buffers.
+  ReturnedResourceArray unused_overlay_resources;
+  for (TransferableResourceArray::const_iterator it =
+           in_use_overlay_resources_.begin();
+       it != in_use_overlay_resources_.end();
+       ++it) {
+    unused_overlay_resources.push_back(it->ToReturnedResource());
+  }

[piman, 2014/03/26 03:43:33]

nit: TransferableResource::ReturnResources

+
+  if (unused_overlay_resources.size())

[piman, 2014/03/26 03:43:33]

nit: if (!unused_overlay_resources.empty())

+    resource_provider()->ReceiveReturnsFromParent(unused_overlay_resources);
+
+  in_use_overlay_resources_.clear();
+  in_use_overlay_resources_.swap(pending_overlay_resources_);
+
   swap_buffer_rect_ = gfx::Rect();
 
   // We don't have real fences, so we mark read fences as passed
@@ -3047,4 +3066,74 @@ bool GLRenderer::IsContextLost() {
   return output_surface_->context_provider()->IsContextLost();
 }
 
+static void CollectResources(const ReturnedResourceArray& returned) {}

[alexst (slow to review), 2014/03/26 01:23:51]

Since these resources never crossed any process boundaries during
PrepareSendToParent/ReceiveReturnsFromParent,  and child and parent are
technically the same here, if I understand it correctly, they'll get sent back
to the actual child renderers on the next sweep? Or is there more than meets the
eye?

[piman, 2014/03/26 03:43:33]

This will be called from DeclareUsedResourcesFromChild when a resource goes from
used to !used, with the count of how many times it was "imported". That's
because we only get a sync point when we're fully done with the resource.
However in this case, because the parent and child are the same, it's ok to not
have a sync point (what ToReturnedResource above does), and return the resource
multiple times with a count of 1 each time, as you're doing.
OTOH returning directly from here would potentially be "too early" since in
theory the producer could kick commands to modify the resource before the
SwapBuffers that takes them out happened. I suppose you could delay the
DeclareUsedResourcesFromChild until the SwapBuffers and ReceiveReturnsFromParent
directly from here. It may be slightly simpler code (no need to track
pending_overlay_resources_ and in_use_overlay_resources_, just need to "pass"
|resources| from ScheduleOverlays to SwapBuffers).

However, see below for alternatives.

+
+static ResourceProvider::ResourceId ResourceRemapHelper(
+    const ResourceProvider::ResourceIdMap& child_to_parent_map,
+    ResourceProvider::ResourceIdArray* resources_in_frame,
+    ResourceProvider::ResourceId id) {
+
+  ResourceProvider::ResourceIdMap::const_iterator it =
+      child_to_parent_map.find(id);
+
+  DCHECK(it != child_to_parent_map.end());
+  DCHECK_EQ(it->first, id);
+  ResourceProvider::ResourceId remapped_id = it->second;
+  resources_in_frame->push_back(id);
+  return remapped_id;
+}
+
+void GLRenderer::ScheduleOverlays(DrawingFrame* frame) {
+  if (!frame->overlay_list.size())
+    return;
+
+  if (!overlay_child_) {
+    overlay_child_ =
+        resource_provider_->CreateChild(base::Bind(&CollectResources));
+  }
+
+  ResourceProvider::ResourceIdArray resources;
+  OverlayCandidateList& overlays = frame->overlay_list;
+  OverlayCandidateList::iterator it;
+  for (it = overlays.begin(); it != overlays.end(); ++it) {
+    const OverlayCandidate& overlay = *it;
+    // Skip primary plane.
+    if (overlay.plane_z_order == 0)
+      continue;
+
+    DCHECK(overlay.resource_id);
+    resources.push_back(overlay.resource_id);
+  }
+
+  // Make sure the overlay resources appear as exported until it is
+  // safe to return them to their respective renderers.
+  resource_provider()->PrepareSendToParent(resources,
+                                           &pending_overlay_resources_);
+  resource_provider()->ReceiveFromChild(overlay_child_,
+                                        pending_overlay_resources_);

[piman, 2014/03/26 03:43:33]

So, this is unexpected. Looking at this code, it kinda makes sense, but I wonder
if there is a better way.

Basically, we're using the resource provider to export resources, and then
re-import them, essentially just to manage the "exported" count. This has some
side effects at the GL level (we will create and destroy textures, not a big
deal, just slightly wasteful, and send extra IPCs to insert sync points that
we'll never need), and requires the remapping below... Conceptually it feels
like a lot of overhead, even though it's probably not measurable (since we have
at most a handful of overlays).

Some ways we could make it simpler/cheaper:
- have ScheduleOverlayPlane take a mailbox directly instead of a texture id. The
service side can do the translation. That way we wouldn't need to manage the
Child side at all. We can use pending_overlay_resources_ to translate
resource->mailbox
- have an "export lock" thing, that we keep around together with the
ScopedReadLockGL (or have a superclass that does both the  ScopedReadLockGL and
the export refcount), so that we don't even need to do the PrepareSendToParent
and friends
- have the lock_for_read_count taken into account before returning/deleting
resources in ResourceProvider

+
+  const ResourceProvider::ResourceIdMap& resource_map =
+      resource_provider_->GetChildToParentMap(overlay_child_);
+  resources.clear();
+  for (it = overlays.begin(); it != overlays.end(); ++it) {
+    const OverlayCandidate& overlay = *it;
+    if (overlay.plane_z_order == 0)
+      continue;
+
+    ResourceProvider::ResourceId overlay_id =
+        ResourceRemapHelper(resource_map, &resources, overlay.resource_id);
+
+    ResourceProvider::ScopedReadLockGL overlay_resource_lock(
+        resource_provider(), overlay_id);
+
+    context_support_->ScheduleOverlayPlane(overlay.plane_z_order,
+                                           overlay.transform,
+                                           overlay_resource_lock.texture_id(),
+                                           overlay.display_rect,
+                                           overlay.uv_rect);
+  }
+
+  resource_provider()->DeclareUsedResourcesFromChild(overlay_child_, resources);
+}
+
 }  // namespace cc