Moving updateClipConservativelyUsingBounds into SkCanvas

This method is being moved from SkPictureRecord to become a protected member of SkCanvas so that other classes derived from SkCanvas can use it.
This Patch also applies the fix provided by updateClipConservativelyUsingBounds to SkPictureUtils::GatherPixelRefs

BUG=https://code.google.com/p/chromium/issues/detail?id=244893

Review URL: https://codereview.chromium.org/15894005

git-svn-id: http://skia.googlecode.com/svn/trunk@9326 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 76 insertions, 74 deletions

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index e26007fa28..d8fd0a4481 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1279,6 +1279,80 @@ bool SkCanvas::clipPath(const SkPath& path, SkRegion::Op op, bool doAA) {
     return clipPathHelper(this, fMCRec->fRasterClip, devPath, op, doAA);
 }
 
+bool SkCanvas::updateClipConservativelyUsingBounds(const SkRect& bounds, SkRegion::Op op,
+                                                   bool inverseFilled) {
+    // This is for updating the clip conservatively using only bounds
+    // information.
+    // Contract:
+    //    The current clip must contain the true clip. The true
+    //    clip is the clip that would have normally been computed
+    //    by calls to clipPath and clipRRect
+    // Objective:
+    //    Keep the current clip as small as possible without
+    //    breaking the contract, using only clip bounding rectangles
+    //    (for performance).
+
+    // N.B.: This *never* calls back through a virtual on canvas, so subclasses
+    // don't have to worry about getting caught in a loop. Thus anywhere
+    // we call a virtual method, we explicitly prefix it with
+    // SkCanvas:: to be sure to call the base-class.
+                                                   
+    if (inverseFilled) {
+        switch (op) {
+            case SkRegion::kIntersect_Op:
+            case SkRegion::kDifference_Op:
+                // These ops can only shrink the current clip. So leaving
+                // the clip unchanges conservatively respects the contract.
+                return this->getClipDeviceBounds(NULL);
+            case SkRegion::kUnion_Op:
+            case SkRegion::kReplace_Op:
+            case SkRegion::kReverseDifference_Op:
+            case SkRegion::kXOR_Op:
+                {
+                    // These ops can grow the current clip up to the extents of
+                    // the input clip, which is inverse filled, so we just set
+                    // the current clip to the device bounds.
+                    SkRect deviceBounds;
+                    SkIRect deviceIBounds;
+                    this->getDevice()->getGlobalBounds(&deviceIBounds);
+                    deviceBounds = SkRect::MakeFromIRect(deviceIBounds);
+                    this->SkCanvas::save(SkCanvas::kMatrix_SaveFlag);
+                    // set the clip in device space
+                    this->SkCanvas::setMatrix(SkMatrix::I());
+                    bool result = this->SkCanvas::clipRect(deviceBounds,
+                        SkRegion::kReplace_Op, false);
+                    this->SkCanvas::restore(); //pop the matrix, but keep the clip
+                    return result;
+                }
+            default:
+                SkASSERT(0); // unhandled op?
+        }
+    } else {
+        // Not inverse filled
+        switch (op) {
+            case SkRegion::kIntersect_Op:
+            case SkRegion::kUnion_Op:
+            case SkRegion::kReplace_Op:
+                return this->SkCanvas::clipRect(bounds, op, false);
+            case SkRegion::kDifference_Op:
+                // Difference can only shrink the current clip.
+                // Leaving clip unchanged conservatively fullfills the contract.
+                return this->getClipDeviceBounds(NULL);
+            case SkRegion::kReverseDifference_Op:
+                // To reverse, we swap in the bounds with a replace op.
+                // As with difference, leave it unchanged.
+                return this->SkCanvas::clipRect(bounds, SkRegion::kReplace_Op, false);
+            case SkRegion::kXOR_Op:
+                // Be conservative, based on (A XOR B) always included in (A union B),
+                // which is always included in (bounds(A) union bounds(B))
+                return this->SkCanvas::clipRect(bounds, SkRegion::kUnion_Op, false);
+            default:
+                SkASSERT(0); // unhandled op?
+        }
+    }
+    return true;
+}
+
 bool SkCanvas::clipRegion(const SkRegion& rgn, SkRegion::Op op) {
     AutoValidateClip avc(this);
 
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index fc8502823d..915736373d 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -759,7 +759,7 @@ bool SkPictureRecord::clipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA
     validate(initialOffset, size);
 
     if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
-        return this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, doAA, false);
+        return this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, false);
     } else {
         return this->INHERITED::clipRRect(rrect, op, doAA);
     }
