[go: nahoru, domu]

[Magnifier] Fix bug where magnified viewport couldn't reach edges of screen

In continuous mouse following mode, the viewport couldn't reach the edges
of the screen, and this problem increased with zoom levels. This fixes
the bug by realizing that the cursor could never reach the bottom or right
of the screen but always remained one DIP within bounds so that it could
be drawn.

Also cleans up some unused and redundant parts of
fullscreen_magnifier_controller.

could not reach bottom / right edges of the screen.

Bug: b:262037897
Test: New unittest added, would have failed without new code
Change-Id: Id7b4863daeb8c186868818576e9ada2b849b07b4
AX-Relnotes: Fixes bug where continuous mouse following mode
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4097121
Reviewed-by: Josiah Krutz <josiahk@google.com>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1082470}
diff --git a/ash/accessibility/magnifier/fullscreen_magnifier_controller.cc b/ash/accessibility/magnifier/fullscreen_magnifier_controller.cc
index 3e2ae5f4..64148b3 100644
--- a/ash/accessibility/magnifier/fullscreen_magnifier_controller.cc
+++ b/ash/accessibility/magnifier/fullscreen_magnifier_controller.cc
@@ -672,17 +672,42 @@
     // calculates where the center point of the magnified region should be,
     // such that where the cursor is located in the magnified region corresponds
     // in proportion to where the cursor is located on the screen overall.
+
+    // Screen size.
     const gfx::Size host_size_in_dip = GetHostSizeDIP();
-    const gfx::SizeF window_size_in_dip = GetWindowRectDIP(scale_).size();
+
+    // Mouse position.
     const float x = location_in_dip.x();
     const float y = location_in_dip.y();
 
-    const int center_point_in_dip_x =
-        x - x * window_size_in_dip.width() / host_size_in_dip.width() +
-        window_size_in_dip.width() / 2;
-    const int center_point_in_dip_y =
-        y - y * window_size_in_dip.height() / host_size_in_dip.height() +
-        window_size_in_dip.height() / 2;
+    // Viewport dimensions for calculation, increased by variable padding:
+    // The cursor can never reach the bottom or right of the screen, it's always
+    // at least one DIP away so that you can see it. (Note the cursor can reach
+    // the top left at (0, 0)). Calculate the viewport size, adding some scaled
+    // viewport padding as we move down and right so that the padding 0 in the
+    // top/left and greater in the bottom right to account for the cursor not
+    // being able to access the bottom corner.
+    const float height =
+        host_size_in_dip.height() / scale_ + 4 * y / host_size_in_dip.height();
+    const float width =
+        host_size_in_dip.width() / scale_ + 4 * x / host_size_in_dip.width();
+
+    // The viewport center point is the mouse center point, minus the scaled
+    // mouse center point to get to the viewport left/top edge, plus half
+    // the viewport size.
+    // In the example below, the host size is 12 units in width, the
+    // mouse point x is at 7, and the viewport width is 3 (scale is 4.0).
+    // The center_point_in_dip_x should be 6, with some integer rounding.
+    // 6 = int(7 - (7 / 4.0) + (3 / 2.0))
+    //  ____________
+    // |            | host
+    // |     ___    |
+    // |    |  *|   |  <-- mouse x = 7, viewport width = 3
+    // |    |___|   |
+    // |____________|
+    //  012345678901   <-- Indexes
+    const int center_point_in_dip_x = x - x / scale_ + width / 2.0;
+    const int center_point_in_dip_y = y - y / scale_ + height / 2.0;
     center_point_in_dip = {center_point_in_dip_x, center_point_in_dip_y};
   }
 
