aura: Fix errors in root window size with display rotation

When a rotation is set, a very tiny error is introduced in the root
window transform due to use of trig functions in computing the transform.

This small error is later turned into a full pixel error in the root
window size.

This bug was just fixed for chromecast but also exists in TestScreen.

Use a common fix for both chromecast and TestScreen, and add a unit test
that ensures root window dimensions precisely match the display size when
a display rotation in set in the root as part of the root window transform.

Bug: 1019015

Change-Id: I930d09d5d079640b17040a721f4334c9cda0272a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1088126
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711374}

chromecast/graphics/cast_window_manager_aura.cc

@@ -38,32 +38,9 @@ namespace chromecast {
 namespace {
 
 gfx::Transform GetPrimaryDisplayRotationTransform() {
-  // NB: Using gfx::Transform::Rotate() introduces very small errors here
-  // which are later exacerbated by use of gfx::EnclosingRect() in
-  // WindowTreeHost::GetTransformedRootWindowBoundsInPixels().
-  const gfx::Transform rotate_90(0.f, -1.f, 0.f, 0.f,  //
-                                 1.f, 0.f, 0.f, 0.f,   //
-                                 0.f, 0.f, 1.f, 0.f,   //
-                                 0.f, 0.f, 0.f, 1.f);
-  const gfx::Transform rotate_180 = rotate_90 * rotate_90;
-  const gfx::Transform rotate_270 = rotate_180 * rotate_90;
-
-  gfx::Transform translation;
-  display::Display display(display::Screen::GetScreen()->GetPrimaryDisplay());
-  switch (display.rotation()) {
-    case display::Display::ROTATE_0:
-      return translation;
-    case display::Display::ROTATE_90:
-      translation.Translate(display.bounds().height(), 0);
-      return translation * rotate_90;
-    case display::Display::ROTATE_180:
-      translation.Translate(display.bounds().width(),
-                            display.bounds().height());
-      return translation * rotate_180;
-    case display::Display::ROTATE_270:
-      translation.Translate(0, display.bounds().width());
-      return translation * rotate_270;
-  }
+  display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  return display::Display::GetRotationTransform(display.rotation(),
+                                                gfx::SizeF(display.size()));
 }
 
 gfx::Rect GetPrimaryDisplayHostBounds() {

ui/aura/test/test_screen.cc

@@ -116,26 +116,9 @@ void TestScreen::SetWorkAreaInsets(const gfx::Insets& insets) {
 }
 
 gfx::Transform TestScreen::GetRotationTransform() const {
-  gfx::Transform rotate;
-  display::Display display(GetPrimaryDisplay());
-  switch (display.rotation()) {
-    case display::Display::ROTATE_0:
-      break;
-    case display::Display::ROTATE_90:
-      rotate.Translate(display.bounds().height(), 0);
-      rotate.Rotate(90);
-      break;
-    case display::Display::ROTATE_270:
-      rotate.Translate(0, display.bounds().width());
-      rotate.Rotate(270);
-      break;
-    case display::Display::ROTATE_180:
-      rotate.Translate(display.bounds().width(), display.bounds().height());
-      rotate.Rotate(180);
-      break;
-  }
-
-  return rotate;
+  display::Display display = GetPrimaryDisplay();
+  return display::Display::GetRotationTransform(display.rotation(),
+                                                gfx::SizeF(display.size()));
 }
 
 gfx::Transform TestScreen::GetUIScaleTransform() const {

ui/aura/window_tree_host_unittest.cc

@@ -42,6 +42,55 @@ TEST_F(WindowTreeHostTest, DPIWindowSize) {
   EXPECT_EQ(gfx::Rect(0, 1, 534, 401), root_window()->bounds());
 }
 
+TEST_F(WindowTreeHostTest,
+       ShouldHaveExactRootWindowBoundsWithDisplayRotation1xScale) {
+  test_screen()->SetDeviceScaleFactor(1.f);
+
+  host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
+  test_screen()->SetDisplayRotation(display::Display::ROTATE_0);
+  EXPECT_EQ(host()->GetBoundsInPixels(), gfx::Rect(0, 0, 400, 300));
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().rotation(),
+            display::Display::ROTATE_0);
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().GetSizeInPixel(),
+            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());
+
+  host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
+  test_screen()->SetDisplayRotation(display::Display::ROTATE_90);
+  EXPECT_EQ(host()->GetBoundsInPixels(), gfx::Rect(0, 0, 400, 300));
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().rotation(),
+            display::Display::ROTATE_90);
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().GetSizeInPixel(),
+            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());
+
+  host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
+  test_screen()->SetDisplayRotation(display::Display::ROTATE_180);
+  EXPECT_EQ(host()->GetBoundsInPixels(), gfx::Rect(0, 0, 400, 300));
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().rotation(),
+            display::Display::ROTATE_180);
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().GetSizeInPixel(),
+            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());
+
+  host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
+  test_screen()->SetDisplayRotation(display::Display::ROTATE_270);
+  EXPECT_EQ(host()->GetBoundsInPixels(), gfx::Rect(0, 0, 400, 300));
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().rotation(),
+            display::Display::ROTATE_270);
+  EXPECT_EQ(display::Screen::GetScreen()->GetPrimaryDisplay().GetSizeInPixel(),
+            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());
+}
+
 #if defined(OS_CHROMEOS)
 TEST_F(WindowTreeHostTest, HoldPointerMovesOnChildResizing) {
   aura::WindowEventDispatcher* dispatcher = host()->dispatcher();

ui/display/display.cc

@@ -17,6 +17,7 @@
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/size_conversions.h"
 #include "ui/gfx/icc_profile.h"
+#include "ui/gfx/transform.h"
 
 namespace display {
 namespace {
@@ -265,6 +266,34 @@ void Display::SetRotationAsDegree(int rotation) {
   }
 }
 
+// static
+gfx::Transform Display::GetRotationTransform(Rotation rotation,
+                                             const gfx::SizeF& size) {
+  // NB: Using gfx::Transform::Rotate() introduces very small errors here
+  // which are later exacerbated by use of gfx::EnclosingRect() in
+  // WindowTreeHost::GetTransformedRootWindowBoundsInPixels().
+  const gfx::Transform rotate_90(0.f, -1.f, 0.f, 0.f,  //
+                                 1.f, 0.f, 0.f, 0.f,   //
+                                 0.f, 0.f, 1.f, 0.f,   //
+                                 0.f, 0.f, 0.f, 1.f);
+  const gfx::Transform rotate_180 = rotate_90 * rotate_90;
+  const gfx::Transform rotate_270 = rotate_180 * rotate_90;
+  gfx::Transform translation;
+  switch (rotation) {
+    case display::Display::ROTATE_0:
+      return translation;
+    case display::Display::ROTATE_90:
+      translation.Translate(size.height(), 0);
+      return translation * rotate_90;
+    case display::Display::ROTATE_180:
+      translation.Translate(size.width(), size.height());
+      return translation * rotate_180;
+    case display::Display::ROTATE_270:
+      translation.Translate(0, size.width());
+      return translation * rotate_270;
+  }
+}
+
 gfx::Insets Display::GetWorkAreaInsets() const {
   return gfx::Insets(work_area_.y() - bounds_.y(), work_area_.x() - bounds_.x(),
                      bounds_.bottom() - work_area_.bottom(),

ui/display/display.h

@@ -13,6 +13,8 @@
 #include "ui/display/types/display_constants.h"
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size_f.h"
+#include "ui/gfx/transform.h"
 
 namespace display {
 
@@ -160,6 +162,11 @@ class DISPLAY_EXPORT Display final {
   int RotationAsDegree() const;
   void SetRotationAsDegree(int rotation);
 
+  // Returns an exact matrix representation of the transform that corrects for
+  // the display's rotation.
+  static gfx::Transform GetRotationTransform(Rotation rotation,
+                                             const gfx::SizeF& size);
+
   TouchSupport touch_support() const { return touch_support_; }
   void set_touch_support(TouchSupport support) { touch_support_ = support; }