Fix overscan calibration UI

* Take rotation into account
* Fix when display zoom is applised.
  The overscan is dp but w/o display zoom. To convert to DP in UI coordinates,
  it first have to be converted to pixel then divided by the scale factor.

Bug: 1021292
Test: unittests + manual
Change-Id: I82785b74013c82e9eb20a7714510e62a421337db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896374Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712763}

ash/BUILD.gn

@@ -1711,6 +1711,7 @@ test("ash_unittests") {
     "display/extended_mouse_warp_controller_unittest.cc",
     "display/mirror_window_controller_unittest.cc",
     "display/mouse_cursor_event_filter_unittest.cc",
+    "display/overscan_calibrator_unittest.cc",
     "display/persistent_window_controller_unittest.cc",
     "display/projecting_observer_unittest.cc",
     "display/resolution_notification_controller_unittest.cc",

ash/display/cros_display_config.h

@@ -60,6 +60,8 @@ class ASH_EXPORT CrosDisplayConfig : public mojom::CrosDisplayConfigController {
 
  private:
   class ObserverImpl;
+  friend class OverscanCalibratorTest;
+
   OverscanCalibrator* GetOverscanCalibrator(const std::string& id);
 
   std::unique_ptr<ObserverImpl> observer_impl_;

ash/display/overscan_calibrator.cc

@@ -19,6 +19,7 @@
 #include "ui/compositor/paint_recorder.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/managed_display_info.h"
+#include "ui/display/screen.h"
 #include "ui/gfx/canvas.h"
 
 namespace ash {
@@ -68,35 +69,64 @@ void DrawTriangle(int x_offset,
   canvas->DrawPath(path, border_flags);
 }
 
+gfx::Insets RotateInsets(display::Display::Rotation rotation,
+                         gfx::Insets&& insets) {
+  switch (rotation) {
+    case display::Display::ROTATE_0:
+      return insets;
+    case display::Display::ROTATE_90:
+      return gfx::Insets(insets.right(), insets.top(), insets.left(),
+                         insets.bottom());
+    case display::Display::ROTATE_180:
+      return gfx::Insets(insets.bottom(), insets.right(), insets.top(),
+                         insets.left());
+    case display::Display::ROTATE_270:
+      return gfx::Insets(insets.left(), insets.bottom(), insets.right(),
+                         insets.top());
+  }
+  NOTREACHED();
+  return std::move(insets);
+}
+
+gfx::Insets ConvertToDisplay(const display::Display& display,
+                             const gfx::Insets& insets) {
+  display::ManagedDisplayInfo info =
+      ash::Shell::Get()->display_manager()->GetDisplayInfo(display.id());
+  return RotateInsets(
+      display.rotation(),
+      insets.Scale(info.device_scale_factor() / display.device_scale_factor()));
+}
+
+gfx::Insets ConvertToHost(const display::Display& display,
+                          const gfx::Insets& insets) {
+  display::ManagedDisplayInfo info =
+      ash::Shell::Get()->display_manager()->GetDisplayInfo(display.id());
+  display::Display::Rotation inverted_rotation =
+      static_cast<display::Display::Rotation>(
+          (4 - static_cast<int>(display.rotation())) % 4);
+  return RotateInsets(
+      inverted_rotation,
+      insets.Scale(display.device_scale_factor() / info.device_scale_factor()));
+}
+
 }  // namespace
 
 OverscanCalibrator::OverscanCalibrator(const display::Display& target_display,
                                        const gfx::Insets& initial_insets)
     : display_(target_display),
-      insets_(initial_insets),
+      insets_(ConvertToDisplay(display_, initial_insets)),
       initial_insets_(initial_insets),
       committed_(false) {
   // Undo the overscan calibration temporarily so that the user can see
   // dark boundary and current overscan region.
   ash::Shell::Get()->window_tree_host_manager()->SetOverscanInsets(
       display_.id(), gfx::Insets());
-
-  display::ManagedDisplayInfo info =
-      ash::Shell::Get()->display_manager()->GetDisplayInfo(display_.id());
-
-  aura::Window* root = ash::Shell::GetRootWindowForDisplayId(display_.id());
-  ui::Layer* parent_layer =
-      ash::Shell::GetContainer(root, ash::kShellWindowId_OverlayContainer)
-          ->layer();
-
-  calibration_layer_.reset(new ui::Layer());
-  calibration_layer_->SetOpacity(0.5f);
-  calibration_layer_->SetBounds(parent_layer->bounds());
-  calibration_layer_->set_delegate(this);
-  parent_layer->Add(calibration_layer_.get());
+  UpdateUILayer();
+  display::Screen::GetScreen()->AddObserver(this);
 }
 
 OverscanCalibrator::~OverscanCalibrator() {
+  display::Screen::GetScreen()->RemoveObserver(this);
   // Overscan calibration has finished without commit, so the display has to
   // be the original offset.
   if (!committed_) {
@@ -107,12 +137,12 @@ OverscanCalibrator::~OverscanCalibrator() {
 
 void OverscanCalibrator::Commit() {
   ash::Shell::Get()->window_tree_host_manager()->SetOverscanInsets(
-      display_.id(), insets_);
+      display_.id(), ConvertToHost(display_, insets_));
   committed_ = true;
 }
 
 void OverscanCalibrator::Reset() {
-  insets_ = initial_insets_;
+  insets_ = ConvertToDisplay(display_, initial_insets_);
   calibration_layer_->SchedulePaint(calibration_layer_->bounds());
 }
 
@@ -144,9 +174,31 @@ void OverscanCalibrator::OnPaintLayer(const ui::PaintContext& context) {
 
 void OverscanCalibrator::OnDeviceScaleFactorChanged(
     float old_device_scale_factor,
-    float new_device_scale_factor) {
-  // TODO(mukai): Cancel the overscan calibration when the device
-  // configuration has changed.
+    float new_device_scale_factor) {}
+
+void OverscanCalibrator::OnDisplayMetricsChanged(
+    const display::Display& display,
+    uint32_t changed_metrics) {
+  if (display_.id() != display.id() || committed_)
+    return;
+  display_ = display;
+  UpdateUILayer();
+  Reset();
+}
+
+void OverscanCalibrator::UpdateUILayer() {
+  display::ManagedDisplayInfo info =
+      ash::Shell::Get()->display_manager()->GetDisplayInfo(display_.id());
+
+  aura::Window* root = ash::Shell::GetRootWindowForDisplayId(display_.id());
+  ui::Layer* parent_layer =
+      ash::Shell::GetContainer(root, ash::kShellWindowId_OverlayContainer)
+          ->layer();
+  calibration_layer_.reset(new ui::Layer());
+  calibration_layer_->SetOpacity(0.5f);
+  calibration_layer_->SetBounds(parent_layer->bounds());
+  calibration_layer_->set_delegate(this);
+  parent_layer->Add(calibration_layer_.get());
 }
 
 }  // namespace ash

ash/display/overscan_calibrator.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "ui/compositor/layer_delegate.h"
 #include "ui/display/display.h"
+#include "ui/display/display_observer.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/rect.h"
 
@@ -22,7 +23,8 @@ namespace ash {
 
 // This is used to show the visible feedback to the user's operations for
 // calibrating display overscan settings.
-class ASH_EXPORT OverscanCalibrator : public ui::LayerDelegate {
+class ASH_EXPORT OverscanCalibrator : public ui::LayerDelegate,
+                                      public display::DisplayObserver {
  public:
   OverscanCalibrator(const display::Display& target_display,
                      const gfx::Insets& initial_insets);
@@ -41,14 +43,20 @@ class ASH_EXPORT OverscanCalibrator : public ui::LayerDelegate {
 
   const gfx::Insets& insets() const { return insets_; }
 
- private:
   // ui::LayerDelegate overrides:
   void OnPaintLayer(const ui::PaintContext& context) override;
   void OnDeviceScaleFactorChanged(float old_device_scale_factor,
                                   float new_device_scale_factor) override;
 
+  // DisplayObserver:
+  void OnDisplayMetricsChanged(const display::Display& display,
+                               uint32_t changed_metrics) override;
+
+ private:
+  void UpdateUILayer();
+
   // The target display.
-  const display::Display display_;
+  display::Display display_;
 
   // The current insets.
   gfx::Insets insets_;

ash/display/overscan_calibrator_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/display/overscan_calibrator.h"
+#include "ash/display/cros_display_config.h"
+
+#include "ash/shell.h"
+#include "ash/test/ash_test_base.h"
+#include "base/bind_helpers.h"
+#include "base/strings/stringprintf.h"
+#include "ui/display/manager/display_manager.h"
+#include "ui/display/manager/managed_display_info.h"
+
+namespace ash {
+
+class OverscanCalibratorTest : public AshTestBase {
+ public:
+  OverscanCalibratorTest() = default;
+  ~OverscanCalibratorTest() override = default;
+  OverscanCalibratorTest(OverscanCalibratorTest&) = delete;
+  OverscanCalibratorTest& operator=(const OverscanCalibratorTest&) = delete;
+
+  OverscanCalibrator* StartCalibration(const std::string& id) {
+    Shell::Get()->cros_display_config()->OverscanCalibration(
+        id, mojom::DisplayConfigOperation::kStart,
+        gfx::Insets(0, 0, 0, 0) /* not used */, base::DoNothing());
+    return Shell::Get()->cros_display_config()->GetOverscanCalibrator(id);
+  }
+};
+
+TEST_F(OverscanCalibratorTest, Rotation) {
+  auto* display_manager = Shell::Get()->display_manager();
+
+  int64_t display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
+  std::string id_str = base::StringPrintf("%ld", display_id);
+
+  auto* calibrator = StartCalibration(id_str);
+  calibrator->UpdateInsets(gfx::Insets(100, 5, 10, 15));
+  calibrator->Commit();
+  display::ManagedDisplayInfo info =
+      display_manager->GetDisplayInfo(display_id);
+  EXPECT_EQ(gfx::Insets(100, 5, 10, 15), info.overscan_insets_in_dip());
+
+  display_manager->SetDisplayRotation(display_id,
+                                      display::Display::Rotation::ROTATE_90,
+                                      display::Display::RotationSource::USER);
+  EXPECT_EQ(gfx::Size(490, 780),
+            display::Screen::GetScreen()->GetPrimaryDisplay().size());
+
+  calibrator = StartCalibration(id_str);
+  // The insets will be rotated and applied in the host coordinates.
+  gfx::Insets insets = calibrator->insets();
+  insets.set_left(105);
+  insets.set_top(0);
+  calibrator->UpdateInsets(insets);
+  calibrator->Commit();
+
+  info = display_manager->GetDisplayInfo(display_id);
+  EXPECT_EQ(gfx::Insets(105, 5, 10, 0), info.overscan_insets_in_dip());
+}
+
+}  // namespace ash

ui/display/manager/managed_display_info.h

@@ -334,6 +334,7 @@ class DISPLAY_MANAGER_EXPORT ManagedDisplayInfo {
   // The size of the display in use. The size can be different from the size
   // of |bounds_in_native_| if the display has overscan insets and/or rotation.
   gfx::Size size_in_pixel_;
+  // TODO(oshima): Change this to store pixel.
   gfx::Insets overscan_insets_in_dip_;
 
   // The zoom level currently applied to the display. This value is appended