From 5e1a7f2cc621d357da5c62a7bc4ef750d94b96f3 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 12 Feb 2014 17:44:30 +0000 Subject: [PATCH] Revert of r13379 (Move fLastMoveToIndex from SkPath to SkPathRef - https://codereview.chromium.org/146913002/) due to image quality regression in Chromium. See crbug.com/343123 (Regression - UI issue observed for any "Notification infobars" in Chrome browser) R=bsalomon@google.com TBR=bsalomon@google.com Author: robertphillips@google.com Review URL: https://codereview.chromium.org/137863006 git-svn-id: http://skia.googlecode.com/svn/trunk@13421 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkPath.h | 6 ++--- include/core/SkPathRef.h | 14 ------------ src/core/SkPath.cpp | 48 ++++++++++++++++++++++++++++++++++++---- src/core/SkPathRef.cpp | 39 -------------------------------- 4 files changed, 46 insertions(+), 61 deletions(-) diff --git a/include/core/SkPath.h b/include/core/SkPath.h index 0f43fe8e3..d78cd8829 100644 --- a/include/core/SkPath.h +++ b/include/core/SkPath.h @@ -978,6 +978,7 @@ private: SkAutoTUnref fPathRef; + int fLastMoveToIndex; uint8_t fFillType; mutable uint8_t fConvexity; mutable uint8_t fDirection; @@ -1013,10 +1014,7 @@ private: // SkPath path; path.lineTo(...); <--- need a leading moveTo(0, 0) // SkPath path; ... path.close(); path.lineTo(...) <-- need a moveTo(previous moveTo) // - void injectMoveToIfNeeded() { - SkPathRef::Editor ed(&fPathRef); - ed.injectMoveToIfNeeded(); - } + inline void injectMoveToIfNeeded(); inline bool hasOnlyMoveTos() const; diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index 2e8c5474f..880271424 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -104,13 +104,6 @@ public: void setBounds(const SkRect& rect) { fPathRef->setBounds(rect); } - // In some cases we need to inject a leading moveTo before we add points - // for lineTo, quadTo, conicTo, cubicTo - // - // SkPath path; path.lineTo(...); <--- need a leading moveTo(0, 0) - // SkPath path; ... path.close(); path.lineTo(...) <-- need a moveTo(previous moveTo) - void injectMoveToIfNeeded() { fPathRef->injectMoveToIfNeeded(); } - private: SkPathRef* fPathRef; }; @@ -261,9 +254,6 @@ public: uint32_t genID() const; private: - // flag to require a moveTo if we begin with something else, like lineTo etc. - static const int kINITIAL_LASTMOVETOINDEX_VALUE = ~0; - enum SerializationOffsets { kIsFinite_SerializationShift = 25, // requires 1 bit kIsOval_SerializationShift = 24, // requires 1 bit @@ -271,7 +261,6 @@ private: }; SkPathRef() { - fLastMoveToIndex = kINITIAL_LASTMOVETOINDEX_VALUE; fBoundsIsDirty = true; // this also invalidates fIsFinite fPointCnt = 0; fVerbCnt = 0; @@ -359,8 +348,6 @@ private: SkDEBUGCODE(this->validate();) } - void injectMoveToIfNeeded(); - /** * Increases the verb count by numVbs and point count by the required amount. * The new points are uninitialized. All the new verbs are set to the specified @@ -446,7 +433,6 @@ private: }; mutable SkRect fBounds; - int fLastMoveToIndex; uint8_t fSegmentMask; mutable uint8_t fBoundsIsDirty; mutable SkBool8 fIsFinite; // only meaningful if bounds are valid diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index ea93963a6..9f94b7268 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -122,6 +122,9 @@ private: //////////////////////////////////////////////////////////////////////////// +// flag to require a moveTo if we begin with something else, like lineTo etc. +#define INITIAL_LASTMOVETOINDEX_VALUE ~0 + SkPath::SkPath() : fPathRef(SkPathRef::CreateEmpty()) #ifdef SK_BUILD_FOR_ANDROID @@ -133,6 +136,7 @@ SkPath::SkPath() void SkPath::resetFields() { //fPathRef is assumed to have been emptied by the caller. + fLastMoveToIndex = INITIAL_LASTMOVETOINDEX_VALUE; fFillType = kWinding_FillType; fConvexity = kUnknown_Convexity; fDirection = kUnknown_Direction; @@ -170,6 +174,7 @@ SkPath& SkPath::operator=(const SkPath& that) { void SkPath::copyFields(const SkPath& that) { //fPathRef is assumed to have been set by the caller. + fLastMoveToIndex = that.fLastMoveToIndex; fFillType = that.fFillType; fConvexity = that.fConvexity; fDirection = that.fDirection; @@ -187,6 +192,7 @@ void SkPath::swap(SkPath& that) { if (this != &that) { fPathRef.swap(&that.fPathRef); + SkTSwap(fLastMoveToIndex, that.fLastMoveToIndex); SkTSwap(fFillType, that.fFillType); SkTSwap(fConvexity, that.fConvexity); SkTSwap(fDirection, that.fDirection); @@ -661,6 +667,9 @@ void SkPath::moveTo(SkScalar x, SkScalar y) { SkPathRef::Editor ed(&fPathRef); + // remember our index + fLastMoveToIndex = fPathRef->countPoints(); + ed.growForVerb(kMove_Verb)->set(x, y); } @@ -670,11 +679,26 @@ void SkPath::rMoveTo(SkScalar x, SkScalar y) { this->moveTo(pt.fX + x, pt.fY + y); } +void SkPath::injectMoveToIfNeeded() { + if (fLastMoveToIndex < 0) { + SkScalar x, y; + if (fPathRef->countVerbs() == 0) { + x = y = 0; + } else { + const SkPoint& pt = fPathRef->atPoint(~fLastMoveToIndex); + x = pt.fX; + y = pt.fY; + } + this->moveTo(x, y); + } +} + void SkPath::lineTo(SkScalar x, SkScalar y) { SkDEBUGCODE(this->validate();) + this->injectMoveToIfNeeded(); + SkPathRef::Editor ed(&fPathRef); - ed.injectMoveToIfNeeded(); ed.growForVerb(kLine_Verb)->set(x, y); DIRTY_AFTER_EDIT; @@ -690,8 +714,9 @@ void SkPath::rLineTo(SkScalar x, SkScalar y) { void SkPath::quadTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2) { SkDEBUGCODE(this->validate();) + this->injectMoveToIfNeeded(); + SkPathRef::Editor ed(&fPathRef); - ed.injectMoveToIfNeeded(); SkPoint* pts = ed.growForVerb(kQuad_Verb); pts[0].set(x1, y1); pts[1].set(x2, y2); @@ -719,8 +744,9 @@ void SkPath::conicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2, } else { SkDEBUGCODE(this->validate();) + this->injectMoveToIfNeeded(); + SkPathRef::Editor ed(&fPathRef); - ed.injectMoveToIfNeeded(); SkPoint* pts = ed.growForVerb(kConic_Verb, w); pts[0].set(x1, y1); pts[1].set(x2, y2); @@ -741,8 +767,9 @@ void SkPath::cubicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2, SkScalar x3, SkScalar y3) { SkDEBUGCODE(this->validate();) + this->injectMoveToIfNeeded(); + SkPathRef::Editor ed(&fPathRef); - ed.injectMoveToIfNeeded(); SkPoint* pts = ed.growForVerb(kCubic_Verb); pts[0].set(x1, y1); pts[1].set(x2, y2); @@ -783,6 +810,15 @@ void SkPath::close() { break; } } + + // signal that we need a moveTo to follow us (unless we're done) +#if 0 + if (fLastMoveToIndex >= 0) { + fLastMoveToIndex = ~fLastMoveToIndex; + } +#else + fLastMoveToIndex ^= ~fLastMoveToIndex >> (8 * sizeof(fLastMoveToIndex) - 1); +#endif } /////////////////////////////////////////////////////////////////////////////// @@ -824,6 +860,8 @@ void SkPath::addPoly(const SkPoint pts[], int count, bool close) { return; } + fLastMoveToIndex = fPathRef->countPoints(); + // +close makes room for the extra kClose_Verb SkPathRef::Editor ed(&fPathRef, count+close, count); @@ -1320,6 +1358,8 @@ void SkPath::addArc(const SkRect& oval, SkScalar startAngle, SkScalar sweepAngle SkDEBUGCODE(this->validate();) SkASSERT(count & 1); + fLastMoveToIndex = fPathRef->countPoints(); + SkPathRef::Editor ed(&fPathRef, 1+(count-1)/2, count); ed.growForVerb(kMove_Verb)->set(pts[0].fX, pts[0].fY); diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index 1272cf259..161eb8041 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -154,7 +154,6 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer) { void SkPathRef::Rewind(SkAutoTUnref* pathRef) { if ((*pathRef)->unique()) { SkDEBUGCODE((*pathRef)->validate();) - (*pathRef)->fLastMoveToIndex = kINITIAL_LASTMOVETOINDEX_VALUE; (*pathRef)->fBoundsIsDirty = true; // this also invalidates fIsFinite (*pathRef)->fVerbCnt = 0; (*pathRef)->fPointCnt = 0; @@ -277,20 +276,6 @@ void SkPathRef::copy(const SkPathRef& ref, SkDEBUGCODE(this->validate();) } -void SkPathRef::injectMoveToIfNeeded() { - if (fLastMoveToIndex < 0) { - SkScalar x, y; - if (this->countVerbs() == 0) { - x = y = 0; - } else { - const SkPoint& pt = this->atPoint(~fLastMoveToIndex); - x = pt.fX; - y = pt.fY; - } - this->growForVerb(SkPath::kMove_Verb, 0)->set(x, y); - } -} - SkPoint* SkPathRef::growForRepeatedVerb(int /*SkPath::Verb*/ verb, int numVbs, SkScalar** weights) { @@ -299,16 +284,11 @@ SkPoint* SkPathRef::growForRepeatedVerb(int /*SkPath::Verb*/ verb, // future this will appear to have been a fluke... static const unsigned int kMIN_COUNT_FOR_MEMSET_TO_BE_FAST = 16; - if (numVbs <= 0) { - return NULL; - } - SkDEBUGCODE(this->validate();) int pCnt; bool dirtyAfterEdit = true; switch (verb) { case SkPath::kMove_Verb: - fLastMoveToIndex = fPointCnt + numVbs - 1; pCnt = numVbs; dirtyAfterEdit = false; break; @@ -330,8 +310,6 @@ SkPoint* SkPathRef::growForRepeatedVerb(int /*SkPath::Verb*/ verb, break; case SkPath::kClose_Verb: SkDEBUGFAIL("growForRepeatedVerb called for kClose_Verb"); - // signal that we need a moveTo to follow us (unless we're done) - fLastMoveToIndex ^= ~fLastMoveToIndex >> (8 * sizeof(fLastMoveToIndex) - 1); pCnt = 0; dirtyAfterEdit = false; break; @@ -383,8 +361,6 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb, SkScalar weight) { bool dirtyAfterEdit = true; switch (verb) { case SkPath::kMove_Verb: - // remember our index - fLastMoveToIndex = fPointCnt; pCnt = 1; dirtyAfterEdit = false; break; @@ -405,8 +381,6 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb, SkScalar weight) { pCnt = 3; break; case SkPath::kClose_Verb: - // signal that we need a moveTo to follow us (unless we're done) - fLastMoveToIndex ^= ~fLastMoveToIndex >> (8 * sizeof(fLastMoveToIndex) - 1); pCnt = 0; dirtyAfterEdit = false; break; @@ -486,34 +460,23 @@ void SkPathRef::validate() const { #ifdef SK_DEBUG_PATH uint32_t mask = 0; - int lastMoveToIndex = kINITIAL_LASTMOVETOINDEX_VALUE; - int pointCnt = 0; for (int i = 0; i < fVerbCnt; ++i) { switch (fVerbs[~i]) { case SkPath::kMove_Verb: - lastMoveToIndex = pointCnt; - ++pointCnt; break; case SkPath::kLine_Verb: mask |= SkPath::kLine_SegmentMask; - ++pointCnt; break; case SkPath::kQuad_Verb: mask |= SkPath::kQuad_SegmentMask; - pointCnt += 2; break; case SkPath::kConic_Verb: mask |= SkPath::kConic_SegmentMask; - pointCnt += 2; break; case SkPath::kCubic_Verb: mask |= SkPath::kCubic_SegmentMask; - pointCnt += 3; break; case SkPath::kClose_Verb: - if (lastMoveToIndex >= 0) { - lastMoveToIndex = ~lastMoveToIndex; - } break; case SkPath::kDone_Verb: SkDEBUGFAIL("Done verb shouldn't be recorded."); @@ -524,8 +487,6 @@ void SkPathRef::validate() const { } } SkASSERT(mask == fSegmentMask); - SkASSERT(lastMoveToIndex == fLastMoveToIndex); - SkASSERT(pointCnt == fPointCnt); #endif // SK_DEBUG_PATH } #endif