[unseasoned-pdf] Avoid scaling down window_rect to DIPS in PdfViewWebPlugin::UpdateGeometry().

PdfViewPluginBase::UpdateGeometryOnViewChanged() is shared by both
Pepper and Pepper-free plugins, which currently requires `new_view_rect`
in DIPs as an input. This input parameter may cause rounding issue in
the Pepper-free plugin when using zoom for DSF is enabled and the device
scale is larger than 1 because UpdateGeometry() needs to scale down
`window_rect` first before UpdateGeometryOnViewChanged() scale it back
up.

To avoid this rounding issue, this CL changes
PdfViewPluginBase::UpdateGeometryOnViewChanged() to accept a
`new_plugin_rect` in device pixels instead of a view rect in DIPs as an
input, also renames this function to
PdfViewPluginBase::UpdateGeometryOnPluginRectChanged().

Also fixes a unit test which was previously affected by the rounding
issue and add `device_to_css_scale_` and `plugin_rect_in_css_pixels_`
to PdfViewWebPlugin to simplify Paint().

Bug: 1238395
Change-Id: I6235ac878e5ee3d5b96f5c4c504c9db402621fb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3161413
Reviewed-by: K. Moon <kmoon@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#922272}

pdf/out_of_process_instance.cc

@@ -530,8 +530,9 @@ bool OutOfProcessInstance::HandleInputEvent(const pp::InputEvent& event) {
 }
 
 void OutOfProcessInstance::DidChangeView(const pp::View& view) {
-  UpdateGeometryOnViewChanged(RectFromPPRect(view.GetRect()),
-                              view.GetDeviceScale());
+  const gfx::Rect new_plugin_rect = gfx::ScaleToEnclosingRectSafe(
+      RectFromPPRect(view.GetRect()), view.GetDeviceScale());
+  UpdateGeometryOnPluginRectChanged(new_plugin_rect, view.GetDeviceScale());
 
   if (IsPrintPreview() && !stop_scrolling()) {
     set_scroll_position(PointFromPPPoint(view.GetScrollOffset()));

pdf/pdf_view_plugin_base.cc

@@ -797,12 +797,10 @@ void PdfViewPluginBase::PrintEnd() {
   engine_->PrintEnd();
 }
 
-void PdfViewPluginBase::UpdateGeometryOnViewChanged(
-    const gfx::Rect& new_view_rect,
+void PdfViewPluginBase::UpdateGeometryOnPluginRectChanged(
+    const gfx::Rect& new_plugin_rect,
     float new_device_scale) {
   DCHECK_GT(new_device_scale, 0.0f);
-  const gfx::Rect new_plugin_rect =
-      gfx::ScaleToEnclosingRectSafe(new_view_rect, new_device_scale);
 
   if (new_device_scale == device_scale_ && new_plugin_rect == plugin_rect_)
     return;
@@ -810,7 +808,13 @@ void PdfViewPluginBase::UpdateGeometryOnViewChanged(
   const float old_device_scale = device_scale_;
   device_scale_ = new_device_scale;
   plugin_rect_ = new_plugin_rect;
-  plugin_dip_size_ = new_view_rect.size();
+  // TODO(crbug.com/1250173): For the Pepper-free plugin, `plugin_dip_size_` is
+  // calculated from the `window_rect` in PdfViewWebPlugin::UpdateGeometry().
+  // We should try to avoid the downscaling during this calculation process and
+  // maybe migrate off `plugin_dip_size_`.
+  plugin_dip_size_ =
+      gfx::ScaleToEnclosingRectSafe(new_plugin_rect, 1.0f / new_device_scale)
+          .size();
 
   paint_manager_.SetSize(plugin_rect_.size(), device_scale_);
 

pdf/pdf_view_plugin_base.h

@@ -237,11 +237,11 @@ class PdfViewPluginBase : public PDFEngine::Client,
   // Returns the plugin-specific image data buffer.
   virtual Image GetPluginImageData() const;
 
-  // Updates the geometry of the plugin and its image data if the view's
-  // size or scale has changed. `new_view_rect` must be in CSS pixels (without
-  // device scale applied).
-  void UpdateGeometryOnViewChanged(const gfx::Rect& new_view_rect,
-                                   float new_device_scale);
+  // Updates the geometry of the plugin and its image data if the plugin rect
+  // or the device scale has changed. `new_plugin_rect` must be in device
+  // pixels (with the device scale applied).
+  void UpdateGeometryOnPluginRectChanged(const gfx::Rect& new_plugin_rect,
+                                         float new_device_scale);
 
   // A helper of OnGeometryChanged() which updates the available area and
   // the background parts, and notifies the PDF engine of geometry changes.

pdf/pdf_view_web_plugin.cc

@@ -357,19 +357,12 @@ void PdfViewWebPlugin::UpdateAllLifecyclePhases(
     blink::DocumentUpdateReason reason) {}
 
 void PdfViewWebPlugin::Paint(cc::PaintCanvas* canvas, const gfx::Rect& rect) {
-  // The scale level used to convert DIPs to CSS pixels.
-  float inverse_scale = 1.0f / (device_scale() * viewport_to_dip_scale_);
-
-  // `rect` is in CSS pixels, and the plugin rect is in DIPs. The plugin rect
-  // needs to be converted into CSS pixels before calculating the rect area to
-  // be invalidated.
-  gfx::Rect plugin_rect_in_css_pixels =
-      gfx::ScaleToEnclosingRectSafe(plugin_rect(), inverse_scale);
-
   // Clip the intersection of the paint rect and the plugin rect, so that
   // painting outside the plugin or the paint rect area can be avoided.
+  // Note: `rect` is in CSS pixels. We need to use `css_plugin_rect_`
+  // to calculate the intersection.
   SkRect invalidate_rect =
-      gfx::RectToSkRect(gfx::IntersectRects(plugin_rect_in_css_pixels, rect));
+      gfx::RectToSkRect(gfx::IntersectRects(css_plugin_rect_, rect));
   cc::PaintCanvasAutoRestore auto_restore(canvas, /*save=*/true);
   canvas->clipRect(invalidate_rect);
 
@@ -382,8 +375,8 @@ void PdfViewWebPlugin::Paint(cc::PaintCanvas* canvas, const gfx::Rect& rect) {
     return;
   }
 
-  if (inverse_scale != 1.0f)
-    canvas->scale(inverse_scale, inverse_scale);
+  if (device_to_css_scale_ != 1.0f)
+    canvas->scale(device_to_css_scale_, device_to_css_scale_);
 
   canvas->drawImage(snapshot_, plugin_rect().x(), plugin_rect().y());
 }
@@ -392,16 +385,7 @@ void PdfViewWebPlugin::UpdateGeometry(const gfx::Rect& window_rect,
                                       const gfx::Rect& clip_rect,
                                       const gfx::Rect& unobscured_rect,
                                       bool is_visible) {
-  float device_scale = container_wrapper_->DeviceScaleFactor();
-  viewport_to_dip_scale_ =
-      client_->IsUseZoomForDSFEnabled() ? 1.0f / device_scale : 1.0f;
-
-  // Note that `window_rect` is in viewport coordinates. It needs to be
-  // converted to DIPs before getting passed into
-  // PdfViewPluginBase::UpdateGeometryOnViewChanged().
-  OnViewportChanged(
-      gfx::ScaleToEnclosingRectSafe(window_rect, viewport_to_dip_scale_),
-      device_scale);
+  OnViewportChanged(window_rect, container_wrapper_->DeviceScaleFactor());
 }
 
 void PdfViewWebPlugin::UpdateFocus(bool focused,
@@ -882,9 +866,27 @@ void PdfViewWebPlugin::UserMetricsRecordAction(const std::string& action) {
   client_->RecordComputedAction(action);
 }
 
-void PdfViewWebPlugin::OnViewportChanged(const gfx::Rect& view_rect,
-                                         float new_device_scale) {
-  UpdateGeometryOnViewChanged(view_rect, new_device_scale);
+void PdfViewWebPlugin::OnViewportChanged(
+    const gfx::Rect& plugin_rect_in_css_pixel,
+    float new_device_scale) {
+  css_plugin_rect_ = plugin_rect_in_css_pixel;
+  float css_to_device_pixel_scale;
+  if (client_->IsUseZoomForDSFEnabled()) {
+    viewport_to_dip_scale_ = 1.0f / new_device_scale;
+    device_to_css_scale_ = 1.0f;
+    css_to_device_pixel_scale = 1.0f;
+  } else {
+    viewport_to_dip_scale_ = 1.0f;
+    device_to_css_scale_ = 1.0f / new_device_scale;
+    css_to_device_pixel_scale = new_device_scale;
+  }
+
+  // `plugin_rect_in_css_pixel` needs to be converted to device pixels before
+  // getting passed into PdfViewPluginBase::UpdateGeometryOnPluginRectChanged().
+  UpdateGeometryOnPluginRectChanged(
+      gfx::ScaleToEnclosingRectSafe(plugin_rect_in_css_pixel,
+                                    css_to_device_pixel_scale),
+      new_device_scale);
 
   if (IsPrintPreview() && !stop_scrolling()) {
     DCHECK_EQ(new_device_scale, device_scale());

pdf/pdf_view_web_plugin.h

@@ -286,7 +286,8 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   bool InitializeCommon(std::unique_ptr<ContainerWrapper> container_wrapper,
                         std::unique_ptr<PDFiumEngine> engine);
 
-  void OnViewportChanged(const gfx::Rect& view_rect, float new_device_scale);
+  void OnViewportChanged(const gfx::Rect& plugin_rect_in_css_pixel,
+                         float new_device_scale);
 
   // Invalidates the entire web plugin container and schedules a paint of the
   // page in it.
@@ -343,6 +344,12 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   // The viewport coordinates to DIP (device-independent pixel) ratio.
   float viewport_to_dip_scale_ = 1.0f;
 
+  // The device pixel to CSS pixel ratio.
+  float device_to_css_scale_ = 1.0f;
+
+  // The plugin rect in CSS pixels.
+  gfx::Rect css_plugin_rect_;
+
   // May be null in unit tests.
   std::unique_ptr<PdfAccessibilityDataHandler> const
       pdf_accessibility_data_handler_;

pdf/pdf_view_web_plugin_unittest.cc

@@ -292,7 +292,7 @@ class PdfViewWebPluginTest : public PdfViewWebPluginWithoutInitializeTest {
                                const gfx::Rect& window_rect,
                                const gfx::Rect& paint_rect) {
     EXPECT_CALL(*client_ptr_, IsUseZoomForDSFEnabled)
-        .WillOnce(Return(use_zoom_for_dsf));
+        .WillRepeatedly(Return(use_zoom_for_dsf));
     UpdatePluginGeometry(device_scale, window_rect);
     canvas_.DrawColor(kDefaultColor);
 
@@ -315,7 +315,7 @@ class PdfViewWebPluginTest : public PdfViewWebPluginWithoutInitializeTest {
                           const gfx::Rect& window_rect,
                           const gfx::Rect& paint_rect) {
     EXPECT_CALL(*client_ptr_, IsUseZoomForDSFEnabled)
-        .WillOnce(Return(use_zoom_for_dsf));
+        .WillRepeatedly(Return(use_zoom_for_dsf));
     UpdatePluginGeometry(device_scale, window_rect);
     canvas_.DrawColor(kDefaultColor);
 
@@ -397,7 +397,7 @@ TEST_F(PdfViewWebPluginTest,
   static constexpr UpdateGeometryParams kUpdateGeometryParams[] = {
       {1.0f, gfx::Rect(3, 4, 5, 6), gfx::Rect(3, 4, 5, 6)},
       {2.0f, gfx::Rect(4, 4, 12, 12), gfx::Rect(4, 4, 12, 12)},
-      {2.0f, gfx::Rect(5, 6, 7, 8), gfx::Rect(4, 6, 8, 8)},
+      {2.0f, gfx::Rect(5, 6, 7, 8), gfx::Rect(5, 6, 7, 8)},
   };
 
   for (const auto& params : kUpdateGeometryParams) {