Scale to DIPs in ppb_graphics2d_impl for proper invalidation

webkit/plugins/ppapi/ppb_graphics_2d_impl.cc

Index: webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
index 79821bb55a0b06764cdc9dafe0eb6aced70006ef..f652bdd8ef9ff8f04e271403b6e0baf9f8060038 100644
--- a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
+++ b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
@@ -350,6 +350,26 @@ int32_t PPB_Graphics2D_Impl::Flush(scoped_refptr<TrackedCallback> callback) {
     // calls, leaving our callback stranded. So we still need to check whether
     // the repainted area is visible to determine how to deal with the callback.
     if (bound_instance_ && !op_rect.IsEmpty()) {
+      bool scroll_valid = true;
+      gfx::Point scroll_delta(operation.scroll_dx, operation.scroll_dy);

[brettw, 2012/07/20 20:56:45]

When I suggested a different function, I was hoping to get all of this block out
and add a test for it. It makes the flow of this function harder to follow,
rather than just the rounding (which I'm less concerned about).

Maybe you can just use some in/out params and take Wez's suggestion of
converting the type to PAINT.

bool ConvertToLogicalPixels(float scale, QueuedOperation::Type* type, gfx::Rect*
op_rect, gfx::Point* scroll_delta);

[Josh Horwich, 2012/07/20 22:33:08]

Done.

I didn't include the in/out of QueuedOperation::Type since that would mean
moving QueuedOperation into the header - but the impl of Flush manages
fallback-to-Invalidate using Wez's suggestion.

+      if (scale_ != 1.0f && scale_ > 0.0f) {
+        // Convert |op_rect| from context coordinates to logical pixels, taking
+        // care to include partially-covered logical pixels (aka DIPs). If the
+        // operation is a scroll, we also find out if we need to fall back to
+        // InvalidateRect due to the possibility that the scroll cannot
+        // accurately be described in logical pixels due to rounding errors.
+        gfx::Rect original_rect = op_rect;
+        op_rect = ScaleRectBounds(op_rect, scale_);
+        if (operation.type == QueuedOperation::SCROLL) {
+          gfx::Point original_delta = scroll_delta;
+          float inverse_scale = 1.0f / scale_;
+          scroll_delta = scroll_delta.Scale(scale_);
+          if (original_rect != ScaleRectBounds(op_rect, inverse_scale) ||
+              original_delta != scroll_delta.Scale(inverse_scale)) {
+            scroll_valid = false;

[Wez, 2012/07/20 05:31:16]

You could just override operation.type to PAINT and avoid the need for
|scroll_valid|.

[Josh Horwich, 2012/07/20 22:33:08]

Done.

+          }
+        }
+      }
 
       // Set |nothing_visible| to false if the change overlaps the visible area.
       gfx::Rect visible_changed_rect =
@@ -360,8 +380,8 @@ int32_t PPB_Graphics2D_Impl::Flush(scoped_refptr<TrackedCallback> callback) {
 
       // Notify the plugin of the entire change (op_rect), even if it is
       // partially or completely off-screen.
-      if (operation.type == QueuedOperation::SCROLL) {
-        bound_instance_->ScrollRect(operation.scroll_dx, operation.scroll_dy,
+      if (operation.type == QueuedOperation::SCROLL && scroll_valid) {
+        bound_instance_->ScrollRect(scroll_delta.x(), scroll_delta.y(),
                                     op_rect);
       } else {
         bound_instance_->InvalidateRect(op_rect);
@@ -524,6 +544,10 @@ void PPB_Graphics2D_Impl::Paint(WebKit::WebCanvas* canvas,
 
   CGContextClipToRect(canvas, bounds);
 
+  // TODO(jhorwich) Figure out if this code is even active anymore, and if so
+  // how to properly handle scaling.
+  DCHECK_EQ(1.0f, scale_);
+
   // TODO(brettw) bug 56673: do a direct memcpy instead of going through CG
   // if the is_always_opaque_ flag is set. Must ensure bitmap is still clipped.
 
@@ -610,6 +634,20 @@ void PPB_Graphics2D_Impl::ViewFlushedPaint() {
     painted_flush_callback_.Execute(PP_OK);
 }
 
+// static
+gfx::Rect PPB_Graphics2D_Impl::ScaleRectBounds(const gfx::Rect& rect,
+                                               float scale) {
+  if (scale != 1.0f && scale > 0.0f) {
+    int left = static_cast<int>(floorf(rect.x() * scale));
+    int top = static_cast<int>(floorf(rect.y() * scale));
+    int right = static_cast<int>(ceilf((rect.x() + rect.width()) * scale));
+    int bottom = static_cast<int>(ceilf((rect.y() + rect.height()) * scale));
+    return gfx::Rect(left, top, right - left, bottom - top);
+  }
+
+  return rect;
+}
+
 void PPB_Graphics2D_Impl::ExecutePaintImageData(PPB_ImageData_Impl* image,
                                                 int x, int y,
                                                 const gfx::Rect& src_rect,