[PDF Ink Signatures] Scale drawing tool cursor size with page zoom

Make the cursor look bigger when the page is zoomed in, as the strokes
drawn are bigger too. Cursor min/max sizes still apply.

Bug: 342602620
Change-Id: I2de0cdfe306c8e1aacc61c56cff70db106fab9c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5787460
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@{#1342438}

pdf/pdf_ink_cursor.cc

@@ -19,9 +19,12 @@ namespace chrome_pdf {
 
 namespace {
 
+constexpr int kMinDiameter = 6;
+constexpr int kMaxDiameter = 32;
+
 void CheckCursorDiameterIsInRange(int diameter) {
-  CHECK_GE(diameter, 6);
-  CHECK_LE(diameter, 32);
+  CHECK_GE(diameter, kMinDiameter);
+  CHECK_LE(diameter, kMaxDiameter);
 }
 
 SkColor GetCursorOutlineColor(SkColor color) {
@@ -40,11 +43,11 @@ SkColor GetCursorOutlineColor(SkColor color) {
 
 }  // namespace
 
-int CursorDiameterFromBrushSize(float brush_size) {
+int CursorDiameterFromBrushSizeAndZoom(float brush_size, float zoom) {
   PdfInkBrush::CheckToolSizeIsInRange(brush_size);
 
   constexpr float kMinSize = 4;  // Cursor become very hard to see if smaller.
-  float cursor_diameter = std::max(brush_size, kMinSize);
+  float cursor_diameter = std::max(brush_size * zoom, kMinSize);
 
   // Fudge factors to better match sizes used in Chrome OS Gallery.
   constexpr int kSmallAdjustment = 2;
@@ -52,7 +55,8 @@ int CursorDiameterFromBrushSize(float brush_size) {
   constexpr int kLargeSize = 10;
   int adjustment =
       cursor_diameter < kLargeSize ? kSmallAdjustment : kLargeAdjustment;
-  return static_cast<int>(round(cursor_diameter)) + adjustment;
+  return std::clamp(static_cast<int>(round(cursor_diameter)) + adjustment,
+                    kMinDiameter, kMaxDiameter);
 }
 
 SkBitmap GenerateToolCursor(SkColor color, int diameter) {

pdf/pdf_ink_cursor.h

@@ -16,7 +16,7 @@ namespace chrome_pdf {
 // Converts brush size into cursor diameter, for use with GenerateToolCursor().
 // `brush_size` must be in the range that passes the validation performed by
 // PdfInkBrush::CheckToolSizeIsInRange().
-int CursorDiameterFromBrushSize(float brush_size);
+int CursorDiameterFromBrushSizeAndZoom(float brush_size, float zoom);
 
 // Draws a custom circular cursor to represent the brush/highlighter/eraser
 // tool. `diameter` must be in the range [6, 32]. If it is too small, then it

pdf/pdf_ink_cursor_unittest.cc

@@ -12,21 +12,58 @@ namespace chrome_pdf {
 
 namespace {
 
-TEST(PdfInkCursorTest, CursorDiameterFromBrushSize) {
+TEST(PdfInkCursorTest, CursorDiameterFromBrushSizeAndZoom) {
   // Very small brush sizes result in a minimum cursor size.
-  EXPECT_EQ(6, CursorDiameterFromBrushSize(1.0f));
-  EXPECT_EQ(6, CursorDiameterFromBrushSize(2.0f));
-  EXPECT_EQ(6, CursorDiameterFromBrushSize(4.0f));
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/1.0f,
+                                                  /*zoom=*/1.0f));
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/2.0f,
+                                                  /*zoom=*/1.0f));
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/4.0f,
+                                                  /*zoom=*/1.0f));
 
   // Small brush sizes have a small offset value.
-  EXPECT_EQ(7, CursorDiameterFromBrushSize(5.0f));
-  EXPECT_EQ(8, CursorDiameterFromBrushSize(6.0f));
-  EXPECT_EQ(11, CursorDiameterFromBrushSize(9.0f));
+  EXPECT_EQ(7, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/5.0f,
+                                                  /*zoom=*/1.0f));
+  EXPECT_EQ(8, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/6.0f,
+                                                  /*zoom=*/1.0f));
+  EXPECT_EQ(11, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/9.0f,
+                                                   /*zoom=*/1.0f));
 
   // Larger brush sizes have a larger offset value.
-  EXPECT_EQ(14, CursorDiameterFromBrushSize(10.0f));
-  EXPECT_EQ(15, CursorDiameterFromBrushSize(11.0f));
-  EXPECT_EQ(20, CursorDiameterFromBrushSize(16.0f));
+  EXPECT_EQ(14, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/10.0f,
+                                                   /*zoom=*/1.0f));
+  EXPECT_EQ(15, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/11.0f,
+                                                   /*zoom=*/1.0f));
+  EXPECT_EQ(20, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/16.0f,
+                                                   /*zoom=*/1.0f));
+
+  // Very small brush sizes with zoom end up close to the minimum cursor size.
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/1.0f,
+                                                  /*zoom=*/0.25f));
+  EXPECT_EQ(7, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/1.0f,
+                                                  /*zoom=*/5.0f));
+
+  // Small brush sizes scale with zoom, but can get clipped at min/max cursor
+  // sizes.
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/5.0f,
+                                                  /*zoom=*/0.5f));
+  EXPECT_EQ(19, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/5.0f,
+                                                   /*zoom=*/3.0f));
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/9.0f,
+                                                  /*zoom=*/0.33f));
+  EXPECT_EQ(32, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/9.0f,
+                                                   /*zoom=*/4.0f));
+
+  // Larger brush sizes scale with zoom, but often get clipped at max cursor
+  // size.
+  EXPECT_EQ(6, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/10.0f,
+                                                  /*zoom=*/0.25f));
+  EXPECT_EQ(24, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/10.0f,
+                                                   /*zoom=*/2.0f));
+  EXPECT_EQ(32, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/10.0f,
+                                                   /*zoom=*/3.0f));
+  EXPECT_EQ(32, CursorDiameterFromBrushSizeAndZoom(/*brush_size=*/16.0f,
+                                                   /*zoom=*/5.0f));
 }
 
 TEST(PdfInkCursorTest, GenerateToolCursor) {

pdf/pdf_ink_module.cc

@@ -219,6 +219,10 @@ bool PdfInkModule::OnMessage(const base::Value::Dict& message) {
   return true;
 }
 
+void PdfInkModule::OnGeometryChanged() {
+  MaybeSetCursor();
+}
+
 const PdfInkBrush* PdfInkModule::GetPdfInkBrushForTesting() const {
   return is_drawing_stroke() ? drawing_stroke_state().brush.get() : nullptr;
 }
@@ -807,8 +811,9 @@ void PdfInkModule::MaybeSetCursor() {
     brush_size = erasing_stroke_state().eraser_size;
   }
 
-  client_->UpdateInkCursorImage(
-      GenerateToolCursor(color, CursorDiameterFromBrushSize(brush_size)));
+  client_->UpdateInkCursorImage(GenerateToolCursor(
+      color,
+      CursorDiameterFromBrushSizeAndZoom(brush_size, client_->GetZoom())));
 }
 
 PdfInkModule::DrawingStrokeState::DrawingStrokeState() = default;

