Stop using (deprecated) getTotalClip

skia/ext/skia_utils_mac.mm

Index: skia/ext/skia_utils_mac.mm
diff --git a/skia/ext/skia_utils_mac.mm b/skia/ext/skia_utils_mac.mm
index f67d3c1c02de34ec181e768770ab2db6fab9650c..72b77ec9ac12c8e04cea1f7109049d01592d1f5c 100644
--- a/skia/ext/skia_utils_mac.mm
+++ b/skia/ext/skia_utils_mac.mm
@@ -364,18 +364,30 @@ foundRight:
 }
 
 CGContextRef SkiaBitLocker::cgContext() {
+  SkIRect clipBounds;
+  if (!canvas_->getClipDeviceBounds(&clipBounds))
+    return 0;  // the clip is empty, nothing to draw
+
   SkBaseDevice* device = canvas_->getTopDevice();
   DCHECK(device);
   if (!device)
     return 0;
+
+  const SkIPoint& pt = device->getOrigin();

[f(malita), 2014/06/19 15:15:46]

Nit: maybe inline this since we're only using pt once.

[reed1, 2014/06/19 16:40:54]

We use it down on line 412

+  // get clipbounds into the coordinate systme of the top layer/device

[f(malita), 2014/06/19 15:15:47]

Typo: system

[reed1, 2014/06/19 16:40:54]

Done.

+  clipBounds.offset(-pt);
+
   releaseIfNeeded(); // This flushes any prior bitmap use
   const SkBitmap& deviceBits = device->accessBitmap(true);
-  useDeviceBits_ = deviceBits.getPixels();
+  // Only draw directly if we have pixels, and we're only rect-clipped.
+  // If not, we allocate an offscreen and draw into that, relying on the
+  // compositing step to apply skia's clip.
+  useDeviceBits_ = deviceBits.getPixels() && canvas_->isClipRect();
   if (useDeviceBits_) {
     bitmap_ = deviceBits;
     bitmap_.lockPixels();
   } else {
-    if (!bitmap_.allocN32Pixels(deviceBits.width(), deviceBits.height()))
+    if (!bitmap_.allocN32Pixels(clipBounds.width(), clipBounds.height()))

[f(malita), 2014/06/19 15:15:47]

We used to allocate a bitmap matching the device backing size, and now we limit
it to the visible part/clip rect.

But for this to work, we also need to account for the clip rect offset in a) the
CGContext CTM and b) the compositing phase - no? I *think* the current
assumption is that the CGContext origin == device origin.

[reed1, 2014/06/19 16:40:54]

I think you're right, I will need to adjust the offset during compose. I don't
have to make this change, but it seems smart to limit the size of the allocation
if possible.

       return 0;
     bitmap_.eraseColor(0);
   }
@@ -391,31 +403,8 @@ CGContextRef SkiaBitLocker::cgContext() {
       -device->height());
   CGContextConcatCTM(cgContext_, contentsTransform);
 
-  const SkIPoint& pt = device->getOrigin();
-  // Skip applying the clip when not writing directly to device.
-  // They're applied in the offscreen case when the bitmap is drawn.
   if (useDeviceBits_) {
-      // Apply clip in device coordinates.
-      CGMutablePathRef clipPath = CGPathCreateMutable();
-      const SkRegion& clipRgn = canvas_->getTotalClip();
-      if (clipRgn.isEmpty()) {
-        // CoreGraphics does not consider a newly created path to be empty.
-        // Explicitly set it to empty so the subsequent drawing is clipped out.
-        // It would be better to make the CGContext hidden if there was a CG
-        // call that does that.
-        CGPathAddRect(clipPath, 0, CGRectMake(0, 0, 0, 0));
-      }
-      SkRegion::Iterator iter(clipRgn);
-      const SkIPoint& pt = device->getOrigin();
-      for (; !iter.done(); iter.next()) {
-        SkIRect skRect = iter.rect();
-        skRect.offset(-pt);
-        CGRect cgRect = SkIRectToCGRect(skRect);
-        CGPathAddRect(clipPath, 0, cgRect);
-      }
-      CGContextAddPath(cgContext_, clipPath);
-      CGContextClip(cgContext_);
-      CGPathRelease(clipPath);
+    CGContextClipToRect(cgContext_, SkIRectToCGRect(clipBounds));
   }
 
   // Apply content matrix.