[Chromecast] Clear cached video plane geometry parameters and don't run

SetGeometry when graphics plane resolution changes.

Reasons for this:
* When switching to an app with a different graphics plane
  resolution, the old video plane geometry parameters are now
  invalid since they were relative to the previous graphics
  plane.
* SetGeometry should not always be run when the graphics
  plane resolution changes because some apps (like Backdrop)
  don't have a video element.

BUG=internal b/27554486
TEST=All tests run on dragonfly
     * Switch Youtube -> Backdrop. No SetGeometry calls are made
       after switching to Backdrop.
     * Switch Youtube -> Netflix. SetGeometry calls are made for
       Netflix playback.
     * Revoke resources while playing Youtube video. Then grant
       resources and playback same Youtube video. SetGeometry
       call is made when Youtube video is played again.

Review URL: https://codereview.chromium.org/1838863004

Cr-Commit-Position: refs/heads/master@{#383839}

chromecast/media/base/video_plane_controller.cc

@@ -142,10 +142,10 @@ class VideoPlaneController::RateLimitedSetVideoPlaneGeometry
 VideoPlaneController::VideoPlaneController(
     scoped_refptr<base::SingleThreadTaskRunner> media_task_runner)
     : is_paused_(false),
-      have_output_res_(false),
-      have_graphics_res_(false),
-      output_res_(0, 0),
-      graphics_res_(0, 0),
+      have_screen_res_(false),
+      have_graphics_plane_res_(false),
+      screen_res_(0, 0),
+      graphics_plane_res_(0, 0),
       have_video_plane_geometry_(false),
       video_plane_display_rect_(0, 0),
       video_plane_transform_(VideoPlane::TRANSFORM_NONE),
@@ -155,10 +155,6 @@ VideoPlaneController::VideoPlaneController(
 
 VideoPlaneController::~VideoPlaneController() {}
 
-// TODO(esum): SetGeometry, SetDeviceResolution, and SetGraphicsPlaneResolution
-// follow the same pattern (copy/paste). Currently it's not worth modularizing
-// since there are only 3 fields. If more fields are needed in the future,
-// consider making a generic method to implement this pattern.
 void VideoPlaneController::SetGeometry(const RectF& display_rect,
                                        VideoPlane::Transform transform) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -182,19 +178,19 @@ void VideoPlaneController::SetGeometry(const RectF& display_rect,
   MaybeRunSetGeometry();
 }
 
-void VideoPlaneController::SetDeviceResolution(const Size& resolution) {
+void VideoPlaneController::SetScreenResolution(const Size& resolution) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(ResolutionSizeValid(resolution));
-  if (have_output_res_ && SizeEqual(resolution, output_res_)) {
-    VLOG(2) << "No change found in output resolution.";
+  if (have_screen_res_ && SizeEqual(resolution, screen_res_)) {
+    VLOG(2) << "No change found in screen resolution.";
     return;
   }
 
-  VLOG(1) << "New output resolution " << resolution.width << "x"
+  VLOG(1) << "New screen resolution " << resolution.width << "x"
           << resolution.height;
 
-  have_output_res_ = true;
-  output_res_ = resolution;
+  have_screen_res_ = true;
+  screen_res_ = resolution;
 
   MaybeRunSetGeometry();
 }
@@ -202,18 +198,22 @@ void VideoPlaneController::SetDeviceResolution(const Size& resolution) {
 void VideoPlaneController::SetGraphicsPlaneResolution(const Size& resolution) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(ResolutionSizeValid(resolution));
-  if (have_graphics_res_ && SizeEqual(resolution, graphics_res_)) {
-    VLOG(2) << "No change found in graphics resolution.";
+  if (have_graphics_plane_res_ && SizeEqual(resolution, graphics_plane_res_)) {
+    VLOG(2) << "No change found in graphics plane resolution.";
     return;
   }
 
-  VLOG(1) << "New graphics resolution " << resolution.width << "x"
+  VLOG(1) << "New graphics plane resolution " << resolution.width << "x"
           << resolution.height;
 
-  have_graphics_res_ = true;
-  graphics_res_ = resolution;
+  have_graphics_plane_res_ = true;
+  graphics_plane_res_ = resolution;
 
-  MaybeRunSetGeometry();
+  // Any cached video plane geometry parameters are no longer valid since they
+  // were relative to the PREVIOUS graphics plane resolution. Thus, the cached
+  // parameters are cleared, and it's the caller's responsibility to call
+  // SetGeometry() with arguments relative to the NEW graphics plane if needed.
+  ClearVideoPlaneGeometry();
 }
 
 void VideoPlaneController::Pause() {
@@ -226,7 +226,7 @@ void VideoPlaneController::Resume() {
   DCHECK(thread_checker_.CalledOnValidThread());
   VLOG(1) << "Resuming controller. VideoPlane SetGeometry calls are active.";
   is_paused_ = false;
-  MaybeRunSetGeometry();
+  ClearVideoPlaneGeometry();
 }
 
 bool VideoPlaneController::is_paused() const {
@@ -235,7 +235,6 @@ bool VideoPlaneController::is_paused() const {
 }
 
 void VideoPlaneController::MaybeRunSetGeometry() {
-  DCHECK(thread_checker_.CalledOnValidThread());
   if (is_paused_) {
     VLOG(2) << "All VideoPlane SetGeometry calls are paused. Ignoring request.";
     return;
@@ -246,13 +245,15 @@ void VideoPlaneController::MaybeRunSetGeometry() {
     return;
   }
 
-  DCHECK(graphics_res_.width != 0 && graphics_res_.height != 0);
+  DCHECK(graphics_plane_res_.width != 0 && graphics_plane_res_.height != 0);
 
   RectF scaled_rect = video_plane_display_rect_;
-  if (graphics_res_.width != output_res_.width ||
-      graphics_res_.height != output_res_.height) {
-    float sx = static_cast<float>(output_res_.width) / graphics_res_.width;
-    float sy = static_cast<float>(output_res_.height) / graphics_res_.height;
+  if (graphics_plane_res_.width != screen_res_.width ||
+      graphics_plane_res_.height != screen_res_.height) {
+    float sx =
+        static_cast<float>(screen_res_.width) / graphics_plane_res_.width;
+    float sy =
+        static_cast<float>(screen_res_.height) / graphics_plane_res_.height;
     scaled_rect.x *= sx;
     scaled_rect.y *= sy;
     scaled_rect.width *= sx;
@@ -266,8 +267,14 @@ void VideoPlaneController::MaybeRunSetGeometry() {
 }
 
 bool VideoPlaneController::HaveDataForSetGeometry() const {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return have_output_res_ && have_graphics_res_ && have_video_plane_geometry_;
+  return have_screen_res_ && have_graphics_plane_res_ &&
+         have_video_plane_geometry_;
+}
+
+void VideoPlaneController::ClearVideoPlaneGeometry() {
+  have_video_plane_geometry_ = false;
+  video_plane_display_rect_ = RectF(0, 0);
+  video_plane_transform_ = VideoPlane::TRANSFORM_NONE;
 }
 
 }  // namespace media

chromecast/media/base/video_plane_controller.h

@@ -23,41 +23,34 @@ namespace media {
 // should use this over VideoPlane::SetGeometry.  Reasons for this:
 // * provides conversion between graphics plane coordinates and screen
 //   resolution coordinates
-// * updates VideoPlane when graphics plane or screen resolution changes
+// * updates VideoPlane when screen resolution changes
 // * handles threading correctly (posting SetGeometry to media thread).
 // * coalesces multiple calls in short space of time to prevent flooding the
 //   media thread with SetGeometry calls (which are expensive on many
 //   platforms).
-// The class collects/caches the data it needs before it can start operating.
-// This means SetGeometry, SetDeviceResolution, and SetGraphicsPlaneResolution
-// need to be called at least once in any order before the class starts making
-// calls to VideoPlane::SetGeometry. All calls to these methods beforehand just
-// set/update the cached parameters.
-// All calls to public methods should be from the same thread.
+// All public methods should be called from the same thread that the class was
+// constructed on.
 class VideoPlaneController {
  public:
   explicit VideoPlaneController(
       scoped_refptr<base::SingleThreadTaskRunner> media_task_runner);
   ~VideoPlaneController();
-  // Sets the video plane geometry (forwards to VideoPlane::SetGeometry)
-  // in *graphics plane coordinates*.
-  //  * This should be called on UI thread (hopping to media thread is handled
-  //    internally).
-  // If there is no change to video plane parameters from the last call to this
-  // method, it is a no-op.
+  // Sets the video plane geometry in *graphics plane coordinates*. If there is
+  // no change to video plane parameters from the last call to this method, it
+  // is a no-op.
   void SetGeometry(const RectF& display_rect, VideoPlane::Transform transform);
 
-  // Sets physical screen resolution.  This must be called at least once when
+  // Sets physical screen resolution. This must be called at least once when
   // the final output resolution (HDMI signal or panel resolution) is known,
-  // then later when it changes. If there is no change to the device resolution
+  // then later when it changes. If there is no change to the screen resolution
   // from the last call to this method, it is a no-op.
-  void SetDeviceResolution(const Size& resolution);
+  void SetScreenResolution(const Size& resolution);
 
-  // Sets graphics hardware plane resolution.  This must be called at least once
-  // when the hardware graphics plane resolution (same resolution as
-  // gfx::Screen) is known, then later when it changes. If there is no change to
-  // the graphics plane resolution from the last call to this method, it is a
-  // no-op.
+  // Sets graphics hardware plane resolution, and clears any cached video plane
+  // geometry parameters. This must be called at least once when the hardware
+  // graphics plane resolution (same resolution as gfx::Screen) is known, then
+  // later when it changes. If there is no change to the graphics plane
+  // resolution from the last call to this method, it is a no-op.
   void SetGraphicsPlaneResolution(const Size& resolution);
 
   // After Pause is called, no further calls to VideoPlane::SetGeometry will be
@@ -68,9 +61,8 @@ class VideoPlaneController {
   // media thread. When this returns, the caller needs to know that absolutely
   // no more SetGeometry calls will be made.
   void Pause();
-  // Makes class active again. Also resets the video plane by posting a call to
-  // VideoPlane::SetGeometry with most recent resolution and geometry parameters
-  // (assuming they are all set). Safe to call multiple times.
+  // Makes class active again, and clears any cached video plane geometry
+  // parameters. Safe to call multiple times.
   void Resume();
   bool is_paused() const;
 
@@ -85,14 +77,16 @@ class VideoPlaneController {
   // Checks if all data has been collected to make calls to
   // VideoPlane::SetGeometry.
   bool HaveDataForSetGeometry() const;
+  // Clears any cached video plane geometry parameters.
+  void ClearVideoPlaneGeometry();
 
   bool is_paused_;
 
   // Current resolutions
-  bool have_output_res_;
-  bool have_graphics_res_;
-  Size output_res_;
-  Size graphics_res_;
+  bool have_screen_res_;
+  bool have_graphics_plane_res_;
+  Size screen_res_;
+  Size graphics_plane_res_;
 
   // Saved video plane parameters (in graphics plane coordinates)
   // for use when screen resolution changes.