Added check for aa/bw rect merging
http://codereview.appspot.com/6449079/
git-svn-id: http://skia.googlecode.com/svn/trunk@4907 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp
index e9a02ec..38856e5 100644
--- a/src/core/SkClipStack.cpp
+++ b/src/core/SkClipStack.cpp
@@ -58,6 +58,13 @@
// 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,18 +89,51 @@
/**
* 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 @@
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 @@
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 @@
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 @@
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 afbbd7e..1def9b4 100644
--- a/tests/ClipStackTest.cpp
+++ b/tests/ClipStackTest.cpp
@@ -127,6 +127,8 @@
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 @@
}
}
+// Exercise the SkClipStack's getConservativeBounds computation
static void test_bounds(skiatest::Reporter* reporter, bool useRects) {
static const int gNumCases = 20;
@@ -351,6 +354,124 @@
}
}
+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 @@
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"