Clip metalayer selection to screen bounds

Although metalayer selection is constructed from stylus
input points, it can sometimes cross the screen bounds.

For example, when a horizontal stroke is recognized, its
bounding rectangle is inflated to guarantee minimal
height and generally be closer to the visual image of
the stroke.

This patch introduces necessary clipping to make sure
that clients do not have to do that.

Bug: b:67021526
Test: ash_unittests HighlighterControllerTest.SelectionInsideScreen
Change-Id: I09d67c8b9f1176ea083a40f581b8a46536152296
Reviewed-on: https://chromium-review.googlesource.com/691278
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505401}

ash/highlighter/highlighter_controller.cc

@@ -131,6 +131,7 @@ void HighlighterController::RecognizeGesture() {
 
   aura::Window* current_window =
       highlighter_view_->GetWidget()->GetNativeWindow()->GetRootWindow();
+  const gfx::Rect bounds = current_window->bounds();
 
   const FastInkPoints& points = highlighter_view_->points();
   gfx::RectF box = points.GetBoundingBoxF();
@@ -145,8 +146,7 @@ void HighlighterController::RecognizeGesture() {
     box = AdjustHorizontalStroke(box, HighlighterView::kPenTipSize);
   } else if (gesture_type == HighlighterGestureType::kClosedShape) {
     const float fraction =
-        box.width() * box.height() /
-        (current_window->bounds().width() * current_window->bounds().height());
+        box.width() * box.height() / (bounds.width() * bounds.height());
     UMA_HISTOGRAM_PERCENTAGE("Ash.Shelf.Palette.Assistant.CircledPercentage",
                              static_cast<int>(fraction * 100));
   }
@@ -157,17 +157,28 @@ void HighlighterController::RecognizeGesture() {
                  base::Unretained(this)));
 
   if (gesture_type != HighlighterGestureType::kNotRecognized) {
-    if (observer_) {
-      observer_->HandleSelection(gfx::ToEnclosingRect(
-          gfx::ScaleRect(box, GetScreenshotScale(current_window))));
+    // |box| is not guaranteed to be inside the screen bounds, clip it.
+    // Not converting |box| to gfx::Rect here to avoid accumulating rounding
+    // errors, instead converting |bounds| to gfx::RectF.
+    box.Intersect(
+        gfx::RectF(bounds.x(), bounds.y(), bounds.width(), bounds.height()));
+    if (box.IsEmpty()) {
+      if (observer_)
+        observer_->HandleFailedSelection();
+    } else {
+      if (observer_) {
+        observer_->HandleSelection(gfx::ToEnclosingRect(
+            gfx::ScaleRect(box, GetScreenshotScale(current_window))));
+      }
+
+      result_view_ = base::MakeUnique<HighlighterResultView>(current_window);
+      result_view_->Animate(
+          box, gesture_type,
+          base::Bind(&HighlighterController::DestroyResultView,
+                     base::Unretained(this)));
+
+      recognized_gesture_counter_++;
     }
-
-    result_view_ = base::MakeUnique<HighlighterResultView>(current_window);
-    result_view_->Animate(box, gesture_type,
-                          base::Bind(&HighlighterController::DestroyResultView,
-                                     base::Unretained(this)));
-
-    recognized_gesture_counter_++;
   } else if (observer_) {
     observer_->HandleFailedSelection();
   }

ash/highlighter/highlighter_controller_test_api.cc

@@ -29,6 +29,8 @@ void HighlighterControllerTestApi::DestroyPointerView() {
 }
 
 void HighlighterControllerTestApi::SimulateInterruptedStrokeTimeout() {
+  if (!instance_->interrupted_stroke_timer_)
+    return;
   instance_->interrupted_stroke_timer_->Stop();
   instance_->RecognizeGesture();
 }

ash/highlighter/highlighter_controller_unittest.cc

@@ -348,4 +348,84 @@ TEST_F(HighlighterControllerTest, InterruptedStroke) {
   EXPECT_TRUE(controller_test_api_->IsFadingAway());
 }
 
+// Test that the selection is never crossing the screen bounds.
+TEST_F(HighlighterControllerTest, SelectionInsideScreen) {
+  controller_test_api_->SetEnabled(true);
+  GetEventGenerator().EnterPenPointerMode();
+
+  constexpr float display_scales[] = {1.f, 1.5f, 2.0f};
+
+  for (size_t i = 0; i < sizeof(display_scales) / sizeof(float); ++i) {
+    std::string display_spec =
+        base::StringPrintf("1000x1000*%.2f", display_scales[i]);
+    SCOPED_TRACE(display_spec);
+    UpdateDisplay(display_spec);
+
+    const gfx::Rect screen(0, 0, 1000, 1000);
+
+    // Rectangle completely offscreen.
+    controller_test_api_->ResetSelection();
+    TraceRect(gfx::Rect(-100, -100, 10, 10));
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_failed_selection_called());
+
+    // Rectangle crossing the left edge.
+    controller_test_api_->ResetSelection();
+    TraceRect(gfx::Rect(-100, 100, 200, 200));
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+
+    // Rectangle crossing the top edge.
+    controller_test_api_->ResetSelection();
+    TraceRect(gfx::Rect(100, -100, 200, 200));
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+
+    // Rectangle crossing the right edge.
+    controller_test_api_->ResetSelection();
+    TraceRect(gfx::Rect(900, 100, 200, 200));
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+
+    // Rectangle crossing the bottom edge.
+    controller_test_api_->ResetSelection();
+    TraceRect(gfx::Rect(100, 900, 200, 200));
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+
+    // Horizontal stroke completely offscreen.
+    controller_test_api_->ResetSelection();
+    GetEventGenerator().MoveTouch(gfx::Point(0, -100));
+    GetEventGenerator().PressTouch();
+    GetEventGenerator().MoveTouch(gfx::Point(1000, -100));
+    GetEventGenerator().ReleaseTouch();
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_failed_selection_called());
+
+    // Horizontal stroke along the top edge of the screen.
+    controller_test_api_->ResetSelection();
+    GetEventGenerator().MoveTouch(gfx::Point(0, 0));
+    GetEventGenerator().PressTouch();
+    GetEventGenerator().MoveTouch(gfx::Point(1000, 0));
+    GetEventGenerator().ReleaseTouch();
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+
+    // Horizontal stroke along the bottom edge of the screen.
+    controller_test_api_->ResetSelection();
+    GetEventGenerator().MoveTouch(gfx::Point(0, 999));
+    GetEventGenerator().PressTouch();
+    GetEventGenerator().MoveTouch(gfx::Point(1000, 999));
+    GetEventGenerator().ReleaseTouch();
+    controller_test_api_->SimulateInterruptedStrokeTimeout();
+    EXPECT_TRUE(controller_test_api_->handle_selection_called());
+    EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
+  }
+}
+
 }  // namespace ash

ash/highlighter/highlighter_selection_observer.h

@@ -16,6 +16,8 @@ class HighlighterSelectionObserver {
  public:
   virtual ~HighlighterSelectionObserver() {}
 
+  // |rect| is the selected rectangle in screen pixes, clipped to screen bounds
+  // if necessary.
   virtual void HandleSelection(const gfx::Rect& rect) = 0;
   virtual void HandleFailedSelection() = 0;
 };