[PDF Ink Signatures] Support extracting visible strokes

Add an iterator interface to allow extracting the visible strokes from
PdfInkModule.  Since PdfInkModule cannot directly reference PDFium, the
Ink strokes need to be exported to a caller that can use them to mark
into a PDF.

Bug: 335517469
Change-Id: Ia8fa90915577d10443ba445c20b07e31544bf611
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5656022
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363923}

pdf/BUILD.gn

@@ -492,7 +492,10 @@ if (enable_pdf) {
         "test/pdf_ink_test_helpers.h",
       ]
 
-      deps += [ "//third_party/ink" ]
+      deps += [
+        "//third_party/ink",
+        "//third_party/ink:input_test_support",
+      ]
     }
 
     if (enable_screen_ai_service) {

pdf/pdf_ink_module.cc

@@ -191,6 +191,10 @@ bool PdfInkModule::DrawThumbnail(SkCanvas& canvas, int page_index) {
   return true;
 }
 
+PdfInkModule::PageInkStrokeIterator PdfInkModule::GetVisibleStrokesIterator() {
+  return PageInkStrokeIterator(strokes_);
+}
+
 bool PdfInkModule::HandleInputEvent(const blink::WebInputEvent& event) {
   if (!enabled()) {
     return false;
@@ -870,4 +874,71 @@ void PdfInkModule::StrokeIdGenerator::ResetIdTo(size_t id) {
   next_stroke_id_ = id;
 }
 
+PdfInkModule::PageInkStrokeIterator::PageInkStrokeIterator(
+    const PdfInkModule::DocumentStrokesMap& strokes)
+    : strokes_(strokes), pages_iterator_(strokes_->cbegin()) {
+  // Set up internal iterators for the first visible stroke, if there is one.
+  AdvanceToNextPageWithVisibleStrokes();
+}
+
+PdfInkModule::PageInkStrokeIterator::~PageInkStrokeIterator() = default;
+
+std::optional<PdfInkModule::PageInkStroke>
+PdfInkModule::PageInkStrokeIterator::GetNextStrokeAndAdvance() {
+  if (pages_iterator_ == strokes_->cend()) {
+    return std::nullopt;
+  }
+
+  // `page_strokes_iterator_` is set up when finding the page, and is updated
+  // after establishing the stroke to return.  So the return value is based
+  // upon the current position of the iterator.  Callers should not get here
+  // if the end of the strokes has been reached for the current page.
+  CHECK(page_strokes_iterator_ != pages_iterator_->second.cend());
+  CHECK(page_strokes_iterator_->should_draw);
+  const ink::Stroke& page_stroke = page_strokes_iterator_->stroke;
+  int page_index = pages_iterator_->first;
+  AdvanceForCurrentPage();
+
+  if (page_strokes_iterator_ == pages_iterator_->second.cend()) {
+    // This was the last stroke for the current page, so advancing requires
+    // moving on to another page and reinitializing `page_strokes_iterator_`.
+    ++pages_iterator_;
+    AdvanceToNextPageWithVisibleStrokes();
+  }
+
+  return PageInkStroke{page_index, raw_ref<const ink::Stroke>(page_stroke)};
+}
+
+void PdfInkModule::PageInkStrokeIterator::
+    AdvanceToNextPageWithVisibleStrokes() {
+  for (; pages_iterator_ != strokes_->cend(); ++pages_iterator_) {
+    // Initialize and scan to the location of the first (if any) visible
+    // stroke for this page.
+    for (page_strokes_iterator_ = pages_iterator_->second.cbegin();
+         page_strokes_iterator_ != pages_iterator_->second.cend();
+         ++page_strokes_iterator_) {
+      if (page_strokes_iterator_->should_draw) {
+        // This page has visible strokes, and `page_strokes_iterator_` has
+        // been initialized to the position of the first visible stroke.
+        return;
+      }
+    }
+  }
+
+  // No pages with visible strokes found.
+}
+
+void PdfInkModule::PageInkStrokeIterator::AdvanceForCurrentPage() {
+  CHECK(pages_iterator_ != strokes_->cend());
+
+  // Advance the iterator to next visible stroke in this page (if any) before
+  // returning.
+  do {
+    ++page_strokes_iterator_;
+    if (page_strokes_iterator_ == pages_iterator_->second.cend()) {
+      break;
+    }
+  } while (!page_strokes_iterator_->should_draw);
+}
+
 }  // namespace chrome_pdf

pdf/pdf_ink_module.h

@@ -82,6 +82,47 @@ class PdfInkModule {
   // strokes for that page.
   using DocumentStrokeInputPointsMap = std::map<int, PageStrokeInputPoints>;
 
+  struct PageInkStroke {
+    int page_index;
+    raw_ref<const ink::Stroke> stroke;
+  };
+
+  // Iterator to get visible strokes.  Once created, the caller should ensure
+  // that there is no further PdfInkModule interactions until the iterator has
+  // been destroyed.
+  class PageInkStrokeIterator {
+   public:
+    explicit PageInkStrokeIterator(const DocumentStrokesMap& strokes);
+    PageInkStrokeIterator(const PageInkStrokeIterator&) = delete;
+    PageInkStrokeIterator& operator=(const PageInkStrokeIterator&) = delete;
+    ~PageInkStrokeIterator();
+
+    // Gets the next visible stroke if there is one, and advances the internal
+    // iterator to the next visible stroke.
+    std::optional<PageInkStroke> GetNextStrokeAndAdvance();
+
+   private:
+    // Helper to advance to the next page which has visible strokes.  If there
+    // is another page with visible strokes, performs the iterators
+    // initialization to be able to get the visible strokes for it.  Leaves
+    // `pages_iterator_` at end position if there are no more pages with
+    // visible strokes.
+    void AdvanceToNextPageWithVisibleStrokes();
+
+    // Helper to advance to the next visible stroke for the current page, if
+    // there is one.  Leaves `page_strokes_iterator_` at end position if there
+    // are no more visible strokes.
+    void AdvanceForCurrentPage();
+
+    const raw_ref<const DocumentStrokesMap> strokes_;
+
+    // Iterator for getting pages with visible strokes.
+    DocumentStrokesMap::const_iterator pages_iterator_;
+
+    // Iterator for getting visible strokes of a particular page.
+    PageStrokes::const_iterator page_strokes_iterator_;
+  };
+
   explicit PdfInkModule(PdfInkModuleClient& client);
   PdfInkModule(const PdfInkModule&) = delete;
   PdfInkModule& operator=(const PdfInkModule&) = delete;
@@ -98,6 +139,11 @@ class PdfInkModule {
   // that page, regardless of page visibility.
   bool DrawThumbnail(SkCanvas& canvas, int page_index);
 
+  // Gets an iterator for the visible strokes across all pages.
+  // Modifying the set of visible strokes while using the iterator is not
+  // supported and can result in undefined behavior.
+  PageInkStrokeIterator GetVisibleStrokesIterator();
+
   // Returns whether the event was handled or not.
   bool HandleInputEvent(const blink::WebInputEvent& event);
 

pdf/pdf_ink_module_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "pdf/pdf_ink_module.h"
 
+#include <array>
 #include <set>
 #include <string_view>
 #include <vector>
@@ -19,6 +20,7 @@
 #include "base/values.h"
 #include "pdf/pdf_features.h"
 #include "pdf/pdf_ink_brush.h"
+#include "pdf/pdf_ink_conversions.h"
 #include "pdf/pdf_ink_module_client.h"
 #include "pdf/pdf_ink_transform.h"
 #include "pdf/test/mouse_event_builder.h"
@@ -29,6 +31,7 @@
 #include "third_party/blink/public/common/input/web_mouse_event.h"
 #include "third_party/ink/src/ink/brush/brush.h"
 #include "third_party/ink/src/ink/geometry/affine_transform.h"
+#include "third_party/ink/src/ink/strokes/input/type_matchers.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkImage.h"
@@ -44,6 +47,7 @@ using testing::ElementsAre;
 using testing::ElementsAreArray;
 using testing::InSequence;
 using testing::Pair;
+using testing::Pointwise;
 using testing::SizeIs;
 
 namespace chrome_pdf {
@@ -68,10 +72,27 @@ constexpr gfx::PointF kTwoPageVerticalLayoutPointOutsidePages(10.0f, 0.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint1InsidePage0(10.0f, 10.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint2InsidePage0(15.0f, 15.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint3InsidePage0(20.0f, 15.0f);
+constexpr gfx::PointF kTwoPageVerticalLayoutPoint4InsidePage0(10.0f, 20.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint1InsidePage1(10.0f, 75.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint2InsidePage1(15.0f, 80.0f);
 constexpr gfx::PointF kTwoPageVerticalLayoutPoint3InsidePage1(20.0f, 80.0f);
 
+// Canonical points after stroking horizontal & vertical lines with some
+// commonly used points.
+// Horizontal line uses: kTwoPageVerticalLayoutPoint2InsidePage0 to
+//                       kTwoPageVerticalLayoutPoint3InsidePage0
+//                   or: kTwoPageVerticalLayoutPoint2InsidePage1 to
+//                       kTwoPageVerticalLayoutPoint3InsidePage1
+// Vertical line uses:   kTwoPageVerticalLayoutPoint1InsidePage0 to
+//                       kTwoPageVerticalLayoutPoint4InsidePage0
+constexpr gfx::PointF kTwoPageVerticalLayoutHorzLinePoint0Canonical(10.0f,
+                                                                    10.0f);
+constexpr gfx::PointF kTwoPageVerticalLayoutHorzLinePoint1Canonical(15.0f,
+                                                                    10.0f);
+constexpr gfx::PointF kTwoPageVerticalLayoutVertLinePoint0Canonical(5.0f, 5.0f);
+constexpr gfx::PointF kTwoPageVerticalLayoutVertLinePoint1Canonical(5.0f,
+                                                                    15.0f);
+
 // The inputs for a stroke that starts in first page, leaves the bounds of that
 // page, but then moves back into the page results in one stroke with two
 // segments.
@@ -86,10 +107,50 @@ constexpr gfx::PointF kTwoPageVerticalLayoutPageExitAndReentrySegment2[] = {
     gfx::PointF(10.0f, 0.0f), gfx::PointF(10.0f, 5.0f),
     gfx::PointF(15.0f, 10.0f)};
 
+// The stroke inputs for vertical and horizontal lines in the pages.  The
+// `.time` fields intentionally get a common value, to match the behavior of
+// `MouseEventBuilder`.
+constexpr auto kTwoPageVerticalLayoutHorzLinePage0Inputs =
+    std::to_array<PdfInkInputData>({
+        {kTwoPageVerticalLayoutHorzLinePoint0Canonical, base::Seconds(0)},
+        {kTwoPageVerticalLayoutHorzLinePoint1Canonical, base::Seconds(0)},
+    });
+constexpr auto kTwoPageVerticalLayoutVertLinePage0Inputs =
+    std::to_array<PdfInkInputData>({
+        {kTwoPageVerticalLayoutVertLinePoint0Canonical, base::Seconds(0)},
+        {kTwoPageVerticalLayoutVertLinePoint1Canonical, base::Seconds(0)},
+    });
+constexpr auto kTwoPageVerticalLayoutHorzLinePage1Inputs =
+    std::to_array<PdfInkInputData>({
+        {kTwoPageVerticalLayoutHorzLinePoint0Canonical, base::Seconds(0)},
+        {kTwoPageVerticalLayoutHorzLinePoint1Canonical, base::Seconds(0)},
+    });
+
 base::FilePath GetInkTestDataFilePath(std::string_view filename) {
   return base::FilePath(FILE_PATH_LITERAL("ink")).AppendASCII(filename);
 }
 
+// Matcher for ink::Stroke objects against their expected color and inputs.
+MATCHER_P(InkStrokeInputsEq, brush_color, "") {
+  const auto& [actual_stroke, expected_inputs] = arg;
+  const auto matcher = ink::StrokeInputBatchEq(expected_inputs);
+  return actual_stroke->GetBrush().GetColor() == brush_color &&
+         testing::Matches(matcher)(actual_stroke->GetInputs());
+}
+
+std::map<int, std::vector<raw_ref<const ink::Stroke>>> CollectVisibleStrokes(
+    PdfInkModule::PageInkStrokeIterator strokes_iter) {
+  std::map<int, std::vector<raw_ref<const ink::Stroke>>> visible_stroke_shapes;
+  for (auto page_stroke = strokes_iter.GetNextStrokeAndAdvance();
+       page_stroke.has_value();
+       page_stroke = strokes_iter.GetNextStrokeAndAdvance()) {
+    visible_stroke_shapes[page_stroke.value().page_index].push_back(
+        page_stroke.value().stroke);
+  }
+
+  return visible_stroke_shapes;
+}
+
 class FakeClient : public PdfInkModuleClient {
  public:
   FakeClient() = default;
@@ -1198,6 +1259,58 @@ TEST_F(PdfInkModuleUndoRedoTest, UndoRedoOnTwoPages) {
               ElementsAre(kPage0Matcher, kPage1Matcher));
 }
 
+using PdfInkModuleGetVisibleStrokesTest = PdfInkModuleStrokeTest;
+
+TEST_F(PdfInkModuleGetVisibleStrokesTest, NoPageStrokes) {
+  std::map<int, std::vector<raw_ref<const ink::Stroke>>>
+      collected_stroke_shapes =
+          CollectVisibleStrokes(ink_module().GetVisibleStrokesIterator());
+  ASSERT_EQ(collected_stroke_shapes.size(), 0u);
+}
+
+TEST_F(PdfInkModuleGetVisibleStrokesTest, MultiplePageStrokes) {
+  EnableAnnotationMode();
+  InitializeVerticalTwoPageLayout();
+
+  // Multiple strokes on one page, plus a stroke on another page.
+  ApplyStrokeWithMouseAtPoints(
+      kTwoPageVerticalLayoutPoint2InsidePage0,
+      base::span_from_ref(kTwoPageVerticalLayoutPoint3InsidePage0),
+      kTwoPageVerticalLayoutPoint3InsidePage0);
+  ApplyStrokeWithMouseAtPoints(
+      kTwoPageVerticalLayoutPoint1InsidePage0,
+      base::span_from_ref(kTwoPageVerticalLayoutPoint4InsidePage0),
+      kTwoPageVerticalLayoutPoint4InsidePage0);
+  ApplyStrokeWithMouseAtPoints(
+      kTwoPageVerticalLayoutPoint2InsidePage1,
+      base::span_from_ref(kTwoPageVerticalLayoutPoint3InsidePage1),
+      kTwoPageVerticalLayoutPoint3InsidePage1);
+
+  std::optional<ink::StrokeInputBatch> expected_page0_horz_line_input_batch =
+      CreateInkInputBatch(kTwoPageVerticalLayoutHorzLinePage0Inputs);
+  ASSERT_TRUE(expected_page0_horz_line_input_batch.has_value());
+  std::optional<ink::StrokeInputBatch> expected_page0_vert_line_input_batch =
+      CreateInkInputBatch(kTwoPageVerticalLayoutVertLinePage0Inputs);
+  ASSERT_TRUE(expected_page0_vert_line_input_batch.has_value());
+  std::optional<ink::StrokeInputBatch> expected_page1_horz_line_input_batch =
+      CreateInkInputBatch(kTwoPageVerticalLayoutHorzLinePage1Inputs);
+  ASSERT_TRUE(expected_page1_horz_line_input_batch.has_value());
+
+  const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting();
+  ASSERT_TRUE(brush);
+
+  std::map<int, std::vector<raw_ref<const ink::Stroke>>> collected_strokes =
+      CollectVisibleStrokes(ink_module().GetVisibleStrokesIterator());
+  EXPECT_THAT(
+      collected_strokes,
+      ElementsAre(
+          Pair(0, Pointwise(InkStrokeInputsEq(brush->GetInkBrush().GetColor()),
+                            {expected_page0_horz_line_input_batch.value(),
+                             expected_page0_vert_line_input_batch.value()})),
+          Pair(1, Pointwise(InkStrokeInputsEq(brush->GetInkBrush().GetColor()),
+                            {expected_page1_horz_line_input_batch.value()}))));
+}
+
 }  // namespace
 
 }  // namespace chrome_pdf

third_party/ink/BUILD.gn

@@ -173,3 +173,81 @@ source_set("ink") {
     "//third_party/ink_stroke_modeler",
   ]
 }
+
+source_set("geometry_test_support") {
+  testonly = true
+
+  visibility = [ ":*" ]
+
+  sources = [
+    "src/ink/geometry/type_matchers.cc",
+    "src/ink/geometry/type_matchers.h",
+  ]
+
+  public_configs = [ ":public_config" ]
+
+  configs -= [ "//build/config/compiler:chromium_code" ]
+  configs += [ "//build/config/compiler:no_chromium_code" ]
+
+  public_deps = [
+    ":ink",
+    "//testing/gtest",
+  ]
+
+  deps = [
+    "//testing/gmock",
+    "//third_party/abseil-cpp:absl",
+  ]
+}
+
+source_set("input_test_support") {
+  testonly = true
+
+  sources = [
+    "src/ink/strokes/input/type_matchers.cc",
+    "src/ink/strokes/input/type_matchers.h",
+  ]
+
+  public_configs = [ ":public_config" ]
+
+  configs -= [ "//build/config/compiler:chromium_code" ]
+  configs += [ "//build/config/compiler:no_chromium_code" ]
+
+  public_deps = [
+    ":ink",
+    "//testing/gmock",
+    "//testing/gtest",
+    "//third_party/abseil-cpp:absl",
+  ]
+
+  deps = [
+    ":geometry_test_support",
+    ":types_test_support",
+  ]
+}
+
+source_set("types_test_support") {
+  testonly = true
+
+  visibility = [ ":*" ]
+
+  sources = [
+    "src/ink/types/type_matchers.cc",
+    "src/ink/types/type_matchers.h",
+  ]
+
+  public_configs = [ ":public_config" ]
+
+  configs -= [ "//build/config/compiler:chromium_code" ]
+  configs += [ "//build/config/compiler:no_chromium_code" ]
+
+  public_deps = [
+    ":ink",
+    "//testing/gtest",
+  ]
+
+  deps = [
+    "//testing/gmock",
+    "//third_party/abseil-cpp:absl",
+  ]
+}