pdf/pdf_ink_module.h

@@ -114,6 +114,9 @@ class PdfInkModule {
   // Returns whether the message was handled or not.
   bool OnMessage(const base::Value::Dict& message);
 
+  // Informs PdfInkModule that the plugin geometry changed.
+  void OnGeometryChanged();
+
   // For testing only. Returns the current `PdfInkBrush` used to draw strokes,
   // or nullptr if there is no brush.
   const PdfInkBrush* GetPdfInkBrushForTesting() const;

pdf/pdf_ink_module_unittest.cc

@@ -339,6 +339,40 @@ TEST_F(PdfInkModuleTest, MaybeSetCursorWhenChangingBrushes) {
   EXPECT_TRUE(ink_module().OnMessage(message));
 }
 
+TEST_F(PdfInkModuleTest, MaybeSetCursorWhenChangingZoom) {
+  {
+    InSequence seq;
+    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+        .WillOnce([](SkBitmap bitmap) {
+          EXPECT_EQ(6, bitmap.width());
+          EXPECT_EQ(6, bitmap.height());
+        });
+    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+        .WillOnce([](SkBitmap bitmap) {
+          EXPECT_EQ(20, bitmap.width());
+          EXPECT_EQ(20, bitmap.height());
+        });
+    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+        .WillOnce([](SkBitmap bitmap) {
+          EXPECT_EQ(10, bitmap.width());
+          EXPECT_EQ(10, bitmap.height());
+        });
+  }
+
+  EnableAnnotationMode();
+  EXPECT_TRUE(ink_module().enabled());
+
+  TestAnnotationBrushMessageParams message_params{/*color_r=*/0,
+                                                  /*color_g=*/255,
+                                                  /*color_b=*/0};
+  base::Value::Dict message = CreateSetAnnotationBrushMessageForTesting(
+      "pen", /*size=*/16.0, &message_params);
+  EXPECT_TRUE(ink_module().OnMessage(message));
+
+  client().set_zoom(0.5f);
+  ink_module().OnGeometryChanged();
+}
+
 class PdfInkModuleStrokeTest : public PdfInkModuleTest {
  protected:
   // Mouse locations used for `RunStrokeCheckTest()`.

pdf/pdf_view_web_plugin.cc

@@ -1874,8 +1874,15 @@ void PdfViewWebPlugin::OnGeometryChanged(double old_zoom,
                                          float old_device_scale) {
   RecalculateAreas(old_zoom, old_device_scale);
 
-  if (accessibility_state_ == AccessibilityState::kLoaded)
+  if (accessibility_state_ == AccessibilityState::kLoaded) {
     PrepareAndSetAccessibilityViewportInfo();
+  }
+
+#if BUILDFLAG(ENABLE_PDF_INK2)
+  if (ink_module_) {
+    ink_module_->OnGeometryChanged();
+  }
+#endif
 }
 
 void PdfViewWebPlugin::RecalculateAreas(double old_zoom,