Capture Mode: Enable cursor-compositing when dragging

This CL uses cursor-compositing to replace the cursor on screen while it
is being dragged.

Originally, when a user drags using the cursor while in capture region
mode, the selected/dragged area lags behind the cursor movements. To
solve that issue, the original screenshot_controller logic hides the
cursor on drag and draws a pseudo cursor directly on the canvas. After
attempting to draw a pseudo cursor on the canvas directly and
encountering issues, it was determined that a better solution would be
to use cursor-compositing, which is what this CL does.

Test: manual, added test
Bug: 1135700
Change-Id: Id35115ab53641c168d8dec4a62756f61d21c967e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487720
Commit-Queue: Richard Chui <richui@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820100}

ash/capture_mode/capture_mode_session.cc

@@ -231,6 +231,15 @@ class CaptureModeSession::ScopedCursorSetter {
 
   bool IsCursorVisible() const { return cursor_manager_->IsCursorVisible(); }
 
+  void HideCursor() {
+    if (original_cursor_locked_ || !IsCursorVisible())
+      return;
+
+    cursor_manager_->UnlockCursor();
+    cursor_manager_->HideCursor();
+    cursor_manager_->LockCursor();
+  }
+
  private:
   wm::CursorManager* const cursor_manager_;
   const gfx::NativeCursor original_cursor_;
@@ -674,6 +683,11 @@ void CaptureModeSession::OnLocatedEventPressed(
   initial_location_in_root_ = location_in_root;
   previous_location_in_root_ = location_in_root;
 
+  // Use cursor compositing instead of the platform cursor when dragging to
+  // ensure the cursor is aligned with the region.
+  is_drag_in_progress_ = true;
+  Shell::Get()->UpdateCursorCompositingEnabled();
+
   if (is_selecting_region_)
     return;
 
@@ -756,6 +770,8 @@ void CaptureModeSession::OnLocatedEventReleased(
   fine_tune_position_ = FineTunePosition::kNone;
   anchor_points_.clear();
 
+  is_drag_in_progress_ = false;
+  Shell::Get()->UpdateCursorCompositingEnabled();
   cursor_setter_->UpdateCursor(
       GetCursorType(GetFineTunePosition(location_in_root, /*is_touch=*/false),
                     is_event_on_capture_bar));
@@ -874,13 +890,15 @@ void CaptureModeSession::MaybeShowMagnifierGlassAtPoint(
   if (!capture_mode_util::IsCornerFineTunePosition(fine_tune_position_))
     return;
 
-  // TODO(richui): Hide cursor here.
+  DCHECK(cursor_setter_);
+  // Cursor will be reshown the next time we call UpdateCursor (which should be
+  // in OnLocatedEventReleased).
+  cursor_setter_->HideCursor();
   magnifier_glass_.ShowFor(current_root_, location_in_root);
 }
 
 void CaptureModeSession::CloseMagnifierGlass() {
   magnifier_glass_.Close();
-  // TODO(richui): Show cursor here.
 }
 
 std::vector<gfx::Point> CaptureModeSession::GetAnchorPointsForPosition(

ash/capture_mode/capture_mode_session.h

@@ -61,6 +61,7 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
 
   aura::Window* current_root() const { return current_root_; }
   bool is_selecting_region() const { return is_selecting_region_; }
+  bool is_drag_in_progress() const { return is_drag_in_progress_; }
 
   // Gets the current window selected for |kWindow| capture source. Returns
   // nullptr if no window is available for selection.
@@ -150,7 +151,7 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   // hide the cursor.
   void MaybeShowMagnifierGlassAtPoint(const gfx::Point& location_in_root);
 
-  // Closes |magnifier_glass_| and shows the cursor.
+  // Closes |magnifier_glass_|.
   void CloseMagnifierGlass();
 
   // Retrieves the anchor points on the current selected region associated with
@@ -239,6 +240,9 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
 
   // The object to specify the cursor type.
   std::unique_ptr<ScopedCursorSetter> cursor_setter_;
+
+  // True when dragging is in progress.
+  bool is_drag_in_progress_ = false;
 };
 
 }  // namespace ash

ash/capture_mode/capture_mode_unittests.cc

@@ -536,7 +536,8 @@ TEST_F(CaptureModeTest, CaptureRegionCoversCaptureModeBar) {
   EXPECT_FALSE(controller->IsActive());
 }
 
-// Tests that the magnifying glass appears while fine tuning the capture region.
+// Tests that the magnifying glass appears while fine tuning the capture region,
+// and that the cursor is hidden if the magnifying glass is present.
 TEST_F(CaptureModeTest, CaptureRegionMagnifierWhenFineTuning) {
   const gfx::Vector2d kDragDelta(50, 50);
   UpdateDisplay("800x800");
@@ -552,39 +553,47 @@ TEST_F(CaptureModeTest, CaptureRegionMagnifierWhenFineTuning) {
 
   auto check_magnifier_shows_properly = [this](const gfx::Point& origin,
                                                const gfx::Point& destination,
-                                               bool should_show) {
-    // If |should_show|, check that the magnifying glass is centered on the
-    // mouse after press and during drag. If not |should_show|, check that
-    // the magnifying glass never shows. Should always be not visible when
-    // mouse button is released.
+                                               bool should_show_magnifier) {
+    // If |should_show_magnifier|, check that the magnifying glass is centered
+    // on the mouse after press and during drag, and that the cursor is hidden.
+    // If not |should_show_magnifier|, check that the magnifying glass never
+    // shows. Should always be not visible when mouse button is released.
     auto* event_generator = GetEventGenerator();
     base::Optional<gfx::Point> expected_origin =
-        should_show ? base::make_optional(origin) : base::nullopt;
+        should_show_magnifier ? base::make_optional(origin) : base::nullopt;
     base::Optional<gfx::Point> expected_destination =
-        should_show ? base::make_optional(destination) : base::nullopt;
+        should_show_magnifier ? base::make_optional(destination)
+                              : base::nullopt;
+
+    auto* cursor_manager = Shell::Get()->cursor_manager();
+    EXPECT_TRUE(cursor_manager->IsCursorVisible());
 
     // Move cursor to |origin| and click.
     event_generator->set_current_screen_location(origin);
     event_generator->PressLeftButton();
     EXPECT_EQ(expected_origin, GetMagnifierGlassCenterPoint());
+    EXPECT_NE(should_show_magnifier, cursor_manager->IsCursorVisible());
 
     // Drag to |destination| while holding left button.
     event_generator->MoveMouseTo(destination);
     EXPECT_EQ(expected_destination, GetMagnifierGlassCenterPoint());
+    EXPECT_NE(should_show_magnifier, cursor_manager->IsCursorVisible());
 
     // Drag back to |origin| while still holding left button.
     event_generator->MoveMouseTo(origin);
     EXPECT_EQ(expected_origin, GetMagnifierGlassCenterPoint());
+    EXPECT_NE(should_show_magnifier, cursor_manager->IsCursorVisible());
 
     // Release left button.
     event_generator->ReleaseLeftButton();
     EXPECT_EQ(base::nullopt, GetMagnifierGlassCenterPoint());
+    EXPECT_TRUE(cursor_manager->IsCursorVisible());
   };
 
   // Drag the capture region from within the existing selected region. The
   // magnifier should not be visible at any point.
   check_magnifier_shows_properly(gfx::Point(400, 250), gfx::Point(500, 350),
-                                 /*should_show=*/false);
+                                 /*should_show_magnifier=*/false);
 
   // Check that each corner fine tune position shows the magnifier when
   // dragging.
@@ -602,7 +611,7 @@ TEST_F(CaptureModeTest, CaptureRegionMagnifierWhenFineTuning) {
             capture_region, fine_tune_position.position);
     check_magnifier_shows_properly(drag_affordance_location,
                                    drag_affordance_location + kDragDelta,
-                                   /*should_show=*/true);
+                                   /*should_show_magnifier=*/true);
   }
 }
 
