Fixed rounding issue on scrollbar rasterization.

cc/layers/painted_scrollbar_layer.cc

Index: cc/layers/painted_scrollbar_layer.cc
diff --git a/cc/layers/painted_scrollbar_layer.cc b/cc/layers/painted_scrollbar_layer.cc
index 71a37d48883f8f07d592ff561e288e344917ed65..80e5f1b6523f59e7c20eca8113f98467b9e71ca9 100644
--- a/cc/layers/painted_scrollbar_layer.cc
+++ b/cc/layers/painted_scrollbar_layer.cc
@@ -170,7 +170,7 @@ gfx::Rect PaintedScrollbarLayer::ScrollbarLayerRectToContentRect(
   // Don't intersect with the bounds as in LayerRectToContentRect() because
   // layer_rect here might be in coordinates of the containing layer.
   gfx::Rect expanded_rect = gfx::ScaleToEnclosingRect(
-      layer_rect, contents_scale_y(), contents_scale_y());
+      layer_rect, contents_scale_x(), contents_scale_y());
   // We should never return a rect bigger than the content_bounds().
   gfx::Size clamped_size = expanded_rect.size();
   clamped_size.SetToMin(content_bounds());
@@ -187,7 +187,7 @@ gfx::Rect PaintedScrollbarLayer::OriginThumbRect() const {
     thumb_size =
         gfx::Size(scrollbar_->ThumbThickness(), scrollbar_->ThumbLength());
   }
-  return ScrollbarLayerRectToContentRect(gfx::Rect(thumb_size));
+  return gfx::Rect(thumb_size);
 }
 
 void PaintedScrollbarLayer::UpdateThumbAndTrackGeometry() {
@@ -205,8 +205,9 @@ bool PaintedScrollbarLayer::Update(ResourceUpdateQueue* queue,
                                    const OcclusionTracker* occlusion) {
   UpdateThumbAndTrackGeometry();
 
+  gfx::Rect track_layer_rect = gfx::Rect(location_, bounds());
   gfx::Rect scaled_track_rect = ScrollbarLayerRectToContentRect(
-      gfx::Rect(location_, bounds()));
+      track_layer_rect);
 
   if (track_rect_.IsEmpty() || scaled_track_rect.IsEmpty())
     return false;
@@ -221,12 +222,16 @@ bool PaintedScrollbarLayer::Update(ResourceUpdateQueue* queue,
     return false;
 
   track_resource_ = ScopedUIResource::Create(
-      layer_tree_host(), RasterizeScrollbarPart(scaled_track_rect, TRACK));
+      layer_tree_host(),
+      RasterizeScrollbarPart(track_layer_rect, scaled_track_rect, TRACK));
 
-  gfx::Rect thumb_rect = OriginThumbRect();
-  if (has_thumb_ && !thumb_rect.IsEmpty()) {
+  gfx::Rect thumb_layer_rect = OriginThumbRect();
+  gfx::Rect scaled_thumb_rect =
+      ScrollbarLayerRectToContentRect(thumb_layer_rect);
+  if (has_thumb_ && !scaled_thumb_rect.IsEmpty()) {
     thumb_resource_ = ScopedUIResource::Create(
-        layer_tree_host(), RasterizeScrollbarPart(thumb_rect, THUMB));
+        layer_tree_host(),
+        RasterizeScrollbarPart(thumb_layer_rect, scaled_thumb_rect, THUMB));
   }
 
   // UI resources changed so push properties is needed.
@@ -235,21 +240,26 @@ bool PaintedScrollbarLayer::Update(ResourceUpdateQueue* queue,
 }
 
 UIResourceBitmap PaintedScrollbarLayer::RasterizeScrollbarPart(
+    const gfx::Rect& layer_rect,
     const gfx::Rect& rect,
     ScrollbarPart part) {
   DCHECK(!rect.size().IsEmpty());
+  DCHECK(!layer_rect.size().IsEmpty());
 
   SkBitmap skbitmap;
   skbitmap.setConfig(SkBitmap::kARGB_8888_Config, rect.width(), rect.height());
   skbitmap.allocPixels();
 
   SkCanvas skcanvas(skbitmap);
-  skcanvas.translate(SkFloatToScalar(-rect.x()), SkFloatToScalar(-rect.y()));
+
+  // Skia will round to the nearest pixel so we need to translate by the
+  // rounded origin coordinates

[danakj, 2014/02/13 19:16:22]

Can you explain this logic? I was expecting you want change the scale slightly
so that we draw to fill the skcanvas bitmap fully.

Here it seems like you're instead just shifting the result left to the origin.
But then we'll have a partial pixel on the right of the canvas that isn't
painted still? How come that's okay?

[bokan, 2014/02/14 20:18:27]

So it looked fine in practice, and I thought it was because the cases where we
rounded were also the cases where we clamping the scaled_rect to content_bounds
removed the last column from the bitmap. However, when I tried to prove it to
myself I found that occasionally that's not true, and examining the memory in
the bitmap confirmed that the right side was in fact clipped (though it's not
visible, maybe the window border overlaps it?).

tldr; I now scale the canvas such that the width/height is exactly the bitmap
width/height and translate accordingly. Test is also better since it checks that
all four corners are actually painted.

+  float roundedX = round(layer_rect.x() * contents_scale_x());

[danakj, 2014/02/13 19:16:22]

rounded_x

[bokan, 2014/02/14 20:18:27]

Done.

+  float roundedY = round(layer_rect.y() * contents_scale_y());

[danakj, 2014/02/13 19:16:22]

rounded_y

[bokan, 2014/02/14 20:18:27]

Done.

+  skcanvas.translate(SkFloatToScalar(-roundedX), SkFloatToScalar(-roundedY));
   skcanvas.scale(SkFloatToScalar(contents_scale_x()),
                  SkFloatToScalar(contents_scale_y()));
 
-  gfx::Rect layer_rect = gfx::ScaleToEnclosingRect(
-      rect, 1.f / contents_scale_x(), 1.f / contents_scale_y());
   SkRect layer_skrect = RectToSkRect(layer_rect);
   SkPaint paint;
   paint.setAntiAlias(false);