Store SkRRects in SkClipStack

src/core/SkClipStack.cpp

Index: src/core/SkClipStack.cpp
diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp
index c66a219b629bbe7c467eafa946e2487fe067afbe..c9f7be0509d83fc78418f6b145ec35fa535700da 100644
--- a/src/core/SkClipStack.cpp
+++ b/src/core/SkClipStack.cpp
@@ -16,17 +16,90 @@
 static const int32_t kFirstUnreservedGenID = 3;
 int32_t SkClipStack::gGenID = kFirstUnreservedGenID;
 
+bool SkClipStack::Element::operator== (const Element& element) const {
+    if (this == &element) {
+        return true;
+    }
+    if (fOp != element.fOp ||
+        fType != element.fType ||
+        fDoAA != element.fDoAA ||
+        fSaveCount != element.fSaveCount) {
+        return false;
+    }
+    switch (fType) {
+        case kPath_Type:
+            return fPath == element.fPath;
+        case kRRect_Type:
+            return fRRect == element.fRRect;
+        case kRect_Type:
+            return fRect == element.fRect;
+        case kEmpty_Type:
+            return true;
+        default:
+            SkDEBUGFAIL("Unexpected type.");
+            return false;
+    }
+}
+
 void SkClipStack::Element::invertShapeFillType() {
     switch (fType) {
         case kRect_Type:
             fPath.reset();
             fPath.addRect(fRect);
-            fPath.setFillType(SkPath::kInverseWinding_FillType);
+            fPath.setFillType(SkPath::kInverseEvenOdd_FillType);

[robertphillips, 2014/02/14 20:36:01]

Do we really care about setting fRect to be empty?

[bsalomon, 2014/02/14 21:20:57]

I guess not

+            fRect.setEmpty();
+            fType = kPath_Type;
+            break;
+        case kRRect_Type:
+            fPath.reset();
+            fPath.addRRect(fRRect);
+            fPath.setFillType(SkPath::kInverseEvenOdd_FillType);

[robertphillips, 2014/02/14 20:36:01]

Same for rrect?

[bsalomon, 2014/02/14 21:20:57]

Done.

+            fRRect.setEmpty();
             fType = kPath_Type;
             break;
         case kPath_Type:
             fPath.toggleInverseFillType();
         case kEmpty_Type:

[robertphillips, 2014/02/14 20:36:01]

Probably

[bsalomon, 2014/02/14 21:20:57]

Agreed, but don't want to conflate any regressions/changes with the those from
the main thrust of this CL.

+            // Should this set to an empty, inverse filled path?
+            break;
+    }
+}
+
+void SkClipStack::Element::initPath(int saveCount, const SkPath& path, SkRegion::Op op,
+                                    bool doAA) {
+    if (!path.isInverseFillType()) {
+        if (SkPath::kNone_PathAsRect != path.asRect()) {
+            this->initRect(saveCount, path.getBounds(), op, doAA);
+            return;
+        }
+        SkRect ovalRect;
+        if (path.isOval(&ovalRect)) {
+            SkRRect rrect;
+            rrect.setOval(ovalRect);
+            this->initRRect(saveCount, rrect, op, doAA);
+            return;
+        }
+    }
+    fPath = path;
+    fType = kPath_Type;
+    this->initCommon(saveCount, op, doAA);
+}
+
+void SkClipStack::Element::asPath(SkPath* path) const {
+    switch (fType) {
+        case kEmpty_Type:
+            path->reset();
+            break;
+        case kRect_Type:
+            path->reset();
+            path->addRect(fRect);
+            break;
+        case kRRect_Type:
+            path->reset();
+            path->addRRect(fRRect);
+            break;
+        case kPath_Type:
+            *path = fPath;
             break;
     }
 }
