Move clip CTM application to SkRasterClip and SkClipStack

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2866

Change-Id: I914a57d6ba128acc457e12586c99ba6766eb940c
Reviewed-on: https://skia-review.googlesource.com/2866
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>

6 files changed, 103 insertions, 134 deletions

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index c55ca10f77..7db8601846 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1545,42 +1545,12 @@ void SkCanvas::clipRect(const SkRect& rect, ClipOp op, bool doAA) {
 }
 
 void SkCanvas::onClipRect(const SkRect& rect, ClipOp op, ClipEdgeStyle edgeStyle) {
-    const bool isScaleTrans = fMCRec->fMatrix.isScaleTranslate();
-    SkRect devR;
-    if (isScaleTrans) {
-        fMCRec->fMatrix.mapRectScaleTranslate(&devR, rect);
-    }
-
-    if (kIntersect_Op == op && kHard_ClipEdgeStyle == edgeStyle && isScaleTrans) {
-        if (devR.round().contains(fMCRec->fRasterClip.getBounds())) {
-#if 0
-            SkDebugf("------- ignored clipRect [%g %g %g %g]\n",
-                     rect.left(), rect.top(), rect.right(), rect.bottom());
-#endif
-            return;
-        }
-    }
-
+    const bool isAA = kSoft_ClipEdgeStyle == edgeStyle;
     AutoValidateClip avc(this);
-
+    fClipStack->clipRect(rect, fMCRec->fMatrix, op, isAA);
+    fMCRec->fRasterClip.op(rect, fMCRec->fMatrix, this->getTopLayerBounds(), (SkRegion::Op)op,
+                           isAA);
     fDeviceCMDirty = true;
-
-    if (isScaleTrans) {
-        const bool isAA = kSoft_ClipEdgeStyle == edgeStyle;
-        fClipStack->clipDevRect(devR, op, isAA);
-        fMCRec->fRasterClip.op(devR, this->getTopLayerBounds(), (SkRegion::Op)op, isAA);
-    } else {
-        // since we're rotated or some such thing, we convert the rect to a path
-        // and clip against that, since it can handle any matrix. However, to
-        // avoid recursion in the case where we are subclassed (e.g. Pictures)
-        // we explicitly call "our" version of clipPath.
-        SkPath  path;
-
-        path.addRect(rect);
-        path.setIsVolatile(true);
-        this->SkCanvas::onClipPath(path, op, edgeStyle);
-    }
-
     fDeviceClipBounds = qr_clip_bounds(fMCRec->fRasterClip.getBounds());
 }
 