@@ -787,83 +787,13 @@ bool SkPictureRecord::clipPath(const SkPath& path, SkRegion::Op op, bool doAA) {
     validate(initialOffset, size);
 
     if (fRecordFlags & SkPicture::kUsePathBoundsForClip_RecordingFlag) {
-        return this->updateClipConservativelyUsingBounds(path.getBounds(), op, doAA,
+        return this->updateClipConservativelyUsingBounds(path.getBounds(), op,
                                                          path.isInverseFillType());
     } else {
         return this->INHERITED::clipPath(path, op, doAA);
     }
 }
 
-bool SkPictureRecord::updateClipConservativelyUsingBounds(const SkRect& bounds, SkRegion::Op op,
-                                                          bool doAA, bool inverseFilled) {
-    // This is for updating the clip when kUsePathBoundsForClip_RecordingFlag
-    // is set. The current clip of the recording canvas is used for quick
-    // culling of clipped-out primitives, which must not yield any false
-    // positives, while still rejecting as many as possible.
-    // Contract:
-    //    The current clip must contain the true clip. The true
-    //    clip is the clip that would have been computed with
-    //    kUsePathBoundsForClip_RecordingFlag disabled.
-    // Objective:
-    //    Keep the current clip as small as possible without
-    //    breaking the contract, using only clip bounding rectangles
-    //    (for performance).
-    if (inverseFilled) {
-        switch (op) {
-            case SkRegion::kIntersect_Op:
-            case SkRegion::kDifference_Op:
-                // These ops can only shrink the current clip. So leaving
-                // the clip unchanges conservatively respects the contract.
-                return this->getClipDeviceBounds(NULL);
-            case SkRegion::kUnion_Op:
-            case SkRegion::kReplace_Op:
-            case SkRegion::kReverseDifference_Op:
-            case SkRegion::kXOR_Op:
-                {
-                    // These ops can grow the current clip up to the extents of
-                    // the input clip, which is inverse filled, so we just set
-                    // the current clip to the device bounds.
-                    SkRect deviceBounds;
-                    SkIRect deviceIBounds;
-                    this->getDevice()->getGlobalBounds(&deviceIBounds);
-                    deviceBounds = SkRect::MakeFromIRect(deviceIBounds);
-                    this->INHERITED::save(SkCanvas::kMatrix_SaveFlag);
-                    // set the clip in device space
-                    this->INHERITED::setMatrix(SkMatrix::I());
-                    bool result = this->INHERITED::clipRect(deviceBounds,
-                        SkRegion::kReplace_Op, doAA);
-                    this->INHERITED::restore(); //pop the matrix, but keep the clip
-                    return result;
-                }
-            default:
-                SkASSERT(0); // unhandled op?
-        }
-    } else {
-        // Not inverse filled
-        switch (op) {
-            case SkRegion::kIntersect_Op:
-            case SkRegion::kUnion_Op:
-            case SkRegion::kReplace_Op:
-                return this->INHERITED::clipRect(bounds, op, doAA);
-            case SkRegion::kDifference_Op:
-                // Difference can only shrink the current clip.
-                // Leaving clip unchanged conservatively fullfills the contract.
-                return this->getClipDeviceBounds(NULL);
-            case SkRegion::kReverseDifference_Op:
-                // To reverse, we swap in the bounds with a replace op.
-                // As with difference, leave it unchanged.
-                return this->INHERITED::clipRect(bounds, SkRegion::kReplace_Op, doAA);
-            case SkRegion::kXOR_Op:
-                // Be conservative, based on (A XOR B) always included in (A union B),
-                // which is always included in (bounds(A) union bounds(B))
-                return this->INHERITED::clipRect(bounds, SkRegion::kUnion_Op, doAA);
-            default:
-                SkASSERT(0); // unhandled op?
-        }
-    }
-    return true;
-}
-
 bool SkPictureRecord::clipRegion(const SkRegion& region, SkRegion::Op op) {
     // op + region index + clip params
     uint32_t size = 3 * kUInt32Size;
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index 95ce5d26cb..0ea857d4e5 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -106,8 +106,6 @@ public:
     void endRecording();
 
 private:
-    bool updateClipConservativelyUsingBounds(const SkRect&, SkRegion::Op,
-                                             bool doAA, bool inverseFilled);
     void handleOptimization(int opt);
     void recordRestoreOffsetPlaceholder(SkRegion::Op);
     void fillRestoreOffsetPlaceholdersForCurrentStackLevel(