@@ -691,7 +716,7 @@
       keyboard::KeyboardUIController::Get()->IsKeyboardVisible();
 
   MoveMagnifierWindowFollowPoint(center_point_in_dip, x_margin, y_margin,
-                                 x_margin, y_margin, reduce_bottom_margin);
+                                 reduce_bottom_margin);
 }
 
 void FullscreenMagnifierController::AfterAnimationMoveCursorTo(
@@ -716,7 +741,7 @@
 }
 
 gfx::RectF FullscreenMagnifierController::GetWindowRectDIP(float scale) const {
-  const gfx::Size size_in_dip = root_window_->bounds().size();
+  const gfx::Size size_in_dip = GetHostSizeDIP();
   const float width = size_in_dip.width() / scale;
   const float height = size_in_dip.height() / scale;
 
@@ -813,50 +838,45 @@
 
 void FullscreenMagnifierController::MoveMagnifierWindowFollowPoint(
     const gfx::Point& point,
-    int x_panning_margin,
-    int y_panning_margin,
-    int x_target_margin,
-    int y_target_margin,
+    int x_margin,
+    int y_margin,
     bool reduce_bottom_margin) {
   DCHECK(root_window_);
   bool start_zoom = false;
 
+  // Current position.
   const gfx::Rect window_rect = GetViewportRect();
-  const int left = window_rect.x();
-  const int right = window_rect.right();
-
-  int x_diff = 0;
-  if (point.x() < left + x_panning_margin) {
-    // Panning left.
-    x_diff = point.x() - (left + x_target_margin);
-    start_zoom = true;
-  } else if (right - x_panning_margin < point.x()) {
-    // Panning right.
-    x_diff = point.x() - (right - x_target_margin);
-    start_zoom = true;
-  }
-  int x = left + x_diff;
-
   const int top = window_rect.y();
   const int bottom = window_rect.bottom();
 
+  int x_diff = 0;
+  if (point.x() < window_rect.x() + x_margin) {
+    // Panning left.
+    x_diff = point.x() - (window_rect.x() + x_margin);
+    start_zoom = true;
+  } else if (point.x() > window_rect.right() - x_margin) {
+    // Panning right.
+    x_diff = point.x() - (window_rect.right() - x_margin);
+    start_zoom = true;
+  }
+  int x = window_rect.x() + x_diff;
+
   // If |reduce_bottom_margin| is true, use kKeyboardBottomPanningMargin instead
-  // of |y_panning_margin|. This is to prevent the magnifier from panning when
+  // of |y_margin|. This is to prevent the magnifier from panning when
   // the user is trying to interact with the bottom of the keyboard.
-  const int bottom_panning_margin = reduce_bottom_margin
-                                        ? kKeyboardBottomPanningMargin / scale_
-                                        : y_panning_margin;
+  const int bottom_panning_margin =
+      reduce_bottom_margin ? kKeyboardBottomPanningMargin / scale_ : y_margin;
 
   int y_diff = 0;
-  if (point.y() < top + y_panning_margin) {
+  if (point.y() < top + y_margin) {
     // Panning up.
-    y_diff = point.y() - (top + y_target_margin);
+    y_diff = point.y() - (top + y_margin);
     start_zoom = true;
   } else if (bottom - bottom_panning_margin < point.y()) {
     // Panning down.
     const int bottom_target_margin =
-        reduce_bottom_margin ? std::min(bottom_panning_margin, y_target_margin)
-                             : y_target_margin;
+        reduce_bottom_margin ? std::min(bottom_panning_margin, y_margin)
+                             : y_margin;
     y_diff = point.y() - (bottom - bottom_target_margin);
     start_zoom = true;
   }
diff --git a/ash/accessibility/magnifier/fullscreen_magnifier_controller.h b/ash/accessibility/magnifier/fullscreen_magnifier_controller.h
index 001026f..65ab4ae 100644
--- a/ash/accessibility/magnifier/fullscreen_magnifier_controller.h
+++ b/ash/accessibility/magnifier/fullscreen_magnifier_controller.h
@@ -224,17 +224,16 @@
   bool ProcessGestures();
 
   // Moves the viewport when |point| is located within
-  // |x_panning_margin| and |y_panning_margin| to the edge of the visible
+  // |x_margin| and |y_margin| to the edge of the visible
   // window region. The viewport will be moved so that the |point| will be
-  // moved to the point where it has |x_target_margin| and |y_target_margin|
-  // to the edge of the visible region. If |reduce_bottom_margin| is true,
-  // then a reduced value will be used as the |y_panning_margin| and
+  // moved to the point where it has |x_margin| and |y_margin|
+  // to the edge of the visible region if possible (less if the mouse is closer
+  // to the edge of the screen). If |reduce_bottom_margin| is true,
+  // then a reduced value will be used as the |y_margin| and
   // |y_target_margin| for the bottom edge.
   void MoveMagnifierWindowFollowPoint(const gfx::Point& point,
-                                      int x_panning_margin,
-                                      int y_panning_margin,
-                                      int x_target_margin,
-                                      int y_target_margin,
+                                      int x_margin,
+                                      int y_margin,
                                       bool reduce_bottom_margin);
 
   // Moves the viewport to center |point| in magnifier screen.
diff --git a/ash/accessibility/magnifier/fullscreen_magnifier_controller_unittest.cc b/ash/accessibility/magnifier/fullscreen_magnifier_controller_unittest.cc
index d43b258..08623a2 100644
--- a/ash/accessibility/magnifier/fullscreen_magnifier_controller_unittest.cc
+++ b/ash/accessibility/magnifier/fullscreen_magnifier_controller_unittest.cc
@@ -1041,4 +1041,43 @@
   EXPECT_NE(viewport_center, GetViewport().CenterPoint());
 }
 
+TEST_F(FullscreenMagnifierControllerTest, ContinuousFollowingReachesEdges) {
+  auto* magnifier = GetFullscreenMagnifierController();
+  magnifier->SetEnabled(true);
+  float scale = 10.0;
+  magnifier->SetScale(scale, /*animate=*/false);
+  magnifier->set_mouse_following_mode(MagnifierMouseFollowingMode::kContinuous);
+  ui::test::EventGenerator* event_generator = GetEventGenerator();
+
+  gfx::Point top_left(0, 0);
+  gfx::Point top_right(kRootWidth - 2, 0);
+  gfx::Point bottom_left(0, kRootHeight - 2);
+  gfx::Point bottom_right(kRootWidth - 2, kRootHeight - 2);
+
+  // Move until viewport upper left corner is at (0, 0).
+  // The generator moves the mouse within the scaled window,
+  // so it takes several iterations to get to the top corner.
+  while (GetViewport().ToString() != "0,0 80x60") {
+    event_generator->MoveMouseToInHost(top_left);
+  }
+  // Reset out of the corner a bit.
+  gfx::Point center(400, 300);
+  event_generator->MoveMouseToInHost(center);
+
+  // Move until viewport is all the way at the top right.
+  while (GetViewport().ToString() != "720,0 80x60") {
+    event_generator->MoveMouseToInHost(top_right);
+  }
+  event_generator->MoveMouseToInHost(center);
+
+  while (GetViewport().ToString() != "0,540 80x60") {
+    event_generator->MoveMouseToInHost(bottom_left);
+  }
+  event_generator->MoveMouseToInHost(center);
+
+  while (GetViewport().ToString() != "720,540 80x60") {
+    event_generator->MoveMouseToInHost(bottom_right);
+  }
+}
+
 }  // namespace ash