@@ -1595,25 +1565,16 @@ void SkCanvas::clipRRect(const SkRRect& rrect, ClipOp op, bool doAA) {
 }
 
 void SkCanvas::onClipRRect(const SkRRect& rrect, ClipOp op, ClipEdgeStyle edgeStyle) {
-    SkRRect transformedRRect;
-    if (rrect.transform(fMCRec->fMatrix, &transformedRRect)) {
-        AutoValidateClip avc(this);
-
-        fDeviceCMDirty = true;
-
-        fClipStack->clipDevRRect(transformedRRect, op, kSoft_ClipEdgeStyle == edgeStyle);
+    AutoValidateClip avc(this);
 
-        fMCRec->fRasterClip.op(transformedRRect, this->getTopLayerBounds(), (SkRegion::Op)op,
-                               kSoft_ClipEdgeStyle == edgeStyle);
-        fDeviceClipBounds = qr_clip_bounds(fMCRec->fRasterClip.getBounds());
-        return;
-    }
+    fDeviceCMDirty = true;
 
-    SkPath path;
-    path.addRRect(rrect);
-    path.setIsVolatile(true);
-    // call the non-virtual version
-    this->SkCanvas::onClipPath(path, op, edgeStyle);
+    bool isAA = kSoft_ClipEdgeStyle == edgeStyle;
+    fClipStack->clipRRect(rrect, fMCRec->fMatrix, op, isAA);
+    fMCRec->fRasterClip.op(rrect, fMCRec->fMatrix, this->getTopLayerBounds(), (SkRegion::Op)op,
+                           isAA);
+    fDeviceClipBounds = qr_clip_bounds(fMCRec->fRasterClip.getBounds());
+    return;
 }
 
 void SkCanvas::clipPath(const SkPath& path, ClipOp op, bool doAA) {
@@ -1645,38 +1606,21 @@ void SkCanvas::onClipPath(const SkPath& path, ClipOp op, ClipEdgeStyle edgeStyle
     AutoValidateClip avc(this);
 
     fDeviceCMDirty = true;
+    bool isAA = kSoft_ClipEdgeStyle == edgeStyle;
 
-    SkPath devPath;
-    if (fMCRec->fMatrix.isIdentity()) {
-        devPath = path;
-    } else {
-        path.transform(fMCRec->fMatrix, &devPath);
-        devPath.setIsVolatile(true);
-    }
-
-    // Check if the transfomation, or the original path itself
-    // made us empty. Note this can also happen if we contained NaN
-    // values. computing the bounds detects this, and will set our
-    // bounds to empty if that is the case. (see SkRect::set(pts, count))
-    if (devPath.getBounds().isEmpty()) {
-        // resetting the path will remove any NaN or other wanky values
-        // that might upset our scan converter.
-        devPath.reset();
-    }
-
-    // if we called path.swap() we could avoid a deep copy of this path
-    fClipStack->clipDevPath(devPath, op, kSoft_ClipEdgeStyle == edgeStyle);
+    fClipStack->clipPath(path, fMCRec->fMatrix, op, isAA);
 
+    const SkPath* rasterClipPath = &path;
+    const SkMatrix* matrix = &fMCRec->fMatrix;
+    SkPath tempPath;
     if (fAllowSimplifyClip) {
-        bool clipIsAA = getClipStack()->asPath(&devPath);
-        if (clipIsAA) {
-            edgeStyle = kSoft_ClipEdgeStyle;
-        }
-
+        isAA = getClipStack()->asPath(&tempPath);
+        rasterClipPath = &tempPath;
+        matrix = &SkMatrix::I();
         op = kReplace_Op;
     }
-
-    fMCRec->fRasterClip.op(devPath, this->getTopLayerBounds(), (SkRegion::Op)op, edgeStyle);
+    fMCRec->fRasterClip.op(*rasterClipPath, *matrix, this->getTopLayerBounds(), (SkRegion::Op)op,
+                           isAA);
     fDeviceClipBounds = qr_clip_bounds(fMCRec->fRasterClip.getBounds());
 }
 
@@ -1725,8 +1669,8 @@ void SkCanvas::validateClip() const {
             default: {
                 SkPath path;
                 element->asPath(&path);
-                tmpClip.op(path, this->getTopLayerBounds(), (SkRegion::Op)element->getOp(),
-                           element->isAA());
+                tmpClip.op(path, SkMatrix::I(), this->getTopLayerBounds(),
+                           (SkRegion::Op)element->getOp(), element->isAA());
                 break;
             }
         }
diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp
index 0019077804..f155b49c4f 100644
--- a/src/core/SkClipStack.cpp
+++ b/src/core/SkClipStack.cpp
@@ -503,24 +503,6 @@ SkClipStack::SkClipStack(const SkClipStack& b)
     *this = b;
 }
 
-SkClipStack::SkClipStack(const SkRect& r)
-    : fDeque(sizeof(Element), kDefaultElementAllocCnt)
-    , fSaveCount(0) {
-    if (!r.isEmpty()) {
-        this->clipDevRect(r, SkCanvas::kReplace_Op, false);
-    }
-}
-
-SkClipStack::SkClipStack(const SkIRect& r)
-    : fDeque(sizeof(Element), kDefaultElementAllocCnt)
-    , fSaveCount(0) {
-    if (!r.isEmpty()) {
-        SkRect temp;
-        temp.set(r);
-        this->clipDevRect(temp, SkCanvas::kReplace_Op, false);
-    }
-}
-
 SkClipStack::~SkClipStack() {
     reset();
 }
