aura: Clean up transformation confusion for root window bounds

Implementations of GetTransformedRootWindowBoundsInPixels() are
currently applying the inverse of the desired transformation: the root
window transform converts root window pixel coordinates to host pixel
coordinates. This function needs to do the reverse: it takes host pixel
coordinates, and returns root window pixel coordinates.

The reason this was not causing bugs is that in production Chrome OS &
Cast both override this function to throw out the origin of the returned
rect. Setting the origin to zero after transforming corrects for applying
the wrong transformation: the rest of the transformation is a 0, 90, 180,
or 270 degree rotation. Since 0 and 180 degree rotations are
self-inverses, and a 90 degree and a 270 degree rotation have identical
effect on sizes, keeping only the size ensured correct results.

Clean up this confusion by inverting the transformation applied by
GetTransformedRootWindowBoundsInPixels(). This means that the origin of
the transformed rect will automatically be correct (i.e., it will be
zero), except during configuration (because bounds & transform aren't
updated atomically).

We still force the origin to zero to avoid any behavioral change from
roundoff. There are some minor discrepancies related to that:

 - Chrome OS puts the window at origin, and rounds the size down
 - Cast puts the window at the origin, and rounds the size up
 - The default implementation allows arbitrary positioning, and
   takes an enclosing rect

These discrepancies are unchanged in this patch.

Bug: 1019015

Change-Id: I9bb40872be2fc76f04e63350a67965d44b71cc6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885633Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713223}

ash/display/root_window_transformers.cc

