Fixed rounding issue on scrollbar rasterization.

cc/layers/painted_scrollbar_layer.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layers/painted_scrollbar_layer.h"
 
 #include "base/auto_reset.h"
 #include "base/basictypes.h"
 #include "base/debug/trace_event.h"
 #include "cc/layers/painted_scrollbar_layer_impl.h"
@@ skipping 152 matching lines @@
   }
 
   ContentsScalingLayer::SetLayerTreeHost(host);
 }
 
 gfx::Rect PaintedScrollbarLayer::ScrollbarLayerRectToContentRect(
     const gfx::Rect& layer_rect) const {
   // 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());
   expanded_rect.set_size(clamped_size);
   return expanded_rect;
 }
 
 gfx::Rect PaintedScrollbarLayer::OriginThumbRect() const {
   gfx::Size thumb_size;
   if (orientation() == HORIZONTAL) {
     thumb_size =
         gfx::Size(scrollbar_->ThumbLength(), scrollbar_->ThumbThickness());
   } else {
     thumb_size =
         gfx::Size(scrollbar_->ThumbThickness(), scrollbar_->ThumbLength());
   }
-  return ScrollbarLayerRectToContentRect(gfx::Rect(thumb_size));
+  return gfx::Rect(thumb_size);
 }
 
 void PaintedScrollbarLayer::UpdateThumbAndTrackGeometry() {
   UpdateProperty(scrollbar_->TrackRect(), &track_rect_);
   UpdateProperty(scrollbar_->Location(), &location_);
   UpdateProperty(scrollbar_->IsOverlay(), &is_overlay_);
   UpdateProperty(scrollbar_->HasThumb(), &has_thumb_);
   if (has_thumb_) {
     UpdateProperty(scrollbar_->ThumbThickness(), &thumb_thickness_);
     UpdateProperty(scrollbar_->ThumbLength(), &thumb_length_);
   }
 }
 
 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;
 
   {
     base::AutoReset<bool> ignore_set_needs_commit(&ignore_set_needs_commit_,
                                                   true);
     ContentsScalingLayer::Update(queue, occlusion);
   }
 
   if (update_rect_.IsEmpty() && track_resource_)
     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.
   SetNeedsPushProperties();
   return true;
 }
 
 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);
   paint.setXfermodeMode(SkXfermode::kClear_Mode);
   skcanvas.drawRect(layer_skrect, paint);
   skcanvas.clipRect(layer_skrect);
 
   scrollbar_->PaintPart(&skcanvas, part, layer_rect);
   // Make sure that the pixels are no longer mutable to unavoid unnecessary
   // allocation and copying.
   skbitmap.setImmutable();
 
   return UIResourceBitmap(skbitmap);
 }
 
 }  // namespace cc