From: caryclark Date: Thu, 25 Aug 2016 18:27:17 +0000 (-0700) Subject: path ops stream-lining X-Git-Tag: submit/tizen/20180928.044319~106^2~721 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=29b2563afb1677515739f1d24fb27733626eca92;p=platform%2Fupstream%2FlibSkiaSharp.git path ops stream-lining The addT() function is a workhorse of pathops. Make it simpler, removing branches and parameters. Separate addOpp() into const and modify parts. Add more debugging that asserts if the function fails and the data is not extreme (e.g., fuzzer generated). TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2273293004 Review-Url: https://codereview.chromium.org/2273293004 --- diff --git a/src/pathops/SkAddIntersections.cpp b/src/pathops/SkAddIntersections.cpp index a6cebca4b8..bdc7ae2565 100644 --- a/src/pathops/SkAddIntersections.cpp +++ b/src/pathops/SkAddIntersections.cpp @@ -505,11 +505,12 @@ bool AddIntersectTs(SkOpContour* test, SkOpContour* next, SkOpCoincidence* coinc SkASSERT(ts[0][pt] >= 0 && ts[0][pt] <= 1); SkASSERT(ts[1][pt] >= 0 && ts[1][pt] <= 1); wt.segment()->debugValidate(); - SkOpPtT* testTAt = wt.segment()->addT(ts[swap][pt], nullptr); + SkOpPtT* testTAt = wt.segment()->addT(ts[swap][pt]); wn.segment()->debugValidate(); - SkOpPtT* nextTAt = wn.segment()->addT(ts[!swap][pt], nullptr); - if (testTAt->addOpp(nextTAt)) { - testTAt->span()->checkForCollapsedCoincidence(); + SkOpPtT* nextTAt = wn.segment()->addT(ts[!swap][pt]); + SkOpPtT* oppPrev = testTAt->oppPrev(nextTAt); + if (oppPrev) { + testTAt->addOpp(nextTAt, oppPrev); } if (testTAt->fPt != nextTAt->fPt) { testTAt->span()->unaligned(); diff --git a/src/pathops/SkOpCoincidence.cpp b/src/pathops/SkOpCoincidence.cpp index 7717d06a80..1c8ab5f524 100755 --- a/src/pathops/SkOpCoincidence.cpp +++ b/src/pathops/SkOpCoincidence.cpp @@ -299,7 +299,7 @@ bool SkOpCoincidence::addEndMovedSpans(const SkOpSpan* base, const SkOpSpanBase* continue; } SkOpSegment* writableSeg = const_cast(testSeg); - SkOpPtT* oppStart = writableSeg->addT(t, nullptr); + SkOpPtT* oppStart = writableSeg->addT(t); SkOpSpan* writableBase = const_cast(base); oppStart->span()->addOppAndMerge(writableBase); if (oppStart->deleted()) { @@ -668,9 +668,9 @@ bool SkOpCoincidence::addOrOverlap(SkOpSegment* coinSeg, SkOpSegment* oppSeg, this->debugValidate(); if (!cs || !os) { SkOpPtT* csWritable = cs ? const_cast(cs) - : coinSeg->addT(coinTs, nullptr); + : coinSeg->addT(coinTs); SkOpPtT* osWritable = os ? const_cast(os) - : oppSeg->addT(oppTs, nullptr); + : oppSeg->addT(oppTs); RETURN_FALSE_IF(callerAborts, !csWritable || !osWritable); csWritable->span()->addOppAndMerge(osWritable->span()); cs = csWritable; @@ -679,9 +679,9 @@ bool SkOpCoincidence::addOrOverlap(SkOpSegment* coinSeg, SkOpSegment* oppSeg, } if (!ce || !oe) { SkOpPtT* ceWritable = ce ? const_cast(ce) - : coinSeg->addT(coinTe, nullptr); + : coinSeg->addT(coinTe); SkOpPtT* oeWritable = oe ? const_cast(oe) - : oppSeg->addT(oppTe, nullptr); + : oppSeg->addT(oppTe); ceWritable->span()->addOppAndMerge(oeWritable->span()); ce = ceWritable; oe = oeWritable; diff --git a/src/pathops/SkOpSegment.cpp b/src/pathops/SkOpSegment.cpp index 87023261c5..3c63683285 100644 --- a/src/pathops/SkOpSegment.cpp +++ b/src/pathops/SkOpSegment.cpp @@ -251,53 +251,39 @@ bool SkOpSegment::addExpanded(double newT, const SkOpSpanBase* test, bool* start if (this->contains(newT)) { return true; } - SkOpPtT* newPtT = this->addT(newT, startOver); + this->globalState()->resetAllocatedOpSpan(); + SkOpPtT* newPtT = this->addT(newT); + *startOver |= this->globalState()->allocatedOpSpan(); if (!newPtT) { return false; } newPtT->fPt = this->ptAtT(newT); // const cast away to change linked list; pt/t values stays unchanged - SkOpSpanBase* writableTest = const_cast(test); - if (writableTest->ptT()->addOpp(newPtT)) { + SkOpPtT* oppPrev = test->ptT()->oppPrev(newPtT); + if (oppPrev) { + SkOpSpanBase* writableTest = const_cast(test); + writableTest->ptT()->addOpp(newPtT, oppPrev); writableTest->checkForCollapsedCoincidence(); } return true; } // Please keep this in sync with debugAddT() -SkOpPtT* SkOpSegment::addT(double t, bool* allocated) { +SkOpPtT* SkOpSegment::addT(double t) { debugValidate(); SkPoint pt = this->ptAtT(t); - SkOpSpanBase* span = &fHead; + SkOpSpanBase* spanBase = &fHead; do { - SkOpPtT* result = span->ptT(); - SkOpPtT* loop; - bool duplicatePt; - if (t == result->fT) { - goto bumpSpan; - } - if (this->match(result, this, t, pt)) { - // see if any existing alias matches segment, pt, and t - loop = result->next(); - duplicatePt = false; - while (loop != result) { - bool ptMatch = loop->fPt == pt; - if (loop->segment() == this && loop->fT == t && ptMatch) { - goto bumpSpan; - } - duplicatePt |= ptMatch; - loop = loop->next(); - } - bumpSpan: - span->bumpSpanAdds(); + SkOpPtT* result = spanBase->ptT(); + if (t == result->fT || this->match(result, this, t, pt)) { + spanBase->bumpSpanAdds(); return result; } if (t < result->fT) { SkOpSpan* prev = result->span()->prev(); - if (!prev) { - return nullptr; - } - SkOpSpan* span = insert(prev); + FAIL_WITH_NULL_IF(!prev); + // marks in global state that new op span has been allocated + SkOpSpan* span = this->insert(prev); span->init(this, prev, t, pt); this->debugValidate(); #if DEBUG_ADD_T @@ -305,17 +291,12 @@ SkOpPtT* SkOpSegment::addT(double t, bool* allocated) { span->segment()->debugID(), span->debugID()); #endif span->bumpSpanAdds(); - if (allocated) { - *allocated = true; - } return span->ptT(); } - if (span == &fTail) { - return nullptr; - } - } while ((span = span->upCast()->next())); + FAIL_WITH_NULL_IF(spanBase == &fTail); + } while ((spanBase = spanBase->upCast()->next())); SkASSERT(0); - return nullptr; + return nullptr; // we never get here, but need this to satisfy compiler } void SkOpSegment::calcAngles() { diff --git a/src/pathops/SkOpSegment.h b/src/pathops/SkOpSegment.h index 312312a153..1b7aad199c 100644 --- a/src/pathops/SkOpSegment.h +++ b/src/pathops/SkOpSegment.h @@ -92,7 +92,7 @@ public: return this; } - SkOpPtT* addT(double t, bool* allocated); + SkOpPtT* addT(double t); template T* allocateArray(int count) { return SkOpTAllocator::AllocateArray(this->globalState()->allocator(), count); @@ -128,7 +128,7 @@ public: } void debugAddAngle(double startT, double endT); - const SkOpPtT* debugAddT(double t, bool* allocated) const; + const SkOpPtT* debugAddT(double t) const; const SkOpAngle* debugAngle(int id) const; #if DEBUG_ANGLE void debugCheckAngleCoin() const; @@ -231,7 +231,9 @@ public: void init(SkPoint pts[], SkScalar weight, SkOpContour* parent, SkPath::Verb verb); SkOpSpan* insert(SkOpSpan* prev) { - SkOpSpan* result = SkOpTAllocator::Allocate(this->globalState()->allocator()); + SkOpGlobalState* globalState = this->globalState(); + globalState->setAllocatedOpSpan(); + SkOpSpan* result = SkOpTAllocator::Allocate(globalState->allocator()); SkOpSpanBase* next = prev->next(); result->setPrev(prev); prev->setNext(result); diff --git a/src/pathops/SkOpSpan.cpp b/src/pathops/SkOpSpan.cpp index 640ba8feb7..98165fcfc5 100755 --- a/src/pathops/SkOpSpan.cpp +++ b/src/pathops/SkOpSpan.cpp @@ -180,7 +180,9 @@ void SkOpPtT::setDeleted() { // please keep this in sync with debugAddOppAndMerge // If the added points envelop adjacent spans, merge them in. void SkOpSpanBase::addOppAndMerge(SkOpSpanBase* opp) { - if (this->ptT()->addOpp(opp->ptT())) { + SkOpPtT* oppPrev = this->ptT()->oppPrev(opp->ptT()); + if (oppPrev) { + this->ptT()->addOpp(opp->ptT(), oppPrev); this->checkForCollapsedCoincidence(); } // compute bounds of points in span diff --git a/src/pathops/SkOpSpan.h b/src/pathops/SkOpSpan.h index ebcc984b33..96a97e0732 100644 --- a/src/pathops/SkOpSpan.h +++ b/src/pathops/SkOpSpan.h @@ -29,24 +29,12 @@ public: }; // please keep in sync with debugAddOpp() - bool addOpp(SkOpPtT* opp) { - // find the fOpp ptr to opp - SkOpPtT* oppPrev = opp->fNext; - if (oppPrev == this) { - return false; - } - while (oppPrev->fNext != opp) { - oppPrev = oppPrev->fNext; - if (oppPrev == this) { - return false; - } - } + void addOpp(SkOpPtT* opp, SkOpPtT* oppPrev) { SkOpPtT* oldNext = this->fNext; SkASSERT(this != opp); this->fNext = opp; SkASSERT(oppPrev != oldNext); oppPrev->fNext = oldNext; - return true; } bool alias() const; @@ -62,7 +50,7 @@ public: return SkDEBUGRELEASE(fID, -1); } - bool debugAddOpp(const SkOpPtT* opp) const; + void debugAddOpp(const SkOpPtT* opp, const SkOpPtT* oppPrev) const; const SkOpAngle* debugAngle(int id) const; const SkOpCoincidence* debugCoincidence() const; bool debugContains(const SkOpPtT* ) const; @@ -70,6 +58,7 @@ public: SkOpContour* debugContour(int id); int debugLoopLimit(bool report) const; bool debugMatchID(int id) const; + const SkOpPtT* debugOppPrev(const SkOpPtT* opp) const; const SkOpPtT* debugPtT(int id) const; void debugResetCoinT() const; const SkOpSegment* debugSegment(int id) const; @@ -109,6 +98,21 @@ public: bool onEnd() const; + SkOpPtT* oppPrev(SkOpPtT* opp) const { + // find the fOpp ptr to opp + SkOpPtT* oppPrev = opp->fNext; + if (oppPrev == this) { + return nullptr; + } + while (oppPrev->fNext != opp) { + oppPrev = oppPrev->fNext; + if (oppPrev == this) { + return nullptr; + } + } + return oppPrev; + } + static bool Overlaps(const SkOpPtT* s1, const SkOpPtT* e1, const SkOpPtT* s2, const SkOpPtT* e2, const SkOpPtT** sOut, const SkOpPtT** eOut) { const SkOpPtT* start1 = s1->fT < e1->fT ? s1 : e1; diff --git a/src/pathops/SkPathOpsDebug.cpp b/src/pathops/SkPathOpsDebug.cpp index df35be0cfe..212e2bcc79 100644 --- a/src/pathops/SkPathOpsDebug.cpp +++ b/src/pathops/SkPathOpsDebug.cpp @@ -582,39 +582,21 @@ void SkDRect::debugInit() { #if DEBUG_COINCIDENCE // commented-out lines keep this in sync with addT() - const SkOpPtT* SkOpSegment::debugAddT(double t, bool* allocated) const { + const SkOpPtT* SkOpSegment::debugAddT(double t) const { debugValidate(); SkPoint pt = this->ptAtT(t); const SkOpSpanBase* span = &fHead; do { const SkOpPtT* result = span->ptT(); - const SkOpPtT* loop; - bool duplicatePt; - if (t == result->fT) { - goto bumpSpan; - } - if (this->match(result, this, t, pt)) { - // see if any existing alias matches segment, pt, and t - loop = result->next(); - duplicatePt = false; - while (loop != result) { - bool ptMatch = loop->fPt == pt; - if (loop->segment() == this && loop->fT == t && ptMatch) { - goto bumpSpan; - } - duplicatePt |= ptMatch; - loop = loop->next(); - } - bumpSpan: + if (t == result->fT || this->match(result, this, t, pt)) { // span->bumpSpanAdds(); return result; } if (t < result->fT) { const SkOpSpan* prev = result->span()->prev(); - if (!prev) { - return nullptr; // FIXME: this is a fail case; nullptr return elsewhere means result was allocated in non-const version - } -// SkOpSpan* span = insert(prev, allocator); + FAIL_WITH_NULL_IF(!prev); + // marks in global state that new op span has been allocated + this->globalState()->setAllocatedOpSpan(); // span->init(this, prev, t, pt); this->debugValidate(); // #if DEBUG_ADD_T @@ -622,15 +604,12 @@ void SkDRect::debugInit() { // span->segment()->debugID(), span->debugID()); // #endif // span->bumpSpanAdds(); - if (allocated) { - *allocated = true; - } return nullptr; } - SkASSERT(span != &fTail); + FAIL_WITH_NULL_IF(span != &fTail); } while ((span = span->upCast()->next())); SkASSERT(0); - return nullptr; + return nullptr; // we never get here, but need this to satisfy compiler } #endif @@ -1493,9 +1472,9 @@ void SkOpCoincidence::debugAddOrOverlap(const SkOpSegment* coinSeg, const SkOpSe this->debugValidate(); if (!cs || !os) { if (!cs) - cs = coinSeg->debugAddT(coinTs, nullptr); + cs = coinSeg->debugAddT(coinTs); if (!os) - os = oppSeg->debugAddT(oppTs, nullptr); + os = oppSeg->debugAddT(oppTs); if (cs && os) cs->span()->debugAddOppAndMerge(id, log, os->span(), &csDeleted, &osDeleted); // cs = csWritable; // os = osWritable; @@ -1505,9 +1484,9 @@ void SkOpCoincidence::debugAddOrOverlap(const SkOpSegment* coinSeg, const SkOpSe } if (!ce || !oe) { if (!ce) - ce = coinSeg->debugAddT(coinTe, nullptr); + ce = coinSeg->debugAddT(coinTe); if (!oe) - oe = oppSeg->debugAddT(oppTe, nullptr); + oe = oppSeg->debugAddT(oppTe); if (ce && oe) ce->span()->debugAddOppAndMerge(id, log, oe->span(), &ceDeleted, &oeDeleted); // ce = ceWritable; // oe = oeWritable; @@ -2059,7 +2038,9 @@ void SkOpSegment::debugValidate() const { // Commented-out lines keep this in sync with addOppAndMerge() // If the added points envelop adjacent spans, merge them in. void SkOpSpanBase::debugAddOppAndMerge(const char* id, SkPathOpsDebug::GlitchLog* log, const SkOpSpanBase* opp, bool* spanDeleted, bool* oppDeleted) const { - if (this->ptT()->debugAddOpp(opp->ptT())) { + const SkOpPtT* oppPrev = this->ptT()->debugOppPrev(opp->ptT()); + if (oppPrev) { + this->ptT()->debugAddOpp(opp->ptT(), oppPrev); this->debugCheckForCollapsedCoincidence(id, log); } // compute bounds of points in span @@ -2315,24 +2296,12 @@ int SkIntersections::debugCoincidentUsed() const { #include "SkOpContour.h" // Commented-out lines keep this in sync with addOpp() -bool SkOpPtT::debugAddOpp(const SkOpPtT* opp) const { - // find the fOpp ptr to opp - const SkOpPtT* oppPrev = opp->fNext; - if (oppPrev == this) { - return false; - } - while (oppPrev->fNext != opp) { - oppPrev = oppPrev->fNext; - if (oppPrev == this) { - return false; - } - } -// const SkOpPtT* oldNext = this->fNext; +void SkOpPtT::debugAddOpp(const SkOpPtT* opp, const SkOpPtT* oppPrev) const { + SkDEBUGCODE(const SkOpPtT* oldNext = this->fNext); SkASSERT(this != opp); // this->fNext = opp; -// SkASSERT(oppPrev != oldNext); + SkASSERT(oppPrev != oldNext); // oppPrev->fNext = oldNext; - return true; } bool SkOpPtT::debugContains(const SkOpPtT* check) const { @@ -2403,6 +2372,10 @@ int SkOpPtT::debugLoopLimit(bool report) const { return 0; } +const SkOpPtT* SkOpPtT::debugOppPrev(const SkOpPtT* opp) const { + return this->oppPrev(const_cast(opp)); +} + void SkOpPtT::debugResetCoinT() const { #if DEBUG_COINCIDENCE_ORDER this->segment()->debugResetCoinT(); diff --git a/src/pathops/SkPathOpsDebug.h b/src/pathops/SkPathOpsDebug.h index c1b1adb707..5ea1dd8b26 100644 --- a/src/pathops/SkPathOpsDebug.h +++ b/src/pathops/SkPathOpsDebug.h @@ -156,12 +156,19 @@ #include "SkTLS.h" #endif -#define FAIL_IF(cond) do { bool fail = (cond); SkOPASSERT(!fail); if (fail) return false; } \ - while (false) +// Tests with extreme numbers may fail, but all other tests should never fail. +#define FAIL_IF(cond) \ + do { bool fail = (cond); SkOPASSERT(!fail); if (fail) return false; } while (false) +#define FAIL_WITH_NULL_IF(cond) \ + do { bool fail = (cond); SkOPASSERT(!fail); if (fail) return nullptr; } while (false) + +// Some functions serve two masters: one allows the function to fail, the other expects success +// always. If abort is true, tests with normal numbers may not fail and assert if they do so. +// If abort is false, both normal and extreme numbers may return false without asserting. #define RETURN_FALSE_IF(abort, cond) \ - do { bool fail = (cond); SkOPASSERT(!(abort) || !fail); if (fail) return false; } \ - while (false) + do { bool fail = (cond); SkOPASSERT(!(abort) || !fail); if (fail) return false; \ + } while (false) class SkPathOpsDebug { public: diff --git a/src/pathops/SkPathOpsTypes.h b/src/pathops/SkPathOpsTypes.h index b336e58855..a645771634 100644 --- a/src/pathops/SkPathOpsTypes.h +++ b/src/pathops/SkPathOpsTypes.h @@ -45,6 +45,10 @@ public: kMaxWindingTries = 10 }; + bool allocatedOpSpan() const { + return fAllocatedOpSpan; + } + SkChunkAlloc* allocator() { return fAllocator; } @@ -127,6 +131,14 @@ public: Phase phase() const { return fPhase; } + + void resetAllocatedOpSpan() { + fAllocatedOpSpan = false; + } + + void setAllocatedOpSpan() { + fAllocatedOpSpan = true; + } void setAngleCoincidence() { fAngleCoincidence = true; @@ -159,6 +171,7 @@ private: SkOpCoincidence* fCoincidence; SkOpContourHead* fContourHead; int fNested; + bool fAllocatedOpSpan; bool fWindingFailed; bool fAngleCoincidence; Phase fPhase; diff --git a/tests/PathOpsAngleTest.cpp b/tests/PathOpsAngleTest.cpp index 12d1d5579a..fe39d40b9c 100644 --- a/tests/PathOpsAngleTest.cpp +++ b/tests/PathOpsAngleTest.cpp @@ -478,9 +478,9 @@ DEF_TEST(PathOpsAngleAfter, reporter) { void SkOpSegment::debugAddAngle(double startT, double endT) { SkOpPtT* startPtT = startT == 0 ? fHead.ptT() : startT == 1 ? fTail.ptT() - : this->addT(startT, nullptr); + : this->addT(startT); SkOpPtT* endPtT = endT == 0 ? fHead.ptT() : endT == 1 ? fTail.ptT() - : this->addT(endT, nullptr); + : this->addT(endT); SkOpAngle* angle = SkOpTAllocator::Allocate(this->globalState()->allocator()); SkOpSpanBase* startSpan = &fHead; while (startSpan->ptT() != startPtT) {