Start of hardware overlay support in CC with Ubercompositor.

cc/output/overlay_renderer.cc

Index: cc/output/overlay_renderer.cc
diff --git a/cc/output/overlay_renderer.cc b/cc/output/overlay_renderer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b9b3fcf64441f44bc574441be32516866735e9f5
--- /dev/null
+++ b/cc/output/overlay_renderer.cc
@@ -0,0 +1,124 @@
+// Copyright 2014 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/overlay_renderer.h"
+
+#include "cc/output/output_surface.h"
+#include "cc/quads/draw_quad.h"
+#include "cc/quads/texture_draw_quad.h"
+#include "ui/gfx/rect_conversions.h"
+#include "ui/gfx/transform.h"
+
+namespace cc {
+
+namespace {
+
+class SingleOverlayOnTop : public OverlayRenderer::Strategy {
+ public:
+  SingleOverlayOnTop(OverlayCandidates* capability_checker,
+                     ResourceProvider* resource_provider);
+  virtual bool Attempt(RenderPassList* render_passes_in_draw_order) OVERRIDE;
+
+ private:
+  OverlayCandidates* capability_checker_;
+  ResourceProvider* resource_provider_;

[piman, 2014/03/14 03:42:34]

nit: DISALLOW_COPY_AND_ASSIGN

[alexst (slow to review), 2014/03/14 19:54:22]

Done.

+};
+
+SingleOverlayOnTop::SingleOverlayOnTop(OverlayCandidates* capability_checker,
+                                       ResourceProvider* resource_provider)
+    : capability_checker_(capability_checker),
+      resource_provider_(resource_provider) {}
+
+bool SingleOverlayOnTop::Attempt(RenderPassList* render_passes_in_draw_order) {

[piman, 2014/03/14 03:42:34]

I think it would be nice to expose this so that we can test it directly.

The OverlayCandidatesTest may not scale as we add more strategies.

Also, I think it would be useful to test negative cases, e.g. the trivial ones
(>1 render pass) but also cases that would fail below.

[alexst (slow to review), 2014/03/14 19:54:22]

I was planning to do it as we added more strategies, but might as well rip it
out into a separate file now and rig the class that uses it to be more testable.

+  // Only attempt to handle very simple case for now.
+  if (render_passes_in_draw_order->size() != 1 || !capability_checker_)
+    return false;
+
+  RenderPass* root_render_pass = render_passes_in_draw_order->back();
+  DCHECK(root_render_pass);
+
+  QuadList& quad_list = root_render_pass->quad_list;
+  QuadList::BackToFrontIterator candidate_quad = quad_list.BackToFrontEnd();
+  for (QuadList::BackToFrontIterator it = quad_list.BackToFrontBegin();
+       it != quad_list.BackToFrontEnd();
+       ++it) {
+    const DrawQuad& quad = *(*it);
+    // Don't try for multiple surfaces or if something is on top of
+    // our candidate.
+    if (candidate_quad != quad_list.BackToFrontEnd())
+      return false;

[piman, 2014/03/14 03:42:34]

You're only setting candidate_quad right before you break. I don't think this
condition can ever be true.

Did you intending for this to catch cases where there's any quad after the
candidate (to avoid any potential intersection)?


If you don't want to allow any quad in front of the candidate (very reasonable
for the simplest strategy) it seems like you could do away with the loop, by
only checking the front-most quad, and promote it iff it satisfies the
conditions.
It would also allow more cases (e.g. if the first and last quad are promotable
textures, current code would fail, whereas we could promote the frontmost one)

[alexst (slow to review), 2014/03/14 19:54:22]

You are right, I was simplifying old test code for this and it need not be as
wordy.

+
+    if (quad.material == DrawQuad::TEXTURE_CONTENT) {
+      ResourceProvider::ResourceId resource_id =
+          TextureDrawQuad::MaterialCast(*it)->resource_id;
+
+      if (!resource_provider_->HasNativeBufferBacking(resource_id))
+        continue;
+
+      candidate_quad = it;
+      break;
+    }
+  }
+
+  // Found nothing useful.
+  if (candidate_quad == quad_list.BackToFrontEnd())
+    return false;
+
+  // Add our primary surface.
+  OverlayCandidates::OverlaySurfaceCandidateList candidates;
+  OverlayCandidates::OverlaySurfaceCandidate main_image;
+  main_image.display_rect = root_render_pass->output_rect;
+  main_image.format = RGBA_8888;
+  candidates.push_back(main_image);
+
+  // Add the overlay.
+  OverlayCandidates::OverlaySurfaceCandidate candidate;
+  const TextureDrawQuad& quad = *TextureDrawQuad::MaterialCast(*candidate_quad);
+  gfx::RectF float_rect(quad.rect);
+  quad.quadTransform().TransformRect(&float_rect);

[piman, 2014/03/14 03:42:34]

This really computes the bounding box of the transformed rect. I don't think
it's the right thing to do in the general case... don't we want to extract an
"OverlayTransform" out of the transform first (if appropriate, otherwise fail),
and pass that to candidate?

[alexst (slow to review), 2014/03/14 19:54:22]

This meant to have a check for identity or translation and fail otherwise. I
want to write the quadTransform -> overlayTransform carefully and separately.

+  candidate.display_rect = gfx::ToNearestRect(float_rect);
+  candidate.crop_rect = BoundingRect(quad.uv_top_left, quad.uv_bottom_right);
+  candidate.format = RGBA_8888;
+  candidates.push_back(candidate);
+
+  // Check for support.
+  capability_checker_->CheckOverlaySupport(&candidates);

[enne (OOO), 2014/03/14 18:13:41]

I'm sure there's a wrinkle I'm not seeing, but why can't this Check function
just return true if it can handle the entire list? Is this so that you could
hand it multiple candidates and maybe it could choose to handle some of them?

[alexst (slow to review), 2014/03/14 19:54:22]

Exactly right. Maybe HW can't handle 4 things, but it can handle 3 if we
manually collapsed the bottom 2 layers. The checker can signal that.

+
+  // If the candidate can be handled by an overlay, create a pass for it.
+  if (candidates[1].overlay_handled) {
+    scoped_ptr<RenderPass> overlay_pass = RenderPass::Create();
+    overlay_pass->overlay_state = RenderPass::SIMPLE_OVERLAY;
+
+    scoped_ptr<DrawQuad> overlay_quad =
+        quad_list.take((candidate_quad + 1).base());

[piman, 2014/03/14 03:42:34]

I think it would be clearer if candidate_quad was a simple QuadList::iterator,
and the reverse_iterator->iterator conversion happened when we assign
candidate_quad.

+    quad_list.erase((candidate_quad + 1).base());
+    overlay_pass->quad_list.push_back(overlay_quad.PassAs<DrawQuad>());

[piman, 2014/03/14 03:42:34]

nit: just Pass() ?

+    render_passes_in_draw_order->insert(render_passes_in_draw_order->begin(),
+                                        overlay_pass.PassAs<RenderPass>());

[piman, 2014/03/14 03:42:34]

nit: just Pass()?

+  }
+  return true;
+}
+}
+
+OverlayRenderer::OverlayRenderer(OutputSurface* surface,
+                                 ResourceProvider* resource_provider) {
+  DCHECK(surface);
+  OverlayCandidates* candidates = surface->overlay_candidates();
+  if (candidates) {
+    strategies_.push_back(scoped_ptr<Strategy>(
+        new SingleOverlayOnTop(candidates, resource_provider)));
+  }
+}
+
+OverlayRenderer::~OverlayRenderer() {}
+
+void OverlayRenderer::ProcessForOverlays(
+    RenderPassList* render_passes_in_draw_order) {
+  for (size_t i = 0; i < strategies_.size(); i++) {

[piman, 2014/03/14 03:42:34]

nit: use iterators and win on the bounds checks.

+    if (strategies_[i]->Attempt(render_passes_in_draw_order))
+      return;
+  }
+}
+
+}  // namespace cc