Made clarifying renamings to SkClipStack's iterators (and added to unit test)

http://codereview.appspot.com/6423051/

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

diff --git a/include/core/SkClipStack.h b/include/core/SkClipStack.h
index 60c4421..daeb826 100644 (file)
--- a/include/core/SkClipStack.h
+++ b/include/core/SkClipStack.h
@@ -45,8 +45,8 @@ public:
     class Iter {
     public:
         enum IterStart {
-            kFront_IterStart = SkDeque::Iter::kFront_IterStart,
-            kBack_IterStart = SkDeque::Iter::kBack_IterStart
+            kBottom_IterStart = SkDeque::Iter::kFront_IterStart,
+            kTop_IterStart = SkDeque::Iter::kBack_IterStart
         };
 
         /**
@@ -80,11 +80,11 @@ public:
         const Clip* prev();
 
         /**
-         * Moves the iterator to the last clip with the specified RegionOp 
+         * Moves the iterator to the topmost clip with the specified RegionOp 
          * and returns that clip. If no clip with that op is found, 
          * returns NULL.
          */
-        const Clip* skipToLast(SkRegion::Op op);
+        const Clip* skipToTopmost(SkRegion::Op op);
 
         /**
          * Restarts the iterator on a clip stack.
@@ -103,17 +103,20 @@ public:
         const Clip* updateClip(const SkClipStack::Rec* rec);
     };
 
-    // Inherit privately from Iter to prevent access to reverse iteration
-    class B2FIter : private Iter {
+    /** 
+     * The B2TIter iterates from the bottom of the stack to the top.
+     * It inherits privately from Iter to prevent access to reverse iteration.
+     */
+    class B2TIter : private Iter {
     public:
-        B2FIter() {}
+        B2TIter() {}
 
         /**
          * Wrap Iter's 2 parameter ctor to force initialization to the 
-         * beginning of the deque
+         * beginning of the deque/bottom of the stack
          */
-        B2FIter(const SkClipStack& stack) 
-        : INHERITED(stack, kFront_IterStart) {
+        B2TIter(const SkClipStack& stack) 
+        : INHERITED(stack, kBottom_IterStart) {
         }
 
         using Iter::Clip;
@@ -121,10 +124,10 @@ public:
 
         /**
          * Wrap Iter::reset to force initialization to the 
-         * beginning of the deque
+         * beginning of the deque/bottom of the stack
          */
         void reset(const SkClipStack& stack) {
-            this->INHERITED::reset(stack, kFront_IterStart);
+            this->INHERITED::reset(stack, kBottom_IterStart);
         }
 
     private:

diff --git a/include/gpu/SkGr.h b/include/gpu/SkGr.h
index 975a1ba..ea79c63 100644 (file)
--- a/include/gpu/SkGr.h
+++ b/include/gpu/SkGr.h
@@ -127,10 +127,10 @@ public:
 
 private:
     const SkClipStack*                  fClipStack;
-    SkClipStack::B2FIter                fIter;
+    SkClipStack::B2TIter                fIter;
     // SkClipStack's auto advances on each get
     // so we store the current pos here.
-    const SkClipStack::B2FIter::Clip*   fCurr;
+    const SkClipStack::B2TIter::Clip*   fCurr;
 };
 
 class SkGlyphCache;

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 567e5f5..4b0c1d9 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1212,8 +1212,8 @@ void SkCanvas::validateClip() const {
     ir.set(0, 0, device->width(), device->height());
     SkRasterClip tmpClip(ir);
 
-    SkClipStack::B2FIter                iter(fClipStack);
-    const SkClipStack::B2FIter::Clip*   clip;
+    SkClipStack::B2TIter                iter(fClipStack);
+    const SkClipStack::B2TIter::Clip*   clip;
     while ((clip = iter.next()) != NULL) {
         if (clip->fPath) {
             clipPathHelper(this, &tmpClip, *clip->fPath, clip->fOp, clip->fDoAA);
@@ -1234,8 +1234,8 @@ void SkCanvas::validateClip() const {
 #endif
 
 void SkCanvas::replayClips(ClipVisitor* visitor) const {
-    SkClipStack::B2FIter                iter(fClipStack);
-    const SkClipStack::B2FIter::Clip*   clip;
+    SkClipStack::B2TIter                iter(fClipStack);
+    const SkClipStack::B2TIter::Clip*   clip;
 
     SkRect empty = { 0, 0, 0, 0 };
     while ((clip = iter.next()) != NULL) {

diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp
index f7ddc97..429d4c0 100644 (file)
--- a/src/core/SkClipStack.cpp
+++ b/src/core/SkClipStack.cpp
@@ -263,7 +263,7 @@ const SkClipStack::Iter::Clip* SkClipStack::Iter::prev() {
     return this->updateClip(rec);
 }
 
-const SkClipStack::Iter::Clip* SkClipStack::Iter::skipToLast(SkRegion::Op op) {
+const SkClipStack::Iter::Clip* SkClipStack::Iter::skipToTopmost(SkRegion::Op op) {
 
     if (NULL == fStack) {
         return NULL;

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 691ee26..7b774b7 100644 (file)
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -217,17 +217,17 @@ void GraphicStackState::pop() {
     fStackDepth--;
 }
 
-// This function initializes iter to be an interator on the "stack" argument
+// This function initializes iter to be an iterator on the "stack" argument
 // and then skips over the leading entries as specified in prefix.  It requires
 // and asserts that "prefix" will be a prefix to "stack."
 static void skip_clip_stack_prefix(const SkClipStack& prefix,
                                    const SkClipStack& stack,
                                    SkClipStack::Iter* iter) {
-    SkClipStack::B2FIter prefixIter(prefix);
-    iter->reset(stack, SkClipStack::Iter::kFront_IterStart);
+    SkClipStack::B2TIter prefixIter(prefix);
+    iter->reset(stack, SkClipStack::Iter::kBottom_IterStart);
 
-    const SkClipStack::B2FIter::Clip* prefixEntry;
-    const SkClipStack::B2FIter::Clip* iterEntry;
+    const SkClipStack::B2TIter::Clip* prefixEntry;
+    const SkClipStack::B2TIter::Clip* iterEntry;
 
     for (prefixEntry = prefixIter.next(); prefixEntry;
             prefixEntry = prefixIter.next()) {
@@ -306,7 +306,7 @@ void GraphicStackState::updateClip(const SkClipStack& clipStack,
     // If the clip stack does anything other than intersect or if it uses
     // an inverse fill type, we have to fall back to the clip region.
     bool needRegion = false;
-    const SkClipStack::B2FIter::Clip* clipEntry;
+    const SkClipStack::B2TIter::Clip* clipEntry;
     for (clipEntry = iter.next(); clipEntry; clipEntry = iter.next()) {
         if (clipEntry->fOp != SkRegion::kIntersect_Op ||
                 (clipEntry->fPath && clipEntry->fPath->isInverseFillType())) {
@@ -323,7 +323,7 @@ void GraphicStackState::updateClip(const SkClipStack& clipStack,
         skip_clip_stack_prefix(fEntries[0].fClipStack, clipStack, &iter);
         SkMatrix transform;
         transform.setTranslate(translation.fX, translation.fY);
-        const SkClipStack::B2FIter::Clip* clipEntry;
+        const SkClipStack::B2TIter::Clip* clipEntry;
         for (clipEntry = iter.next(); clipEntry; clipEntry = iter.next()) {
             SkASSERT(clipEntry->fOp == SkRegion::kIntersect_Op);
             if (clipEntry->fRect) {

diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp
index a1e8323..85a9997 100644 (file)
--- a/tests/ClipStackTest.cpp
+++ b/tests/ClipStackTest.cpp
@@ -10,12 +10,18 @@
 #include "SkPath.h"
 #include "SkRect.h"
 
+
+
 static void test_assign_and_comparison(skiatest::Reporter* reporter) {
     SkClipStack s;
     bool doAA = false;
 
+    REPORTER_ASSERT(reporter, 0 == s.getSaveCount());
+
     // Build up a clip stack with a path, an empty clip, and a rect.
     s.save();
+    REPORTER_ASSERT(reporter, 1 == s.getSaveCount());
+
     SkPath p;
     p.moveTo(5, 6);
     p.lineTo(7, 8);
@@ -24,12 +30,16 @@ static void test_assign_and_comparison(skiatest::Reporter* reporter) {
     s.clipDevPath(p, SkRegion::kIntersect_Op, doAA);
 
     s.save();
+    REPORTER_ASSERT(reporter, 2 == s.getSaveCount());
+
     SkRect r = SkRect::MakeLTRB(1, 2, 3, 4);
     s.clipDevRect(r, SkRegion::kIntersect_Op, doAA);
     r = SkRect::MakeLTRB(10, 11, 12, 13);
     s.clipDevRect(r, SkRegion::kIntersect_Op, doAA);
 
     s.save();
+    REPORTER_ASSERT(reporter, 3 == s.getSaveCount());
+
     r = SkRect::MakeLTRB(14, 15, 16, 17);
     s.clipDevRect(r, SkRegion::kUnion_Op, doAA);
 
@@ -39,17 +49,23 @@ static void test_assign_and_comparison(skiatest::Reporter* reporter) {
 
     // Test that different save levels triggers not equal.
     s.restore();
+    REPORTER_ASSERT(reporter, 2 == s.getSaveCount());
     REPORTER_ASSERT(reporter, s != copy);
 
     // Test that an equal, but not copied version is equal.
     s.save();
+    REPORTER_ASSERT(reporter, 3 == s.getSaveCount());
+
     r = SkRect::MakeLTRB(14, 15, 16, 17);
     s.clipDevRect(r, SkRegion::kUnion_Op, doAA);
     REPORTER_ASSERT(reporter, s == copy);
 
     // Test that a different op on one level triggers not equal.
     s.restore();
+    REPORTER_ASSERT(reporter, 2 == s.getSaveCount());
     s.save();
+    REPORTER_ASSERT(reporter, 3 == s.getSaveCount());
+
     r = SkRect::MakeLTRB(14, 15, 16, 17);
     s.clipDevRect(r, SkRegion::kIntersect_Op, doAA);
     REPORTER_ASSERT(reporter, s != copy);
@@ -69,22 +85,33 @@ static void test_assign_and_comparison(skiatest::Reporter* reporter) {
 
     // Test that different rects triggers not equal.
     s.restore();
+    REPORTER_ASSERT(reporter, 2 == s.getSaveCount());
     s.save();
+    REPORTER_ASSERT(reporter, 3 == s.getSaveCount());
+
     r = SkRect::MakeLTRB(24, 25, 26, 27);
     s.clipDevRect(r, SkRegion::kUnion_Op, doAA);
     REPORTER_ASSERT(reporter, s != copy);
 
     // Sanity check
     s.restore();
+    REPORTER_ASSERT(reporter, 2 == s.getSaveCount());
+
     copy.restore();
+    REPORTER_ASSERT(reporter, 2 == copy.getSaveCount());
     REPORTER_ASSERT(reporter, s == copy);
     s.restore();
+    REPORTER_ASSERT(reporter, 1 == s.getSaveCount());
     copy.restore();
+    REPORTER_ASSERT(reporter, 1 == copy.getSaveCount());
     REPORTER_ASSERT(reporter, s == copy);
 
     // Test that different paths triggers not equal.
     s.restore();
+    REPORTER_ASSERT(reporter, 0 == s.getSaveCount());
     s.save();
+    REPORTER_ASSERT(reporter, 1 == s.getSaveCount());
+
     p.addRect(r);
     s.clipDevPath(p, SkRegion::kIntersect_Op, doAA);
     REPORTER_ASSERT(reporter, s != copy);
@@ -92,9 +119,7 @@ static void test_assign_and_comparison(skiatest::Reporter* reporter) {
 
 static void assert_count(skiatest::Reporter* reporter, const SkClipStack& stack,
                          int count) {
-    REPORTER_ASSERT(reporter, count == stack.getSaveCount());
-
-    SkClipStack::B2FIter iter(stack);
+    SkClipStack::B2TIter iter(stack);
     int counter = 0;
     while (iter.next()) {
         counter += 1;
@@ -102,9 +127,66 @@ static void assert_count(skiatest::Reporter* reporter, const SkClipStack& stack,
     REPORTER_ASSERT(reporter, count == counter);
 }
 
+static void test_iterators(skiatest::Reporter* reporter) {
+    SkClipStack stack;
+
+    static const SkRect gRects[] = {
+        { 0,   0,  40,  40 },
+        { 60,  0, 100,  40 },
+        { 0,  60,  40, 100 },
+        { 60, 60, 100, 100 }
+    };
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(gRects); i++) {
+        // the union op will prevent these from being fused together
+        stack.clipDevRect(gRects[i], SkRegion::kUnion_Op, false);
+    }
+
+    assert_count(reporter, stack, 4);
+
+    // bottom to top iteration
+    {
+        const SkClipStack::B2TIter::Clip* clip = NULL;
+
+        SkClipStack::B2TIter iter(stack);
+        int i;
+
+        for (i = 0, clip = iter.next(); clip; ++i, clip = iter.next()) {
+            REPORTER_ASSERT(reporter, *clip->fRect == gRects[i]);
+        }
+
+        SkASSERT(i == 4);
+    }
+
+    // top to bottom iteration
+    {
+        const SkClipStack::Iter::Clip* clip = NULL;
+
+        SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart);
+        int i;
+
+        for (i = 3, clip = iter.prev(); clip; --i, clip = iter.prev()) {
+            REPORTER_ASSERT(reporter, *clip->fRect == gRects[i]);
+        }
+
+        SkASSERT(i == -1);
+    }
+
+    // skipToTopmost
+    {
+        const SkClipStack::Iter::Clip*clip = NULL;
+
+        SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart);
+
+        clip = iter.skipToTopmost(SkRegion::kUnion_Op);
+        REPORTER_ASSERT(reporter, *clip->fRect == gRects[3]);
+    }
+}
+
 static void TestClipStack(skiatest::Reporter* reporter) {
     SkClipStack stack;
 
+    REPORTER_ASSERT(reporter, 0 == stack.getSaveCount());
     assert_count(reporter, stack, 0);
 
     static const SkIRect gRects[] = {
@@ -118,8 +200,8 @@ static void TestClipStack(skiatest::Reporter* reporter) {
     }
 
     // all of the above rects should have been intersected, leaving only 1 rect
-    SkClipStack::B2FIter iter(stack);
-    const SkClipStack::B2FIter::Clip* clip = iter.next();
+    SkClipStack::B2TIter iter(stack);
+    const SkClipStack::B2TIter::Clip* clip = iter.next();
     SkRect answer;
     answer.iset(25, 25, 75, 75);
 
@@ -132,9 +214,11 @@ static void TestClipStack(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, !iter.next());
 
     stack.reset();
+    REPORTER_ASSERT(reporter, 0 == stack.getSaveCount());
     assert_count(reporter, stack, 0);
 
     test_assign_and_comparison(reporter);
+    test_iterators(reporter);
 }
 
 #include "TestClassDef.h"