[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}
diff --git a/pdf/pdf_ink_module.cc b/pdf/pdf_ink_module.cc
index cd850de2..ca19b46 100644
--- a/pdf/pdf_ink_module.cc
+++ b/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 @@
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;
diff --git a/pdf/pdf_ink_module.h b/pdf/pdf_ink_module.h
index b9b0889..bec2674 100644
--- a/pdf/pdf_ink_module.h
+++ b/pdf/pdf_ink_module.h
@@ -149,7 +149,8 @@
// 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
diff --git a/pdf/pdf_ink_module_unittest.cc b/pdf/pdf_ink_module_unittest.cc
index ffd2856..81bfb324 100644
--- a/pdf/pdf_ink_module_unittest.cc
+++ b/pdf/pdf_ink_module_unittest.cc
@@ -602,13 +602,13 @@
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) {