diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp index e9a02ecbb..38856e586 100644 --- a/src/core/SkClipStack.cpp +++ b/src/core/SkClipStack.cpp @@ -58,6 +58,13 @@ struct SkClipStack::Rec { // bounding box members are updated in a following updateBound call } + void setEmpty() { + fState = kEmpty_State; + fFiniteBound.setEmpty(); + fFiniteBoundType = kNormal_BoundsType; + fIsIntersectionOfRects = false; + } + bool operator==(const Rec& b) const { if (fSaveCount != b.fSaveCount || fOp != b.fOp || fState != b.fState || fDoAA != b.fDoAA) { @@ -82,17 +89,50 @@ struct SkClipStack::Rec { /** * Returns true if this Rec can be intersected in place with a new clip */ - bool canBeIntersected(int saveCount, SkRegion::Op op) const { + bool canBeIntersectedInPlace(int saveCount, SkRegion::Op op) const { if (kEmpty_State == fState && ( SkRegion::kDifference_Op == op || SkRegion::kIntersect_Op == op)) { return true; } - return fSaveCount == saveCount && - SkRegion::kIntersect_Op == fOp && - SkRegion::kIntersect_Op == op; + return fSaveCount == saveCount && + SkRegion::kIntersect_Op == op && + (SkRegion::kIntersect_Op == fOp || SkRegion::kReplace_Op == fOp); } + /** + * This method checks to see if two rect clips can be safely merged into + * one. The issue here is that to be strictly correct all the edges of + * the resulting rect must have the same anti-aliasing. + */ + bool rectRectIntersectAllowed(const SkRect& newR, bool newAA) const { + SkASSERT(kRect_State == fState); + + if (fDoAA == newAA) { + // if the AA setting is the same there is no issue + return true; + } + + if (!SkRect::Intersects(fRect, newR)) { + // The calling code will correctly set the result to the empty clip + return true; + } + + if (fRect.contains(newR)) { + // if the new rect carves out a portion of the old one there is no + // issue + return true; + } + + // So either the two overlap in some complex manner or newR contains oldR. + // In the first, case the edges will require different AA. In the second, + // the AA setting that would be carried forward is incorrect (e.g., oldR + // is AA while newR is BW but since newR contains oldR, oldR will be + // drawn BW) since the new AA setting will predominate. + return false; + } + + /** * The different combination of fill & inverse fill when combining * bounding boxes @@ -283,7 +323,8 @@ struct SkClipStack::Rec { if (SkRegion::kReplace_Op == fOp || (SkRegion::kIntersect_Op == fOp && NULL == prior) || - (SkRegion::kIntersect_Op == fOp && prior->fIsIntersectionOfRects)) { + (SkRegion::kIntersect_Op == fOp && prior->fIsIntersectionOfRects && + prior->rectRectIntersectAllowed(fRect, fDoAA))) { fIsIntersectionOfRects = true; } @@ -486,32 +527,29 @@ void SkClipStack::clipDevRect(const SkRect& rect, SkRegion::Op op, bool doAA) { SkDeque::Iter iter(fDeque, SkDeque::Iter::kBack_IterStart); Rec* rec = (Rec*) iter.prev(); - if (rec && rec->canBeIntersected(fSaveCount, op)) { + if (rec && rec->canBeIntersectedInPlace(fSaveCount, op)) { switch (rec->fState) { case Rec::kEmpty_State: SkASSERT(rec->fFiniteBound.isEmpty()); SkASSERT(kNormal_BoundsType == rec->fFiniteBoundType); SkASSERT(!rec->fIsIntersectionOfRects); return; - case Rec::kRect_State: { - if (!rec->fRect.intersect(rect)) { - rec->fState = Rec::kEmpty_State; - rec->fFiniteBound.setEmpty(); - rec->fFiniteBoundType = kNormal_BoundsType; - rec->fIsIntersectionOfRects = false; + case Rec::kRect_State: + if (rec->rectRectIntersectAllowed(rect, doAA)) { + if (!rec->fRect.intersect(rect)) { + rec->setEmpty(); + return; + } + + rec->fDoAA = doAA; + Rec* prev = (Rec*) iter.prev(); + rec->updateBound(prev); return; } - - Rec* prev = (Rec*) iter.prev(); - rec->updateBound(prev); - return; - } + break; case Rec::kPath_State: if (!SkRect::Intersects(rec->fPath.getBounds(), rect)) { - rec->fState = Rec::kEmpty_State; - rec->fFiniteBound.setEmpty(); - rec->fFiniteBoundType = kNormal_BoundsType; - rec->fIsIntersectionOfRects = false; + rec->setEmpty(); return; } break; @@ -527,7 +565,7 @@ void SkClipStack::clipDevPath(const SkPath& path, SkRegion::Op op, bool doAA) { return this->clipDevRect(alt, op, doAA); } Rec* rec = (Rec*)fDeque.back(); - if (rec && rec->canBeIntersected(fSaveCount, op)) { + if (rec && rec->canBeIntersectedInPlace(fSaveCount, op)) { const SkRect& pathBounds = path.getBounds(); switch (rec->fState) { case Rec::kEmpty_State: @@ -537,19 +575,13 @@ void SkClipStack::clipDevPath(const SkPath& path, SkRegion::Op op, bool doAA) { return; case Rec::kRect_State: if (!SkRect::Intersects(rec->fRect, pathBounds)) { - rec->fState = Rec::kEmpty_State; - rec->fFiniteBound.setEmpty(); - rec->fFiniteBoundType = kNormal_BoundsType; - rec->fIsIntersectionOfRects = false; + rec->setEmpty(); return; } break; case Rec::kPath_State: if (!SkRect::Intersects(rec->fPath.getBounds(), pathBounds)) { - rec->fState = Rec::kEmpty_State; - rec->fFiniteBound.setEmpty(); - rec->fFiniteBoundType = kNormal_BoundsType; - rec->fIsIntersectionOfRects = false; + rec->setEmpty(); return; } break; diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp index afbbd7e23..1def9b4e9 100644 --- a/tests/ClipStackTest.cpp +++ b/tests/ClipStackTest.cpp @@ -127,6 +127,8 @@ static void assert_count(skiatest::Reporter* reporter, const SkClipStack& stack, REPORTER_ASSERT(reporter, count == counter); } +// Exercise the SkClipStack's bottom to top and bidirectional iterators +// (including the skipToTopmost functionality) static void test_iterators(skiatest::Reporter* reporter) { SkClipStack stack; @@ -183,6 +185,7 @@ static void test_iterators(skiatest::Reporter* reporter) { } } +// Exercise the SkClipStack's getConservativeBounds computation static void test_bounds(skiatest::Reporter* reporter, bool useRects) { static const int gNumCases = 20; @@ -351,6 +354,124 @@ static void test_isWideOpen(skiatest::Reporter* reporter) { } } +int count(const SkClipStack& stack) { + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart); + + const SkClipStack::Iter::Clip* clip = NULL; + int count = 0; + + for (clip = iter.prev(); clip; clip = iter.prev(), ++count) { + ; + } + + return count; +} + +// Test out SkClipStack's merging of rect clips. In particular exercise +// merging of aa vs. bw rects. +static void test_rect_merging(skiatest::Reporter* reporter) { + + SkRect overlapLeft = SkRect::MakeLTRB(10, 10, 50, 50); + SkRect overlapRight = SkRect::MakeLTRB(40, 40, 80, 80); + + SkRect nestedParent = SkRect::MakeLTRB(10, 10, 90, 90); + SkRect nestedChild = SkRect::MakeLTRB(40, 40, 60, 60); + + SkRect bound; + SkClipStack::BoundsType type; + bool isIntersectionOfRects; + + // all bw overlapping - should merge + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kReplace_Op, false); + + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, false); + + REPORTER_ASSERT(reporter, 1 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, isIntersectionOfRects); + } + + // all aa overlapping - should merge + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kReplace_Op, true); + + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, true); + + REPORTER_ASSERT(reporter, 1 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, isIntersectionOfRects); + } + + // mixed overlapping - should _not_ merge + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kReplace_Op, true); + + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, false); + + REPORTER_ASSERT(reporter, 2 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, !isIntersectionOfRects); + } + + // mixed nested (bw inside aa) - should merge + { + SkClipStack stack; + + stack.clipDevRect(nestedParent, SkRegion::kReplace_Op, true); + + stack.clipDevRect(nestedChild, SkRegion::kIntersect_Op, false); + + REPORTER_ASSERT(reporter, 1 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, isIntersectionOfRects); + } + + // mixed nested (aa inside bw) - should merge + { + SkClipStack stack; + + stack.clipDevRect(nestedParent, SkRegion::kReplace_Op, false); + + stack.clipDevRect(nestedChild, SkRegion::kIntersect_Op, true); + + REPORTER_ASSERT(reporter, 1 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, isIntersectionOfRects); + } + + // reverse nested (aa inside bw) - should _not_ merge + { + SkClipStack stack; + + stack.clipDevRect(nestedChild, SkRegion::kReplace_Op, false); + + stack.clipDevRect(nestedParent, SkRegion::kIntersect_Op, true); + + REPORTER_ASSERT(reporter, 2 == count(stack)); + + stack.getBounds(&bound, &type, &isIntersectionOfRects); + + REPORTER_ASSERT(reporter, !isIntersectionOfRects); + } +} static void TestClipStack(skiatest::Reporter* reporter) { SkClipStack stack; @@ -388,9 +509,10 @@ static void TestClipStack(skiatest::Reporter* reporter) { test_assign_and_comparison(reporter); test_iterators(reporter); - test_bounds(reporter, true); - test_bounds(reporter, false); + test_bounds(reporter, true); // once with rects + test_bounds(reporter, false); // once with paths test_isWideOpen(reporter); + test_rect_merging(reporter); } #include "TestClassDef.h"