Ozone overlay candidates

ui/ozone/platform/dri/gbm_surface_factory.cc

Index: ui/ozone/platform/dri/gbm_surface_factory.cc
diff --git a/ui/ozone/platform/dri/gbm_surface_factory.cc b/ui/ozone/platform/dri/gbm_surface_factory.cc
index 6e354b8b8a21c25e4808dc4e9a38ea13d4ab24bb..01d084ee1a3debd78893234d4a8ab6f863fa74ea 100644
--- a/ui/ozone/platform/dri/gbm_surface_factory.cc
+++ b/ui/ozone/platform/dri/gbm_surface_factory.cc
@@ -13,9 +13,34 @@
 #include "ui/ozone/platform/dri/gbm_surfaceless.h"
 #include "ui/ozone/platform/dri/screen_manager.h"
 #include "ui/ozone/public/native_pixmap.h"
+#include "ui/ozone/public/overlay_candidates_ozone.h"
 #include "ui/ozone/public/surface_ozone_egl.h"
 
 namespace ui {
+namespace {

[dnicoara, 2014/08/20 19:36:38]

Please add blank line between namespaces and between namespace and class (same
at the end).

[achaulk, 2014/08/21 15:16:32]

Done.

+class SingleOverlayOzone : public OverlayCandidatesOzone {

[dnicoara, 2014/08/20 19:36:38]

As a rule, please don't use Ozone in internal implementation names.

[achaulk, 2014/08/21 15:16:32]

Acknowledged.

+ public:

[dnicoara, 2014/08/20 19:36:39]

needs a virtual destructor. Also, please add a DISALLOW_COPY_AND_ASSIGN

[achaulk, 2014/08/21 15:16:32]

Done.

+  static bool IsTransformSupported(gfx::OverlayTransform transform) {

[dnicoara, 2014/08/20 19:36:38]

There's no need for a static, just make it private.

+    switch (transform) {
+      case gfx::OVERLAY_TRANSFORM_NONE:
+        return true;
+      default:
+        return false;
+    }
+  }
+  virtual void CheckOverlaySupport(OverlaySurfaceCandidateList* surfaces) {

[dnicoara, 2014/08/20 19:36:38]

nit: rename surfaces to candidates (or surface_candidates)

+    if (surfaces->size() == 2) {

[alexst (slow to review), 2014/08/20 19:52:36]

So this isn't universally true, some HW may not even have an overlay, and so
this will break things. GBM platform is becoming a generic ChromeOS one.

I think for testing it's useful, but we should hide this under a flag,
--ozone-test-support-one-overlay or something.

In the general case, we should add a package to the CROS image, call it
overlay_candidates.so which would then handle all the device specifics. We
should load it here and defer to HW.

[achaulk, 2014/08/21 15:16:32]

Sure, a flag makes sense

+      const OverlayCandidatesOzone::OverlaySurfaceCandidate& first =

[dnicoara, 2014/08/20 19:36:38]

Is this one marked to true by default? It isn't specified in the documentation.
Should we just treat all the candidates the same and mark it as handled in here?

[achaulk, 2014/08/21 15:16:32]

I think the idea is we mark all surfaces we want to get sent separately

[dnicoara, 2014/08/21 15:57:20]

Sorry, I think something went over my head. Is the primary plane treated
specially? Shouldn't we treat it the same way. I'm thinking of what happens if
we want to do some unsupported transformation on the primary?

[achaulk, 2014/08/21 16:04:29]

Well normally the primary plane wouldn't get transformed at all right? And it
won't be sent separately as an overlay

[dnicoara, 2014/08/22 02:00:15]

But in surfaceless, the primary plane is an "overlay", isn't it? At least it is
part of |surfaces|, so I'd assume we want to do some basic assertions.

[achaulk, 2014/08/22 02:43:11]

Yes, but surfaceless unconditionally sends the primary plane as an overlay -
it's independent from this path

+          (*surfaces)[0];
+      OverlayCandidatesOzone::OverlaySurfaceCandidate& second = (*surfaces)[1];
+      if (first.plane_z_order == 0 && second.plane_z_order > 0 &&

[dnicoara, 2014/08/21 15:57:20]

Are these guaranteed to be in order? Why not treat them all the same?

[achaulk, 2014/08/21 16:04:29]

They are in the only thing that sends us planes so far

[dnicoara, 2014/08/22 02:00:14]

I think we shouldn't assume order. Or if we do, then we should make it explicit
in the interface description.

If we assume order, it might make more sense to move 'first.plane_z_order == 0'
into a DCHECK.

[achaulk, 2014/08/22 17:55:01]

Done.

+          IsTransformSupported(second.transform)) {
+        second.overlay_handled = true;
+      }
+    }
+  }
+};
+}  // namespace
 
 GbmSurfaceFactory::GbmSurfaceFactory(bool allow_surfaceless)
     : DriSurfaceFactory(NULL, NULL),
@@ -125,6 +150,11 @@ scoped_refptr<ui::NativePixmap> GbmSurfaceFactory::CreateNativePixmap(
   return scoped_refptr<GbmPixmap>(new GbmPixmap(buffer));
 }
 
+OverlayCandidatesOzone* GbmSurfaceFactory::GetOverlayCandidates(
+    gfx::AcceleratedWidget w) {
+  return new SingleOverlayOzone();
+}
+
 bool GbmSurfaceFactory::ScheduleOverlayPlane(
     gfx::AcceleratedWidget widget,
     int plane_z_order,