@@ -867,7 +876,7 @@ TEST_F(CaptureModeTest, RegionCursorStates) {
   EXPECT_TRUE(cursor_manager->IsCursorVisible());
   EXPECT_EQ(CursorType::kCell, cursor_manager->GetCursor().type());
 
-  const gfx::Rect target_region(gfx::Rect(200, 200, 400, 400));
+  const gfx::Rect target_region(gfx::Rect(200, 200, 200, 200));
   SelectRegion(target_region);
 
   // Makes sure that the cursor is updated when the user releases the region
@@ -939,4 +948,50 @@ TEST_F(CaptureModeTest, RegionCursorStates) {
   EXPECT_EQ(original_cursor_type, cursor_manager->GetCursor().type());
 }
 
+// Tests that in Region mode, cursor compositing is used instead of the system
+// cursor when the cursor is being dragged.
+TEST_F(CaptureModeTest, RegionDragCursorCompositing) {
+  auto* event_generator = GetEventGenerator();
+  auto* session = StartImageRegionCapture()->capture_mode_session();
+  auto* cursor_manager = Shell::Get()->cursor_manager();
+
+  // Initially cursor should be visible and cursor compositing is not enabled.
+  EXPECT_FALSE(session->is_drag_in_progress());
+  EXPECT_FALSE(IsCursorCompositingEnabled());
+  EXPECT_TRUE(cursor_manager->IsCursorVisible());
+
+  const gfx::Rect target_region(gfx::Rect(200, 200, 200, 200));
+
+  // For each start and end point try dragging and verify that cursor
+  // compositing is functioning as expected.
+  struct {
+    std::string trace;
+    gfx::Point start_point;
+    gfx::Point end_point;
+  } kDragCases[] = {
+      {"initial_region", target_region.origin(), target_region.bottom_right()},
+      {"edge_resize", target_region.right_center(),
+       gfx::Point(target_region.right_center() + gfx::Vector2d(50, 0))},
+      {"corner_resize", target_region.origin(), gfx::Point(175, 175)},
+      {"move", gfx::Point(250, 250), gfx::Point(300, 300)},
+  };
+
+  for (auto test_case : kDragCases) {
+    SCOPED_TRACE(test_case.trace);
+
+    event_generator->MoveMouseTo(test_case.start_point);
+    event_generator->PressLeftButton();
+    EXPECT_TRUE(session->is_drag_in_progress());
+    EXPECT_TRUE(IsCursorCompositingEnabled());
+
+    event_generator->MoveMouseTo(test_case.end_point);
+    EXPECT_TRUE(session->is_drag_in_progress());
+    EXPECT_TRUE(IsCursorCompositingEnabled());
+
+    event_generator->ReleaseLeftButton();
+    EXPECT_FALSE(session->is_drag_in_progress());
+    EXPECT_FALSE(IsCursorCompositingEnabled());
+  }
+}
+
 }  // namespace ash

ash/display/cursor_window_controller.cc

@@ -5,6 +5,7 @@
 #include "ash/display/cursor_window_controller.h"
 
 #include "ash/capture_mode/capture_mode_controller.h"
+#include "ash/capture_mode/capture_mode_session.h"
 #include "ash/display/display_color_manager.h"
 #include "ash/display/mirror_window_controller.h"
 #include "ash/display/window_tree_host_manager.h"
@@ -137,10 +138,18 @@ bool CursorWindowController::ShouldEnableCursorCompositing() {
   if (is_cursor_motion_blur_enabled_)
     return true;
 
-  if (features::IsCaptureModeEnabled() &&
-      CaptureModeController::Get()->is_recording_in_progress()) {
-    // To let the video capturer record the cursor.
-    return true;
+  if (features::IsCaptureModeEnabled()) {
+    auto* controller = CaptureModeController::Get();
+    if (controller->is_recording_in_progress()) {
+      // To let the video capturer record the cursor.
+      return true;
+    }
+
+    auto* session = controller->capture_mode_session();
+    if (session && session->is_drag_in_progress()) {
+      // To ensure the cursor is aligned with the dragged region.
+      return true;
+    }
   }
 
   // During startup, we may not have a preference service yet. We need to check