From: bsalomon@google.com Date: Wed, 7 Nov 2012 21:19:10 +0000 (+0000) Subject: Combine multiple intersecting rects in SkClipStack::Iter. X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~14356 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e8ca6c6e3a55634ac76efe5aceafaf8d669f43ba;p=platform%2Fupstream%2FlibSkiaSharp.git Combine multiple intersecting rects in SkClipStack::Iter. R=robertphillips@google.com Review URL: https://codereview.appspot.com/6816104 git-svn-id: http://skia.googlecode.com/svn/trunk@6339 2bbb7eff-a529-9590-31e7-b0007b416f81 --- diff --git a/include/core/SkClipStack.h b/include/core/SkClipStack.h index 79fd2c9..443e6ad 100644 --- a/include/core/SkClipStack.h +++ b/include/core/SkClipStack.h @@ -103,7 +103,7 @@ public: /** * The generation ID has three reserved values to indicate special - * (potentially ignoreable) cases + * (potentially ignorable) cases */ static const int32_t kInvalidGenID = 0; static const int32_t kEmptyGenID = 1; // no pixels writeable @@ -159,6 +159,14 @@ public: const Clip* prev(); /** + * This is a variant of next() that greedily attempts to combine elements when possible. + * Currently it attempts to combine intersecting rectangles, though it may do more in the + * future. The returned Clip may not refer to a single element in the stack, so its + * generation ID may be invalid. + */ + const Clip* nextCombined(); + + /** * 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. @@ -174,7 +182,7 @@ public: const SkClipStack* fStack; Clip fClip; SkDeque::Iter fIter; - + SkRect fCombinedRect; // used for nextCombined() /** * updateClip updates fClip to the current state of fIter. It unifies * functionality needed by both next() and prev(). diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp index a5b591e..38f2580 100644 --- a/src/core/SkClipStack.cpp +++ b/src/core/SkClipStack.cpp @@ -819,6 +819,57 @@ const SkClipStack::Iter::Clip* SkClipStack::Iter::skipToTopmost(SkRegion::Op op) return this->next(); } +const SkClipStack::Iter::Clip* SkClipStack::Iter::nextCombined() { + const Clip* clip; + + if (NULL != (clip = this->next()) && + SkRegion::kIntersect_Op == clip->fOp && + NULL != clip->fRect) { + fCombinedRect = *clip->fRect; + bool doAA = clip->fDoAA; + + while(NULL != (clip = this->next()) && + SkRegion::kIntersect_Op == clip->fOp && + NULL != clip->fRect) { // backup if non-null + /** + * If the AA settings don't match on consecutive rects we can still continue if + * either contains the other. Otherwise, we must stop. + */ + if (doAA != clip->fDoAA) { + if (fCombinedRect.contains(*clip->fRect)) { + fCombinedRect = *clip->fRect; + doAA = clip->fDoAA; + } else if (!clip->fRect->contains(fCombinedRect)) { + break; + } + } else if (!fCombinedRect.intersect(*fClip.fRect)) { + fCombinedRect.setEmpty(); + clip = NULL; // prevents unnecessary rewind below. + break; + } + } + // If we got here and clip is non-NULL then we got an element that we weren't able to + // combine. We need to backup one to ensure that the callers next next() call returns it. + if (NULL != clip) { + // If next() above returned the last element then due to Iter's internal workings prev() + // will return NULL. In that case we reset to the last element. + if (NULL == this->prev()) { + this->reset(*fStack, SkClipStack::Iter::kTop_IterStart); + } + } + + // Must do this last because it is overwritten in the above backup. + fClip.fRect = &fCombinedRect; + fClip.fPath = NULL; + fClip.fOp = SkRegion::kIntersect_Op; + fClip.fDoAA = doAA; + fClip.fGenID = kInvalidGenID; + return &fClip; + } else { + return clip; + } +} + void SkClipStack::Iter::reset(const SkClipStack& stack, IterStart startLoc) { fStack = &stack; fIter.reset(stack.fDeque, static_cast(startLoc)); diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 606b90e..2db006a 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -88,7 +88,7 @@ bool requires_AA(const SkClipStack& clipIn) { const SkClipStack::Iter::Clip* clip = NULL; for (clip = iter.skipToTopmost(SkRegion::kReplace_Op); NULL != clip; - clip = iter.next()) { + clip = iter.nextCombined()) { if (clip->fDoAA) { return true; @@ -117,13 +117,7 @@ bool GrClipMaskManager::useSWOnlyPath(const SkClipStack& clipIn) { for (clip = iter.skipToTopmost(SkRegion::kReplace_Op); NULL != clip; - clip = iter.next()) { - - if (SkRegion::kReplace_Op == clip->fOp) { - // Everything before a replace op can be ignored so start - // afresh w.r.t. determining if any element uses the SW path - useSW = false; - } + clip = iter.nextCombined()) { // rects can always be drawn directly w/o using the software path // so only paths need to be checked @@ -297,7 +291,7 @@ const SkClipStack::Iter::Clip* process_initial_clip_elements( for (clip = iter->skipToTopmost(SkRegion::kReplace_Op); NULL != clip && !done; - clip = iter->next()) { + clip = iter->nextCombined()) { switch (clip->fOp) { case SkRegion::kReplace_Op: // replace ignores everything previous @@ -644,7 +638,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, GrAutoScratchTexture temp; bool first = true; // walk through each clip element and perform its set op - for ( ; NULL != clip; clip = iter.next()) { + for ( ; NULL != clip; clip = iter.nextCombined()) { SkRegion::Op op = clip->fOp; if (first) { @@ -785,7 +779,7 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, // walk through each clip element and perform its set op // with the existing clip. - for ( ; NULL != clip; clip = iter.next()) { + for ( ; NULL != clip; clip = iter.nextCombined()) { GrPathFill fill; bool fillInverted = false; // enabled at bottom of loop @@ -1151,7 +1145,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClipData& clipDataIn, helper.clear(clearToInside ? 0xFF : 0x00); bool first = true; - for ( ; NULL != clip; clip = iter.next()) { + for ( ; NULL != clip; clip = iter.nextCombined()) { SkRegion::Op op = clip->fOp; if (first) { diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp index 8a74dc8..0516d9e 100644 --- a/tests/ClipStackTest.cpp +++ b/tests/ClipStackTest.cpp @@ -473,6 +473,173 @@ static void test_rect_merging(skiatest::Reporter* reporter) { } } + +// This is similar to the above test but tests the iterator's ability to merge rects in the +// middle of a clip stack's sequence using nextCombined(). There is a save after every clip +// element to prevent the clip stack from merging the rectangles as they are added. +static void test_iter_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 farAway = SkRect::MakeLTRB(1000, 1000, 1010, 1010); + + SkRect overlapIntersect; + overlapIntersect.intersect(overlapLeft, overlapRight); + + SkPath path1, path2; + path1.addCircle(SkIntToScalar(30), SkIntToScalar(30), SkIntToScalar(1000)); + path2.addOval(SkRect::MakeWH(500, 600)); + + const SkClipStack::Iter::Clip* clip; + + // call nextCombined with an empty clip stack + { + SkClipStack stack; + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + REPORTER_ASSERT(reporter, NULL == iter.nextCombined()); + } + + // two bw overlapping - should merge, bracketed by paths + { + SkClipStack stack; + stack.clipDevPath(path1, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevRect(overlapLeft, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevPath(path2, SkRegion::kIntersect_Op, false); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, *clip->fPath == path1 && !clip->fDoAA); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, !clip->fDoAA && *clip->fRect == overlapIntersect); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, *clip->fPath == path2 && !clip->fDoAA); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // same as above but rects are aa and no final path. + { + SkClipStack stack; + stack.clipDevPath(path1, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevRect(overlapLeft, SkRegion::kIntersect_Op, true); stack.save(); + + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, true); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, *clip->fPath == path1 && !clip->fDoAA); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fDoAA && *clip->fRect == overlapIntersect); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // mixed overlapping - no paths - should _not_ merge + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kIntersect_Op, true); stack.save(); + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, false); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fDoAA && *clip->fRect == overlapLeft); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, !clip->fDoAA && *clip->fRect == overlapRight); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // three rects in a row where the third rect uses a non-intersect op. + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kIntersect_Op, true); stack.save(); + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, true); stack.save(); + stack.clipDevRect(nestedParent, SkRegion::kXOR_Op, true); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fDoAA && *clip->fRect == overlapIntersect); + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fDoAA && *clip->fRect == nestedParent); + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // mixed nested (bw inside aa) - should merge + { + SkClipStack stack; + stack.clipDevRect(nestedParent, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevRect(nestedChild, SkRegion::kIntersect_Op, true); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fDoAA && *clip->fRect == nestedChild); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // mixed nested (aa inside bw) - should merge + { + SkClipStack stack; + stack.clipDevRect(nestedChild, SkRegion::kIntersect_Op, false); stack.save(); + + stack.clipDevRect(nestedParent, SkRegion::kIntersect_Op, true); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, !clip->fDoAA && *clip->fRect == nestedChild); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } + + // three rect intersects in a row where result is empty after the second. + { + SkClipStack stack; + + stack.clipDevRect(overlapLeft, SkRegion::kIntersect_Op, false); stack.save(); + stack.clipDevRect(farAway, SkRegion::kIntersect_Op, false); stack.save(); + stack.clipDevRect(overlapRight, SkRegion::kIntersect_Op, false); stack.save(); + + SkClipStack::Iter iter(stack, SkClipStack::Iter::kBottom_IterStart); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, clip->fRect->isEmpty()); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, *clip->fRect == overlapRight); + + clip = iter.nextCombined(); + REPORTER_ASSERT(reporter, NULL == clip); + } +} + static void TestClipStack(skiatest::Reporter* reporter) { SkClipStack stack; @@ -513,6 +680,7 @@ static void TestClipStack(skiatest::Reporter* reporter) { test_bounds(reporter, false); // once with paths test_isWideOpen(reporter); test_rect_merging(reporter); + test_iter_rect_merging(reporter); } #include "TestClassDef.h"