[PDF Ink Signatures] Extrapolate stroke when crossing page boundary

When input events cross over the page boundary sufficiently fast, the
discrete events PdfInkModule receives may leave gaps between the end of
a stroke segment and the page boundary. Detect when this condition
happens in PdfInkModule::ContinueStroke(), and extrapolate the missing
segment using CalculatePageBoundaryIntersectPoint().

Bug: 352578791
Change-Id: Iba19f14607e08bd4c33d2663c549f7d264c6af44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5749713
Reviewed-by: Alan Screen <awscreen@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1335821}

pdf/pdf_ink_module.cc

@@ -22,6 +22,7 @@
 #include "base/ranges/algorithm.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "pdf/draw_utils/page_boundary_intersect.h"
 #include "pdf/ink/ink_affine_transform.h"
 #include "pdf/ink/ink_brush.h"
 #include "pdf/ink/ink_in_progress_stroke.h"
@@ -332,56 +333,89 @@ bool PdfInkModule::ContinueStroke(const gfx::PointF& position) {
     return false;
   }
 
-  int page_index = client_->VisiblePageIndexFromPoint(position);
-  if (page_index != state.page_index) {
-    // Stroke has left the page.  Do not add this input point.
-    if (!state.inputs.back().empty()) {
-      // Create a new segment to collect any further points.
-      state.inputs.push_back(StrokeInputSegment());
-
-      // Even if the last event position was not on the page boundary, no
-      // further points are captured in the stroke from that position to this
-      // new out-of-bounds position.  So there is no need to invalidate further
-      // from it, just drop it since it is now stale for any new points.
-      state.input_last_event_position.reset();
-    }
-
-    // Treat event as handled.
-    return true;
-  }
-
-  CHECK_GE(state.page_index, 0);
-  gfx::PointF page_position =
-      ConvertEventPositionToCanonicalPosition(position, state.page_index);
-
-  const bool is_start_of_new_segment = state.inputs.back().empty();
-  CHECK_NE(is_start_of_new_segment,
-           state.input_last_event_position.has_value());
-  if (!is_start_of_new_segment &&
-      position == state.input_last_event_position.value()) {
+  CHECK(state.input_last_event_position.has_value());
+  const gfx::PointF last_position = state.input_last_event_position.value();
+  if (position == last_position) {
     // Since the position did not change, do nothing.
     return true;
   }
 
+  const int page_index = client_->VisiblePageIndexFromPoint(position);
+  const int last_page_index = client_->VisiblePageIndexFromPoint(last_position);
+  if (page_index != state.page_index && last_page_index != state.page_index) {
+    // If `position` is outside the page, and so was `last_position`, then just
+    // update `last_position` and treat the event as handled.
+    state.input_last_event_position = position;
+    return true;
+  }
+
+  CHECK_GE(state.page_index, 0);
+  if (page_index != state.page_index) {
+    // `position` is outside the page, and `last_position` is inside the page.
+    CHECK_EQ(last_page_index, state.page_index);
+    const gfx::PointF boundary_position = CalculatePageBoundaryIntersectPoint(
+        client_->GetPageContentsRect(state.page_index), last_position,
+        position);
+    if (boundary_position != last_position) {
+      // Record the last point before leaving the page, if `last_position` was
+      // not already on the page boundary.
+      gfx::PointF canonical_boundary_position =
+          ConvertEventPositionToCanonicalPosition(boundary_position,
+                                                  state.page_index);
+      base::TimeDelta time_diff = base::Time::Now() - state.start_time.value();
+      state.inputs.back().push_back({
+          .position = InkPoint{canonical_boundary_position.x(),
+                               canonical_boundary_position.y()},
+          .elapsed_time_seconds = static_cast<float>(time_diff.InSecondsF()),
+      });
+
+      client_->Invalidate(
+          state.brush->GetInvalidateArea(last_position, boundary_position));
+    }
+
+    // Remember `position` for use in the next event and treat event as handled.
+    state.input_last_event_position = position;
+    return true;
+  }
+
+  gfx::PointF invalidation_position = last_position;
+  if (last_page_index != state.page_index) {
+    // If the stroke left the page and is now re-entering, then start a new
+    // segment.
+    CHECK(!state.inputs.back().empty());
+    state.inputs.push_back(StrokeInputSegment());
+    const gfx::PointF boundary_position = CalculatePageBoundaryIntersectPoint(
+        client_->GetPageContentsRect(state.page_index), position,
+        last_position);
+    if (boundary_position != position) {
+      // Record the first point after entering the page.
+      gfx::PointF canonical_boundary_position =
+          ConvertEventPositionToCanonicalPosition(boundary_position,
+                                                  state.page_index);
+      base::TimeDelta time_diff = base::Time::Now() - state.start_time.value();
+      state.inputs.back().push_back({
+          .position = InkPoint{canonical_boundary_position.x(),
+                               canonical_boundary_position.y()},
+          .elapsed_time_seconds = static_cast<float>(time_diff.InSecondsF()),
+      });
+      invalidation_position = boundary_position;
+    }
+  }
+
+  gfx::PointF canonical_position =
+      ConvertEventPositionToCanonicalPosition(position, state.page_index);
   base::TimeDelta time_diff = base::Time::Now() - state.start_time.value();
   state.inputs.back().push_back({
-      .position = InkPoint{page_position.x(), page_position.y()},
+      .position = InkPoint{canonical_position.x(), canonical_position.y()},
       .elapsed_time_seconds = static_cast<float>(time_diff.InSecondsF()),
   });
 
-  if (is_start_of_new_segment) {
-    // Only invalidate around the single point in the new segment.
-    client_->Invalidate(state.brush->GetInvalidateArea(position, position));
-  } else {
-    // Invalidate area covering a straight line between this position and the
-    // previous one.  Update last location to support invalidating from here to
-    // the next position.
-    client_->Invalidate(state.brush->GetInvalidateArea(
-        position, state.input_last_event_position.value()));
-  }
+  // Invalidate area covering a straight line between this position and the
+  // previous one.
+  client_->Invalidate(
+      state.brush->GetInvalidateArea(position, invalidation_position));
 
-  // Update last location to support invalidating from here to
-  // the next position.
+  // Remember `position` for use in the next event.
   state.input_last_event_position = position;
 
   return true;

pdf/pdf_ink_module.h

@@ -149,7 +149,8 @@ class PdfInkModule {
     // The event position for the last input.  Coordinates match the
     // screen-based position that are provided during stroking from
     // `blink::WebMouseEvent` positions.  Used after stroking has already
-    // started, to support invalidation.
+    // started, for invalidation and for extrapolating where a stroke crosses
+    // the page boundary.
     std::optional<gfx::PointF> input_last_event_position;
 
     // The points that make up the current stroke, divided into

pdf/pdf_ink_module_unittest.cc

@@ -602,13 +602,13 @@ TEST_F(PdfInkModuleStrokeTest, StrokePageExitAndReentryWithQuickMoves) {
                                kQuickPageExitAndReentryPoints,
                                kTwoPageVerticalLayoutPoint2InsidePage0);
 
-  // TODO(crbug.com/352578791): The strokes should be:
-  // 1) `kTwoPageVerticalLayoutPageExitAndReentrySegment1`
-  // 2) {gfx::PointF(6.666667f, 0.0f), gfx::PointF(10.0f, 10.0f)}
-  EXPECT_THAT(StrokeInputPositions(),
-              ElementsAre(Pair(
-                  0, ElementsAre(ElementsAre(gfx::PointF(5.0f, 5.0f)),
-                                 ElementsAre(gfx::PointF(10.0f, 10.0f))))));
+  EXPECT_THAT(
+      StrokeInputPositions(),
+      ElementsAre(Pair(
+          0, ElementsAre(ElementsAreArray(
+                             kTwoPageVerticalLayoutPageExitAndReentrySegment1),
+                         ElementsAreArray({gfx::PointF(6.666667f, 0.0f),
+                                           gfx::PointF(10.0f, 10.0f)})))));
 }
 
 TEST_F(PdfInkModuleStrokeTest, EraseStroke) {