@@ -100,10 +173,10 @@ void SkClipStack::Element::combineBoundsDiff(FillCombo combination, const SkRect
             // is erased, so the only pixels that can remain set
             // occur w/in the intersection of the two finite bounds
             if (!fFiniteBound.intersect(prevFinite)) {
-                fFiniteBound.setEmpty();
-                fGenID = kEmptyGenID;
+                this->setEmpty();
+            } else {
+                fFiniteBoundType = kNormal_BoundsType;
             }
-            fFiniteBoundType = kNormal_BoundsType;
             break;
         case kPrev_Cur_FillCombo:
             // The most conservative result bound is that of the
@@ -205,8 +278,7 @@ void SkClipStack::Element::combineBoundsIntersection(int combination, const SkRe
             break;
         case kPrev_Cur_FillCombo:
             if (!fFiniteBound.intersect(prevFinite)) {
-                fFiniteBound.setEmpty();
-                fGenID = kEmptyGenID;
+                this->setEmpty();
             }
             break;
         default:
@@ -228,10 +300,10 @@ void SkClipStack::Element::combineBoundsRevDiff(int combination, const SkRect& p
             break;
         case kInvPrev_Cur_FillCombo:
             if (!fFiniteBound.intersect(prevFinite)) {
-                fFiniteBound.setEmpty();
-                fGenID = kEmptyGenID;
+                this->setEmpty();
+            } else {
+                fFiniteBoundType = kNormal_BoundsType;
             }
-            fFiniteBoundType = kNormal_BoundsType;
             break;
         case kPrev_InvCur_FillCombo:
             fFiniteBound.join(prevFinite);
@@ -258,27 +330,34 @@ void SkClipStack::Element::updateBoundAndGenID(const Element* prior) {
     // First, optimistically update the current Element's bound information
     // with the current clip's bound
     fIsIntersectionOfRects = false;
-    if (kRect_Type == fType) {
-        fFiniteBound = fRect;
-        fFiniteBoundType = kNormal_BoundsType;
-
-        if (SkRegion::kReplace_Op == fOp ||
-            (SkRegion::kIntersect_Op == fOp && NULL == prior) ||
-            (SkRegion::kIntersect_Op == fOp && prior->fIsIntersectionOfRects &&
-                prior->rectRectIntersectAllowed(fRect, fDoAA))) {
-            fIsIntersectionOfRects = true;
-        }
-
-    } else {
-        SkASSERT(kPath_Type == fType);
-
-        fFiniteBound = fPath.getBounds();
+    switch (fType) {
+        case kRect_Type:
+            fFiniteBound = fRect;
+            fFiniteBoundType = kNormal_BoundsType;
 
-        if (fPath.isInverseFillType()) {
-            fFiniteBoundType = kInsideOut_BoundsType;
-        } else {
+            if (SkRegion::kReplace_Op == fOp ||
+                (SkRegion::kIntersect_Op == fOp && NULL == prior) ||
+                (SkRegion::kIntersect_Op == fOp && prior->fIsIntersectionOfRects &&
+                    prior->rectRectIntersectAllowed(fRect, fDoAA))) {
+                fIsIntersectionOfRects = true;
+            }
+            break;
+        case kRRect_Type:
+            fFiniteBound = fRRect.getBounds();
             fFiniteBoundType = kNormal_BoundsType;
-        }
+            break;
+        case kPath_Type:
+            fFiniteBound = fPath.getBounds();
+
+            if (fPath.isInverseFillType()) {
+                fFiniteBoundType = kInsideOut_BoundsType;
+            } else {
+                fFiniteBoundType = kNormal_BoundsType;
+            }
+            break;
+        case kEmpty_Type:
+            SkDEBUGFAIL("We shouldn't get here with an empty element.");
+            break;
     }
 
     if (!fDoAA) {
@@ -344,7 +423,7 @@ void SkClipStack::Element::updateBoundAndGenID(const Element* prior) {
             // so nothing to do
             break;
         default:
-            SkDebugf("SkRegion::Op error/n");
+            SkDebugf("SkRegion::Op error\n");
             SkASSERT(0);
             break;
     }
@@ -528,81 +607,62 @@ bool SkClipStack::quickContains(const SkRect& rect) const {
     return true;
 }
 
-void SkClipStack::clipDevRect(const SkRect& rect, SkRegion::Op op, bool doAA) {
-
+void SkClipStack::pushElement(const Element& element) {
     // Use reverse iterator instead of back because Rect path may need previous
     SkDeque::Iter iter(fDeque, SkDeque::Iter::kBack_IterStart);
-    Element* element = (Element*) iter.prev();
+    Element* prior = (Element*) iter.prev();
 
-    if (NULL != element) {
-        if (element->canBeIntersectedInPlace(fSaveCount, op)) {
-            switch (element->fType) {
+    if (NULL != prior) {
+        if (prior->canBeIntersectedInPlace(fSaveCount, element.getOp())) {
+            switch (prior->fType) {
                 case Element::kEmpty_Type:
-                    element->checkEmpty();
+                    prior->checkEmpty();
                     return;
                 case Element::kRect_Type:
-                    if (element->rectRectIntersectAllowed(rect, doAA)) {
-                        if (!element->fRect.intersect(rect)) {
-                            element->setEmpty();
+                    if (Element::kRect_Type == element.getType()) {
+                        if (prior->rectRectIntersectAllowed(element.getRect(), element.isAA())) {
+                            if (!prior->fRect.intersect(element.getRect())) {
+                                prior->setEmpty();
+                                return;
+                            }
+
+                            prior->fDoAA = element.isAA();
+                            Element* priorPrior = (Element*) iter.prev();
+                            prior->updateBoundAndGenID(priorPrior);
                             return;
                         }
-
-                        element->fDoAA = doAA;
-                        Element* prev = (Element*) iter.prev();
-                        element->updateBoundAndGenID(prev);
-                        return;
+                        break;
                     }
-                    break;
-                case Element::kPath_Type:
-                    if (!SkRect::Intersects(element->fPath.getBounds(), rect)) {
-                        element->setEmpty();
+                    // fallthrough
+                default:
+                    if (!SkRect::Intersects(prior->getBounds(), element.getBounds())) {
+                        prior->setEmpty();
                         return;
                     }
                     break;
             }
-        } else if (SkRegion::kReplace_Op == op) {
+        } else if (SkRegion::kReplace_Op == element.getOp()) {
             this->restoreTo(fSaveCount - 1);
-            element = (Element*) fDeque.back();
+            prior = (Element*) fDeque.back();
         }
     }
-    new (fDeque.push_back()) Element(fSaveCount, rect, op, doAA);
-    ((Element*) fDeque.back())->updateBoundAndGenID(element);
+    Element* newElement = SkNEW_PLACEMENT_ARGS(fDeque.push_back(), Element, (element));
+    newElement->updateBoundAndGenID(prior);
 }
 
-void SkClipStack::clipDevPath(const SkPath& path, SkRegion::Op op, bool doAA) {
-    SkRect alt;
-    if (path.isRect(&alt) && !path.isInverseFillType()) {
-        return this->clipDevRect(alt, op, doAA);
-    }
+void SkClipStack::clipDevRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) {
+    Element element(fSaveCount, rrect, op, doAA);
+    this->pushElement(element);
+}
 
-    Element* element = (Element*)fDeque.back();
-    if (NULL != element) {
-        if (element->canBeIntersectedInPlace(fSaveCount, op)) {
-            const SkRect& pathBounds = path.getBounds();
-            switch (element->fType) {
-                case Element::kEmpty_Type:
-                    element->checkEmpty();
-                    return;
-                case Element::kRect_Type:
-                    if (!SkRect::Intersects(element->fRect, pathBounds)) {
-                        element->setEmpty();
-                        return;
-                    }
-                    break;
-                case Element::kPath_Type:
-                    if (!SkRect::Intersects(element->fPath.getBounds(), pathBounds)) {
-                        element->setEmpty();
-                        return;
-                    }
-                    break;
-            }
-        } else if (SkRegion::kReplace_Op == op) {
-            this->restoreTo(fSaveCount - 1);
-            element = (Element*) fDeque.back();
-        }
-    }
-    new (fDeque.push_back()) Element(fSaveCount, path, op, doAA);
-    ((Element*) fDeque.back())->updateBoundAndGenID(element);
+void SkClipStack::clipDevRect(const SkRect& rect, SkRegion::Op op, bool doAA) {
+    Element element(fSaveCount, rect, op, doAA);
+    this->pushElement(element);
+}
+
+void SkClipStack::clipDevPath(const SkPath& path, SkRegion::Op op, bool doAA) {
+    Element element(fSaveCount, path, op, doAA);
+    this->pushElement(element);
 }
 
 void SkClipStack::clipEmpty() {
@@ -610,12 +670,12 @@ void SkClipStack::clipEmpty() {
     Element* element = (Element*) fDeque.back();
 
     if (element && element->canBeIntersectedInPlace(fSaveCount, SkRegion::kIntersect_Op)) {
+        element->setEmpty();

[robertphillips, 2014/02/14 20:36:01]

Seems like we don't really need this switch any more.

[bsalomon, 2014/02/14 21:20:57]

Done.

         switch (element->fType) {
             case Element::kEmpty_Type:
                 element->checkEmpty();
                 return;
-            case Element::kRect_Type:
-            case Element::kPath_Type:
+            default:
                 element->setEmpty();
                 return;
         }