@@ -102,10 +102,10 @@ class AshRootWindowTransformer : public RootWindowTransformer {
     display::ManagedDisplayInfo info =
         display_manager->GetDisplayInfo(display.id());
     host_insets_ = info.GetOverscanInsetsInPixel();
-    root_window_bounds_transform_ =
+    gfx::Transform insets_and_rotation_transform =
         CreateInsetsTransform(host_insets_, display.device_scale_factor()) *
         CreateRootWindowRotationTransform(display);
-    transform_ = root_window_bounds_transform_;
+    transform_ = insets_and_rotation_transform;
     insets_and_scale_transform_ = CreateReverseRotatedInsetsTransform(
         info.GetLogicalActiveRotation(), host_insets_,
         display.device_scale_factor());
@@ -118,6 +118,8 @@ class AshRootWindowTransformer : public RootWindowTransformer {
     }
 
     CHECK(transform_.GetInverse(&invert_transform_));
+    CHECK(insets_and_rotation_transform.GetInverse(
+        &root_window_bounds_transform_));
 
     root_window_bounds_transform_.Scale(1.f / display.device_scale_factor(),
                                         1.f / display.device_scale_factor());
@@ -132,8 +134,9 @@ class AshRootWindowTransformer : public RootWindowTransformer {
     gfx::RectF new_bounds = gfx::RectF(gfx::SizeF(host_size));
     new_bounds.Inset(host_insets_);
     root_window_bounds_transform_.TransformRect(&new_bounds);
-    // Ignore the origin because RootWindow's insets are handled by
-    // the transform.
+
+    // Root window origin will be (0,0) except during bounds changes.
+    // Set to exactly zero to avoid rounding issues.
     // Floor the size because the bounds is no longer aligned to
     // backing pixel when |root_window_scale_| is specified
     // (850 height at 1.25 scale becomes 1062.5 for example.)
@@ -155,8 +158,21 @@ class AshRootWindowTransformer : public RootWindowTransformer {
   // |gfx::Transform::GetInverse|.
   gfx::Transform invert_transform_;
 
-  // The transform of the root window bounds. This is used to calculate
-  // the size of root window.
+  // The transform of the root window bounds. This is used to calculate the size
+  // of the root window. It is the composition of the following transforms
+  //   - inverse of insets. Insets position the content area within the display.
+  //   - inverse of rotation. Rotation changes orientation of the content area.
+  //   - inverse of device scale. Scaling up content shrinks the content area.
+  //
+  // Insets also shrink the content area but this happens prior to applying the
+  // transformation in GetRootWindowBounds().
+  //
+  // Magnification does not affect the window size. Content is clipped in this
+  // case, but the magnifier allows panning to reach clipped areas.
+  //
+  // The transforms are inverted because GetTransform() is the transform from
+  // root window coordinates to host coordinates, but this transform is used in
+  // the reverse direction (derives root window bounds from display bounds).
   gfx::Transform root_window_bounds_transform_;
 
   gfx::Insets host_insets_;

ash/host/transformer_helper.cc

@@ -44,7 +44,7 @@ class SimpleRootWindowTransformer : public RootWindowTransformer {
   gfx::Rect GetRootWindowBounds(const gfx::Size& host_size) const override {
     gfx::Rect bounds(host_size);
     gfx::RectF new_bounds(ui::ConvertRectToDIP(root_window_->layer(), bounds));
-    transform_.TransformRect(&new_bounds);
+    GetInverseTransform().TransformRect(&new_bounds);
     return gfx::Rect(gfx::ToFlooredSize(new_bounds.size()));
   }
 

chromecast/graphics/cast_window_tree_host_aura.cc

@@ -5,6 +5,7 @@
 #include "chromecast/graphics/cast_window_tree_host_aura.h"
 
 #include "ui/aura/null_window_targeter.h"
+#include "ui/gfx/geometry/size_conversions.h"
 #include "ui/platform_window/platform_window_init_properties.h"
 
 namespace chromecast {
@@ -33,11 +34,13 @@ void CastWindowTreeHostAura::DispatchEvent(ui::Event* event) {
 }
 
 gfx::Rect CastWindowTreeHostAura::GetTransformedRootWindowBoundsInPixels(
-    const gfx::Size& host_size_in_pixels) const {
-  gfx::RectF new_bounds(WindowTreeHost::GetTransformedRootWindowBoundsInPixels(
-      host_size_in_pixels));
-  new_bounds.set_origin(gfx::PointF());
-  return gfx::ToEnclosingRect(new_bounds);
+    const gfx::Size& size_in_pixels) const {
+  gfx::RectF new_bounds = gfx::RectF(gfx::Rect(size_in_pixels));
+  GetInverseRootTransform().TransformRect(&new_bounds);
+
+  // Root window origin will be (0,0) except during bounds changes.
+  // Set to exactly zero to avoid rounding issues.
+  return gfx::Rect(gfx::ToCeiledSize(new_bounds.size()));
 }
 
 }  // namespace chromecast

ui/aura/test/test_screen.cc

@@ -95,7 +95,7 @@ void TestScreen::SetDisplayRotation(display::Display::Rotation rotation) {
   display.set_rotation(rotation);
   display.SetScaleAndBounds(display.device_scale_factor(), new_bounds);
   display_list().UpdateDisplay(display);
-  host_->SetRootTransform(GetRotationTransform() * GetUIScaleTransform());
+  host_->SetRootTransform(GetUIScaleTransform() * GetRotationTransform());
 }
 
 void TestScreen::SetUIScale(float ui_scale) {
@@ -106,7 +106,7 @@ void TestScreen::SetUIScale(float ui_scale) {
       gfx::ScaleRect(gfx::RectF(bounds_in_pixel), 1.0f / ui_scale));
   display.SetScaleAndBounds(display.device_scale_factor(), new_bounds);
   display_list().UpdateDisplay(display);
-  host_->SetRootTransform(GetRotationTransform() * GetUIScaleTransform());
+  host_->SetRootTransform(GetUIScaleTransform() * GetRotationTransform());
 }
 
 void TestScreen::SetWorkAreaInsets(const gfx::Insets& insets) {

ui/aura/window_tree_host.cc

@@ -539,10 +539,8 @@ void WindowTreeHost::OnDisplayMetricsChanged(const display::Display& display,
 
 gfx::Rect WindowTreeHost::GetTransformedRootWindowBoundsInPixels(
     const gfx::Size& size_in_pixels) const {
-  gfx::Rect bounds(size_in_pixels);
-  gfx::RectF new_bounds =
-      gfx::ScaleRect(gfx::RectF(bounds), 1.0f / device_scale_factor_);
-  window()->layer()->transform().TransformRect(&new_bounds);
+  gfx::RectF new_bounds = gfx::RectF(gfx::Rect(size_in_pixels));
+  GetInverseRootTransform().TransformRect(&new_bounds);
   return gfx::ToEnclosingRect(new_bounds);
 }
 

ui/aura/window_tree_host_unittest.cc

@@ -34,7 +34,7 @@ TEST_F(WindowTreeHostTest, DPIWindowSize) {
   EXPECT_EQ(gfx::Rect(0, 0, 534, 400), root_window()->bounds());
 
   gfx::Transform transform;
-  transform.Translate(0, 1.1f);
+  transform.Translate(0, -1.1f);
   host()->SetRootTransform(transform);
   EXPECT_EQ(gfx::Rect(0, 1, 534, 401), root_window()->bounds());
 
@@ -55,7 +55,7 @@ TEST_F(WindowTreeHostTest,
             gfx::Size(400, 300));
   EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().bounds(),
             gfx::Rect(0, 0, 400, 300));
-  EXPECT_EQ(gfx::Size(400, 300), host()->window()->bounds().size());
+  EXPECT_EQ(gfx::Rect(400, 300), host()->window()->bounds());
 
   host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
   test_screen()->SetDisplayRotation(display::Display::ROTATE_90);
@@ -66,7 +66,7 @@ TEST_F(WindowTreeHostTest,
             gfx::Size(300, 400));
   EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().bounds(),
             gfx::Rect(0, 0, 300, 400));
-  EXPECT_EQ(gfx::Size(300, 400), host()->window()->bounds().size());
+  EXPECT_EQ(gfx::Rect(300, 400), host()->window()->bounds());
 
   host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
   test_screen()->SetDisplayRotation(display::Display::ROTATE_180);
@@ -77,7 +77,7 @@ TEST_F(WindowTreeHostTest,
             gfx::Size(400, 300));
   EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().bounds(),
             gfx::Rect(0, 0, 400, 300));
-  EXPECT_EQ(gfx::Size(400, 300), host()->window()->bounds().size());
+  EXPECT_EQ(gfx::Rect(400, 300), host()->window()->bounds());
 
   host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
   test_screen()->SetDisplayRotation(display::Display::ROTATE_270);
@@ -88,7 +88,7 @@ TEST_F(WindowTreeHostTest,
             gfx::Size(300, 400));
   EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().bounds(),
             gfx::Rect(0, 0, 300, 400));
-  EXPECT_EQ(gfx::Size(300, 400), host()->window()->bounds().size());
+  EXPECT_EQ(gfx::Rect(300, 400), host()->window()->bounds());
 }
 
 #if defined(OS_CHROMEOS)

ui/aura/window_unittest.cc

@@ -53,6 +53,7 @@
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/vector2d.h"
+#include "ui/gfx/overlay_transform_utils.h"
 #include "ui/gfx/skia_util.h"
 
 DEFINE_UI_CLASS_PROPERTY_TYPE(const char*)
@@ -441,7 +442,8 @@ TEST_F(WindowTest, WindowEmbeddingClientHasValidLocalSurfaceId) {
 TEST_F(WindowTest, MoveCursorToWithTransformRootWindow) {
   gfx::Transform transform;
   transform.Translate(100.0, 100.0);
-  transform.Rotate(90.0);
+  transform = transform * OverlayTransformToTransform(
+                              gfx::OVERLAY_TRANSFORM_ROTATE_90, gfx::Size());
   transform.Scale(2.0, 5.0);
   host()->SetRootTransform(transform);
   host()->MoveCursorToLocationInDIP(gfx::Point(10, 10));
@@ -473,7 +475,8 @@ TEST_F(WindowTest, MoveCursorToWithTransformWindow) {
             display::Screen::GetScreen()->GetCursorScreenPoint().ToString());
 
   gfx::Transform transform3;
-  transform3.Rotate(90.0);
+  transform3 = transform3 * OverlayTransformToTransform(
+                                gfx::OVERLAY_TRANSFORM_ROTATE_90, gfx::Size());
   w1->SetTransform(transform3);
   w1->MoveCursorTo(gfx::Point(5, 5));
   EXPECT_EQ("5,15",
@@ -481,7 +484,8 @@ TEST_F(WindowTest, MoveCursorToWithTransformWindow) {
 
   gfx::Transform transform4;
   transform4.Translate(100.0, 100.0);
-  transform4.Rotate(90.0);
+  transform4 = transform4 * OverlayTransformToTransform(
+                                gfx::OVERLAY_TRANSFORM_ROTATE_90, gfx::Size());
   transform4.Scale(2.0, 5.0);
   w1->SetTransform(transform4);
   w1->MoveCursorTo(gfx::Point(10, 10));
@@ -505,7 +509,9 @@ TEST_F(WindowTest, MoveCursorToWithComplexTransform) {
   // The root window expects transforms that produce integer rects.
   gfx::Transform root_transform;
   root_transform.Translate(60.0, 70.0);
-  root_transform.Rotate(-90.0);
+  root_transform =
+      root_transform * OverlayTransformToTransform(
+                           gfx::OVERLAY_TRANSFORM_ROTATE_270, gfx::Size());
   root_transform.Translate(-50.0, -50.0);
   root_transform.Scale(2.0, 3.0);
 
@@ -1654,10 +1660,8 @@ TEST_F(WindowTest, Transform) {
                                  .bounds());
 
   // Rotate it clock-wise 90 degrees.
-  gfx::Transform transform;
-  transform.Translate(size.height(), 0);
-  transform.Rotate(90.0);
-  host()->SetRootTransform(transform);
+  host()->SetRootTransform(
+      OverlayTransformToTransform(gfx::OVERLAY_TRANSFORM_ROTATE_90, size));
 
   // The size should be the transformed size.
   gfx::Size transformed_size(size.height(), size.width());
@@ -1682,10 +1686,8 @@ TEST_F(WindowTest, TransformGesture) {
       delegate.get(), -1234, gfx::Rect(0, 0, 20, 20), root_window()));
 
   // Rotate the root-window clock-wise 90 degrees.
-  gfx::Transform transform;
-  transform.Translate(size.height(), 0.0);
-  transform.Rotate(90.0);
-  host()->SetRootTransform(transform);
+  host()->SetRootTransform(
+      OverlayTransformToTransform(gfx::OVERLAY_TRANSFORM_ROTATE_90, size));
 
   ui::TouchEvent press(
       ui::ET_TOUCH_PRESSED, gfx::Point(size.height() - 10, 10), getTime(),

ui/snapshot/snapshot_aura_unittest.cc

@@ -318,7 +318,7 @@ TEST_P(SnapshotAuraTest, RotateAndUIScale) {
   test_screen()->SetUIScale(kUIScale);
   test_screen()->SetDisplayRotation(display::Display::ROTATE_90);
 
-  gfx::Rect test_bounds(100, 100, 300, 200);
+  gfx::Rect test_bounds(100, 100, 200, 300);
   SetupTestWindow(test_bounds);
   WaitForDraw();
 
@@ -344,7 +344,7 @@ TEST_P(SnapshotAuraTest, RotateAndUIScaleAndScaleFactor) {
   test_screen()->SetUIScale(kUIScale);
   test_screen()->SetDisplayRotation(display::Display::ROTATE_90);
 
-  gfx::Rect test_bounds(20, 30, 150, 100);
+  gfx::Rect test_bounds(20, 30, 100, 150);
   SetupTestWindow(test_bounds);
   WaitForDraw();