[PDF Ink Signatures] Avoid flashing after stroke completion

Painting in PdfViewWebPlugin uses a current image snapshot to draw onto
the canvas.  This gets updated after PaintManager finishes doing a flush
of paint requests.

After a stroke has been added to a PDF page, PdfInkModule makes an
invalidate call which kicks off the request to redraw the area where the
stroke occurred.  However, there can be a call for PdfViewWebPlugin to
paint before the snapshot has been updated.  This presents a period
where the snapshot does not include the newly drawn Ink stroke, and
there are no longer any in-progress Ink inputs held by PdfInkModule.
This can result in a flash, where the paint uses only the now-stale
snapshot.

Avoid the flash effect where a newly drawn Ink stroke temporarily
disappears by retaining the snapshot rendering of the in-progress Ink
stroke.  Reuse this in PdfViewWebPlugin::Paint() until the current image
snapshot gets updated.

Fixed: 380057101
Change-Id: Icda9260a6d022f3458f0ae1e28dfa5d492daa8c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6363095
Reviewed-by: Andy Phan <andyphan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1433897}

pdf/pdf_view_web_plugin.cc

@@ -605,7 +605,11 @@ void PdfViewWebPlugin::Paint(cc::PaintCanvas* canvas, const gfx::Rect& rect) {
   canvas->drawImage(snapshot_, 0, 0);
 
 #if BUILDFLAG(ENABLE_PDF_INK2)
-  if (ink_module_ && ink_module_->HasInputsToDraw()) {
+  if (!ink_module_) {
+    return;
+  }
+
+  if (ink_module_->HasInputsToDraw()) {
     SkBitmap sk_bitmap;
     sk_bitmap.allocPixels(
         SkImageInfo::MakeN32Premul(rect.width(), rect.height()));
@@ -615,13 +619,18 @@ void PdfViewWebPlugin::Paint(cc::PaintCanvas* canvas, const gfx::Rect& rect) {
 
     sk_sp<SkImage> snapshot = sk_bitmap.asImage();
     CHECK(snapshot);
-    cc::PaintImage cc_snapshot =
+    snapshot_ink_inputs_ =
         cc::PaintImageBuilder::WithDefault()
             .set_image(std::move(snapshot), cc::PaintImage::GetNextContentId())
             .set_id(cc::PaintImage::GetNextId())
             .set_no_cache(true)
             .TakePaintImage();
-    canvas->drawImage(cc_snapshot, 0, 0);
+    canvas->drawImage(snapshot_ink_inputs_.value(), 0, 0);
+  } else if (snapshot_ink_inputs_.has_value()) {
+    // Waiting on `snapshot_` to get refreshed to reflect the change for an
+    // added stroke, so reapply the last Ink inputs snapshot to avoid a flash
+    // of a recently added stroke temporarily disappearing.
+    canvas->drawImage(snapshot_ink_inputs_.value(), 0, 0);
   }
 #endif  // BUILDFLAG(ENABLE_PDF_INK2)
 }
@@ -2264,6 +2273,14 @@ void PdfViewWebPlugin::UpdateSnapshot(sk_sp<SkImage> snapshot) {
           .set_no_cache(true)
           .TakePaintImage();
 
+#if BUILDFLAG(ENABLE_PDF_INK2)
+  // `paint_manager_` updates the snapshot after it has completed painting,
+  // which uses `engine_` in `DoPaint()`.  Any newly added Ink stroke will now
+  // be applied in the snapshot, so there is no need to retain any prior
+  // `snapshot_ink_inputs_`.
+  snapshot_ink_inputs_.reset();
+#endif
+
   if (!plugin_rect_.IsEmpty())
     InvalidatePluginContainer();
 }

pdf/pdf_view_web_plugin.h

@@ -481,6 +481,10 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   const std::vector<gfx::Rect>& deferred_invalidates_for_testing() const {
     return deferred_invalidates_;
   }
+
+  bool HasInkInputsSnapshotForTesting() const {
+    return snapshot_ink_inputs_.has_value();
+  }
 #endif  // BUILDFLAG(ENABLE_PDF_INK2)
 
  private:
@@ -768,6 +772,11 @@ class PdfViewWebPlugin final : public PDFiumEngineClient,
   // The current image snapshot.
   cc::PaintImage snapshot_;
 
+#if BUILDFLAG(ENABLE_PDF_INK2)
+  // The last saved image snapshot for rendering of Ink inputs.
+  std::optional<cc::PaintImage> snapshot_ink_inputs_;
+#endif
+
   // Translate from snapshot to device pixels.
   gfx::Vector2dF snapshot_translate_;
 

pdf/pdf_view_web_plugin_unittest.cc

@@ -3004,9 +3004,10 @@ TEST_F(PdfViewWebPluginInkTest, DrawInProgressStroke) {
 
   // Draw the canvas for the in-progress stroke.
   plugin_->Paint(canvas_.sk_canvas(), kScreenRect);
-  EXPECT_TRUE(MatchesPngFile(
+  const base::FilePath stroked_image_png_file =
-      canvas_.GetBitmap().asImage().get(),
+      GetInkTestDataFilePath(FILE_PATH_LITERAL("diagonal_stroke.png"));
-      GetInkTestDataFilePath(FILE_PATH_LITERAL("diagonal_stroke.png"))));
+  EXPECT_TRUE(MatchesPngFile(canvas_.GetBitmap().asImage().get(),
+                             stroked_image_png_file));
 
   // Finish the stroke.  After a stroke is finished there is nothing more to
   // be drawn by PdfInkModule, as the completed stroke is provided by a
@@ -3017,6 +3018,27 @@ TEST_F(PdfViewWebPluginInkTest, DrawInProgressStroke) {
                          .CreateLeftMouseUpAtPosition(kStrokeEndingPosition)
                          .Build(),
                      blink::WebInputEventResult::kHandledApplication);
+
+  // Updating of `PdfViewWebPlugin::snapshot_` does not happen automatically
+  // on the invalidate call, but later after the tasks PaintManager posted have
+  // a chance to run.  This means painting uses the last snapshot, which does
+  // not include the last Ink stroke.  This results in the most recent stroke
+  // disappearing, causing a flash for the user unless the snapshot from the
+  // most recent stroke is reused.
+  EXPECT_TRUE(plugin_->HasInkInputsSnapshotForTesting());
+  plugin_->Paint(canvas_.sk_canvas(), kScreenRect);
+  EXPECT_TRUE(MatchesPngFile(canvas_.GetBitmap().asImage().get(),
+                             stroked_image_png_file));
+
+  // Simulate how the snapshot eventually gets updated, after all necessary
+  // tasks that normally happen from the PaintManager finally complete.  That
+  // results in a blank canvas here for this test, as PdfViewWebPlugin no
+  // longer uses the last Ink rendering snapshot for painting, and
+  // ApplyStroke() was mocked out so there is nothing to draw from the PDF
+  // engine.
+  plugin_->UpdateSnapshot(CreateSkiaImageForTesting(
+      plugin_->GetPluginRectForTesting().size(), SK_ColorWHITE));
+  EXPECT_FALSE(plugin_->HasInkInputsSnapshotForTesting());
   plugin_->Paint(canvas_.sk_canvas(), kScreenRect);
   EXPECT_TRUE(cc::MatchesBitmap(canvas_.GetBitmap(), blank_bitmap,
                                 cc::ExactPixelComparator()));