Stop using (deprecated) getTotalClip

skia/ext/skia_utils_mac.mm

 // Copyright (c) 2012 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 "skia/ext/skia_utils_mac.h"
 
 #import <AppKit/AppKit.h>
 
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
@@ skipping 338 matching lines @@
     // Neutralize the global matrix by concatenating the inverse. In the
     // future, Skia may provide some mechanism to set the device portion of
     // the matrix to identity without clobbering any hosting matrix (e.g., the
     // picture's matrix).
     const SkMatrix& skMatrix = canvas_->getTotalMatrix();
     SkMatrix inverse;
     if (!skMatrix.invert(&inverse))
       return;
     canvas_->save();
     canvas_->concat(inverse);
-    canvas_->drawBitmap(subset, bounds.fLeft, bounds.fTop);
+    canvas_->drawBitmap(subset, bounds.x() + bitmapOffset_.x(),
+                                bounds.y() + bitmapOffset_.y());
     canvas_->restore();
   }
   CGContextRelease(cgContext_);
   cgContext_ = 0;
 }
 
 CGContextRef SkiaBitLocker::cgContext() {
+  SkIRect clip_bounds;
+  if (!canvas_->getClipDeviceBounds(&clip_bounds))
+    return 0;  // the clip is empty, nothing to draw
+
   SkBaseDevice* device = canvas_->getTopDevice();
   DCHECK(device);
   if (!device)
     return 0;
+
   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();

[f(malita), 2014/06/19 18:39:54]

        bitmapOffset_.setZero();

[reed1, 2014/06/19 18:57:18]

It won't get read in this case, but I will add the setZero() call.

   } else {
-    if (!bitmap_.allocN32Pixels(deviceBits.width(), deviceBits.height()))
+    if (!bitmap_.allocN32Pixels(clip_bounds.width(), clip_bounds.height()))
       return 0;
     bitmap_.eraseColor(0);
+    bitmapOffset_.set(clip_bounds.x(), clip_bounds.y());
   }
   base::ScopedCFTypeRef<CGColorSpaceRef> colorSpace(
       CGColorSpaceCreateDeviceRGB());
   cgContext_ = CGBitmapContextCreate(bitmap_.getPixels(), bitmap_.width(),
     bitmap_.height(), 8, bitmap_.rowBytes(), colorSpace, 
     kCGBitmapByteOrder32Host | kCGImageAlphaPremultipliedFirst);
 
   // Apply device matrix.
   CGAffineTransform contentsTransform = CGAffineTransformMakeScale(1, -1);
   contentsTransform = CGAffineTransformTranslate(contentsTransform, 0,
       -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.
+  // get clip_bounds into the coordinate system of the top layer/device
+  clip_bounds.offset(-device->getOrigin());
+
   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(clip_bounds));
   }
 
   // Apply content matrix.
   SkMatrix skMatrix = canvas_->getTotalMatrix();
-  skMatrix.postTranslate(-SkIntToScalar(pt.fX), -SkIntToScalar(pt.fY));
+  skMatrix.postTranslate(-SkIntToScalar(clip_bounds.x()),

[f(malita), 2014/06/19 18:39:54]

Do we still need to include the clip offset here when useDeviceBits_ == true?

[reed1, 2014/06/19 18:57:18]

I think so. From CG's perspective, both scenarios are the same -- we just create
a CGContext that points to some of our memory. In both cases, we need to tell CG
the offset between local coordinates and the bits we are pointing at, and
clip_bounds.origin is that offset.

+                         -SkIntToScalar(clip_bounds.y()));
   CGAffineTransform affine = SkMatrixToCGAffineTransform(skMatrix);
   CGContextConcatCTM(cgContext_, affine);
   
   return cgContext_;
 }
 
 }  // namespace gfx