Fixes to Cast use of overlays for video frames

ui/ozone/platform/cast/overlay_manager_cast.cc

Index: ui/ozone/platform/cast/overlay_manager_cast.cc
diff --git a/ui/ozone/platform/cast/overlay_manager_cast.cc b/ui/ozone/platform/cast/overlay_manager_cast.cc
index cb7d6419ba97d83aeacc4b1f1578694c579e1db5..ed7a7d2479c640bb1e25ab7db0bee4c45872a3a1 100644
--- a/ui/ozone/platform/cast/overlay_manager_cast.cc
+++ b/ui/ozone/platform/cast/overlay_manager_cast.cc
@@ -4,6 +4,11 @@
 
 #include "ui/ozone/platform/cast/overlay_manager_cast.h"
 
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/single_thread_task_runner.h"
+#include "chromecast/media/base/media_message_loop.h"
 #include "chromecast/public/cast_media_shlib.h"
 #include "chromecast/public/graphics_types.h"
 #include "chromecast/public/video_plane.h"
@@ -13,6 +18,16 @@
 namespace ui {
 namespace {
 
+void SetVideoPlaneGeometry(
+    const chromecast::RectF& display_rect,
+    chromecast::media::VideoPlane::CoordinateType coordinate_type,
+    chromecast::media::VideoPlane::Transform transform) {
+  chromecast::media::VideoPlane* video_plane =
+      chromecast::media::CastMediaShlib::GetVideoPlane();
+  CHECK(video_plane);
+  video_plane->SetGeometry(display_rect, coordinate_type, transform);
+}
+
 // Translates a gfx::OverlayTransform into a VideoPlane::Transform.
 // Could be just a lookup table once we have unit tests for this code
 // to ensure it stays in sync with OverlayTransform.
@@ -37,8 +52,19 @@ chromecast::media::VideoPlane::Transform ConvertTransform(
   }
 }
 
+bool ExactlyEqual(const chromecast::RectF& r1, const chromecast::RectF& r2) {

[erickung1, 2015/07/08 17:12:33]

wonder if this can become util function?

Although graphics_types.h is in the public folder now and we intend not to have
.cc function implemented.

[halliwell, 2015/07/08 17:16:41]

I'm not sure how widely useful exact float comparisons will be for this struct
... I'd suggest moving it if we end up needing again (presumably has to be an
inline function in a public/ header)

+  return r1.x == r2.x && r1.y == r2.y && r1.width == r2.width &&
+         r1.height == r2.height;
+}
+
 class OverlayCandidatesCast : public OverlayCandidatesOzone {
  public:
+  OverlayCandidatesCast()
+      : media_task_runner_(
+            chromecast::media::MediaMessageLoop::GetTaskRunner()),
+        transform_(gfx::OVERLAY_TRANSFORM_INVALID),
+        display_rect_(0, 0, 0, 0) {}
+
   void CheckOverlaySupport(OverlaySurfaceCandidateList* surfaces) override {

[gunsch, 2015/07/08 16:02:52]

readability nit: there are now four consecutive close braces below and I wasn't
able to easily scan what goes where. could you (optionally, either/both):

1) not inline this definition into the header

2) change "if (candidate.plane_z_order == -1) { ... }" to "if
(candidate.plane_z_order != -1) { continue; }" and drop a level of indentation?

[erickung1, 2015/07/08 17:12:33]

or 

for (...) {
  if (candidate.plane_z_order != -1)
    continue;
  
  /* block of code */

  if (candidate.transform == transform_ &&
      ExactlyEqual(display_rect, display_rect_))
    continue;

  /* block of code: assign value and post task */
  return;

}

  
}

[halliwell, 2015/07/08 17:16:41]

Moved out of class definition and used continue as well.

     for (auto& candidate : *surfaces) {
       if (candidate.plane_z_order == -1) {
@@ -47,20 +73,34 @@ class OverlayCandidatesCast : public OverlayCandidatesOzone {
         // Compositor requires all overlay rectangles to have integer coords
         candidate.display_rect = gfx::ToEnclosedRect(candidate.display_rect);
 
-        chromecast::media::VideoPlane* video_plane =
-            chromecast::media::CastMediaShlib::GetVideoPlane();
-
         chromecast::RectF display_rect(
             candidate.display_rect.x(), candidate.display_rect.y(),
             candidate.display_rect.width(), candidate.display_rect.height());
-        video_plane->SetGeometry(
-            display_rect,
-            chromecast::media::VideoPlane::COORDINATE_TYPE_GRAPHICS_PLANE,
-            ConvertTransform(candidate.transform));
+
+        // Update video plane geometry + transform to match compositor quad.
+        // This must be done on media thread - and no point doing if it hasn't
+        // changed.
+        if (candidate.transform != transform_ ||
+            !ExactlyEqual(display_rect, display_rect_)) {
+          transform_ = candidate.transform;
+          display_rect_ = display_rect;
+
+          media_task_runner_->PostTask(
+              FROM_HERE,
+              base::Bind(
+                  &SetVideoPlaneGeometry, display_rect,
+                  chromecast::media::VideoPlane::COORDINATE_TYPE_GRAPHICS_PLANE,
+                  ConvertTransform(candidate.transform)));
+        }
         return;
       }
     }
   }
+
+ private:
+  scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
+  gfx::OverlayTransform transform_;
+  chromecast::RectF display_rect_;
 };
 
 }  // namespace