@@ -745,18 +727,41 @@ void SkClipStack::pushElement(const Element& element) {
     newElement->updateBoundAndGenID(prior);
 }
 
-void SkClipStack::clipDevRRect(const SkRRect& rrect, SkCanvas::ClipOp op, bool doAA) {
-    Element element(fSaveCount, rrect, op, doAA);
-    this->pushElement(element);
+void SkClipStack::clipRRect(const SkRRect& rrect, const SkMatrix& matrix, SkCanvas::ClipOp op,
+                            bool doAA) {
+    SkRRect transformedRRect;
+    if (rrect.transform(matrix, &transformedRRect)) {
+        Element element(fSaveCount, transformedRRect, op, doAA);
+        this->pushElement(element);
+        return;
+    }
+    SkPath path;
+    path.addRRect(rrect);
+    path.setIsVolatile(true);
+    this->clipPath(path, matrix, op, doAA);
 }
 
-void SkClipStack::clipDevRect(const SkRect& rect, SkCanvas::ClipOp op, bool doAA) {
-    Element element(fSaveCount, rect, op, doAA);
-    this->pushElement(element);
+void SkClipStack::clipRect(const SkRect& rect, const SkMatrix& matrix, SkCanvas::ClipOp op,
+                           bool doAA) {
+    if (matrix.rectStaysRect()) {
+        SkRect devRect;
+        matrix.mapRect(&devRect, rect);
+        Element element(fSaveCount, devRect, op, doAA);
+        this->pushElement(element);
+        return;
+    }
+    SkPath path;
+    path.addRect(rect);
+    path.setIsVolatile(true);
+    this->clipPath(path, matrix, op, doAA);
 }
 
-void SkClipStack::clipDevPath(const SkPath& path, SkCanvas::ClipOp op, bool doAA) {
-    Element element(fSaveCount, path, op, doAA);
+void SkClipStack::clipPath(const SkPath& path, const SkMatrix& matrix, SkCanvas::ClipOp op,
+                           bool doAA) {
+    SkPath devPath;
+    path.transform(matrix, &devPath);
+
+    Element element(fSaveCount, devPath, op, doAA);
     this->pushElement(element);
 }
 
diff --git a/src/core/SkRasterClip.cpp b/src/core/SkRasterClip.cpp
index 88bfbafc36..1090c66f34 100644
--- a/src/core/SkRasterClip.cpp
+++ b/src/core/SkRasterClip.cpp
@@ -185,18 +185,20 @@ bool SkRasterClip::setPath(const SkPath& path, const SkRegion& clip, bool doAA)
     return this->updateCacheAndReturnNonEmpty();
 }
 
-bool SkRasterClip::op(const SkRRect& rrect, const SkIRect& bounds, SkRegion::Op op, bool doAA) {
+bool SkRasterClip::op(const SkRRect& rrect, const SkMatrix& matrix, const SkIRect& bounds,
+                      SkRegion::Op op, bool doAA) {
     if (fForceConservativeRects) {
-        return this->op(rrect.getBounds(), bounds, op, doAA);
+        return this->op(rrect.getBounds(), matrix, bounds, op, doAA);
     }
 
     SkPath path;
     path.addRRect(rrect);
 
-    return this->op(path, bounds, op, doAA);
+    return this->op(path, matrix, bounds, op, doAA);
 }
 
-bool SkRasterClip::op(const SkPath& path, const SkIRect& bounds, SkRegion::Op op, bool doAA) {
+bool SkRasterClip::op(const SkPath& path, const SkMatrix& matrix, const SkIRect& bounds,
+                      SkRegion::Op op, bool doAA) {
     AUTO_RASTERCLIP_VALIDATE(*this);
 
     if (fForceConservativeRects) {
@@ -207,9 +209,12 @@ bool SkRasterClip::op(const SkPath& path, const SkIRect& bounds, SkRegion::Op op
             case kReplaceClippedAgainstGlobalBounds_MutateResult:
                 ir = bounds;
                 break;
-            case kContinue_MutateResult:
-                ir = path.getBounds().roundOut();
+            case kContinue_MutateResult: {
+                SkRect bounds = path.getBounds();
+                matrix.mapRect(&bounds);
+                ir = bounds.roundOut();
                 break;
+            }
         }
         return this->op(ir, op);
     }
@@ -218,6 +223,13 @@ bool SkRasterClip::op(const SkPath& path, const SkIRect& bounds, SkRegion::Op op
     // region that results from scan converting devPath.
     SkRegion base;
 
+    SkPath devPath;
+    if (matrix.isIdentity()) {
+        devPath = path;
+    } else {
+        path.transform(matrix, &devPath);
+        devPath.setIsVolatile(true);
+    }
     if (SkRegion::kIntersect_Op == op) {
         // since we are intersect, we can do better (tighter) with currRgn's
         // bounds, than just using the device. However, if currRgn is complex,
@@ -226,21 +238,21 @@ bool SkRasterClip::op(const SkPath& path, const SkIRect& bounds, SkRegion::Op op
             // FIXME: we should also be able to do this when this->isBW(),
             // but relaxing the test above triggers GM asserts in
             // SkRgnBuilder::blitH(). We need to investigate what's going on.
-            return this->setPath(path, this->bwRgn(), doAA);
+            return this->setPath(devPath, this->bwRgn(), doAA);
         } else {
             base.setRect(this->getBounds());
             SkRasterClip clip(fForceConservativeRects);
-            clip.setPath(path, base, doAA);
+            clip.setPath(devPath, base, doAA);
             return this->op(clip, op);
         }
     } else {
         base.setRect(bounds);
 
         if (SkRegion::kReplace_Op == op) {
-            return this->setPath(path, base, doAA);
+            return this->setPath(devPath, base, doAA);
         } else {
             SkRasterClip clip(fForceConservativeRects);
-            clip.setPath(path, base, doAA);
+            clip.setPath(devPath, base, doAA);
             return this->op(clip, op);
         }
     }
@@ -309,8 +321,10 @@ static bool nearly_integral(SkScalar x) {
     return x - SkScalarFloorToScalar(x) < domain;
 }
 
-bool SkRasterClip::op(const SkRect& r, const SkIRect& bounds, SkRegion::Op op, bool doAA) {
+bool SkRasterClip::op(const SkRect& localRect, const SkMatrix& matrix, const SkIRect& bounds,
+                      SkRegion::Op op, bool doAA) {
     AUTO_RASTERCLIP_VALIDATE(*this);
+    SkRect devRect;
 
     if (fForceConservativeRects) {
         SkIRect ir;
@@ -321,30 +335,40 @@ bool SkRasterClip::op(const SkRect& r, const SkIRect& bounds, SkRegion::Op op, b
                 ir = bounds;
                 break;
             case kContinue_MutateResult:
-                ir = r.roundOut();
+                matrix.mapRect(&devRect, localRect);
+                ir = devRect.roundOut();
                 break;
         }
         return this->op(ir, op);
     }
+    const bool isScaleTrans = matrix.isScaleTranslate();
+    if (!isScaleTrans) {
+        SkPath path;
+        path.addRect(localRect);
+        path.setIsVolatile(true);
+        return this->op(path, matrix, bounds, op, doAA);
+    }
+
+    matrix.mapRect(&devRect, localRect);
 
     if (fIsBW && doAA) {
         // check that the rect really needs aa, or is it close enought to
         // integer boundaries that we can just treat it as a BW rect?
-        if (nearly_integral(r.fLeft) && nearly_integral(r.fTop) &&
-            nearly_integral(r.fRight) && nearly_integral(r.fBottom)) {
+        if (nearly_integral(devRect.fLeft) && nearly_integral(devRect.fTop) &&
+            nearly_integral(devRect.fRight) && nearly_integral(devRect.fBottom)) {
             doAA = false;
         }
     }
 
     if (fIsBW && !doAA) {
         SkIRect ir;
-        r.round(&ir);
+        devRect.round(&ir);
         (void)fBW.op(ir, op);
     } else {
         if (fIsBW) {
             this->convertToAA();
         }
-        (void)fAA.op(r, op, doAA);
+        (void)fAA.op(devRect, op, doAA);
     }
     return this->updateCacheAndReturnNonEmpty();
 }
diff --git a/src/core/SkRasterClip.h b/src/core/SkRasterClip.h
index 14d34fdaf0..4b462479c3 100644
--- a/src/core/SkRasterClip.h
+++ b/src/core/SkRasterClip.h
@@ -62,9 +62,9 @@ public:
 
     bool op(const SkIRect&, SkRegion::Op);
     bool op(const SkRegion&, SkRegion::Op);
-    bool op(const SkRect&, const SkIRect&, SkRegion::Op, bool doAA);
-    bool op(const SkRRect&, const SkIRect&, SkRegion::Op, bool doAA);
-    bool op(const SkPath&, const SkIRect&, SkRegion::Op, bool doAA);
+    bool op(const SkRect&, const SkMatrix& matrix, const SkIRect&, SkRegion::Op, bool doAA);
+    bool op(const SkRRect&, const SkMatrix& matrix, const SkIRect&, SkRegion::Op, bool doAA);
+    bool op(const SkPath&, const SkMatrix& matrix, const SkIRect&, SkRegion::Op, bool doAA);
 
     void translate(int dx, int dy, SkRasterClip* dst) const;
     void translate(int dx, int dy) {
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index a002120738..f13b4bc1f6 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -1637,10 +1637,9 @@ void SkPDFDevice::handlePathAnnotation(const SkPath& path,
         return;
     }
 
-    SkPath transformedPath = path;
-    transformedPath.transform(*d.fMatrix);
     SkRasterClip clip = *d.fRC;
-    clip.op(transformedPath, SkIRect::MakeWH(width(), height()), SkRegion::kIntersect_Op,
+    clip.op(path, *d.fMatrix, SkIRect::MakeWH(width(), height()),
+            SkRegion::kIntersect_Op,
             false);
     SkRect transformedRect = SkRect::Make(clip.getBounds());
 
@@ -1761,8 +1760,7 @@ SkPDFDevice::ContentEntry* SkPDFDevice::setUpContentEntry(const SkClipStack* cli
             synthesizedClipStack = fExistingClipStack;
             SkPath clipPath;
             clipRegion.getBoundaryPath(&clipPath);
-            synthesizedClipStack.clipDevPath(clipPath, SkCanvas::kReplace_Op,
-                                             false);
+            synthesizedClipStack.clipPath(clipPath, SkMatrix::I(), SkCanvas::kReplace_Op, false);
             clipStack = &synthesizedClipStack;
         }
     }
diff --git a/src/svg/SkSVGDevice.cpp b/src/svg/SkSVGDevice.cpp
index 99e824657f..16e2b3f7b5 100644
--- a/src/svg/SkSVGDevice.cpp
+++ b/src/svg/SkSVGDevice.cpp
@@ -726,11 +726,9 @@ void SkSVGDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bm, const S
 
     SkClipStack adjustedClipStack;
     if (srcOrNull && *srcOrNull != SkRect::Make(bm.bounds())) {
-        SkRect devClipRect;
-        draw.fMatrix->mapRect(&devClipRect, dst);
-
         adjustedClipStack = *draw.fClipStack;
-        adjustedClipStack.clipDevRect(devClipRect, SkCanvas::kIntersect_Op, paint.isAntiAlias());
+        adjustedClipStack.clipRect(dst, *draw.fMatrix, SkCanvas::kIntersect_Op,
+                                   paint.isAntiAlias());
         adjustedDraw.fClipStack = &adjustedClipStack;
     }