From c0d4aa2088a0788f9df221497945d2ba1b342f44 Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Wed, 13 Apr 2011 21:12:04 +0000 Subject: [PATCH] fix asMode() to always succeed if the xfermode was built from a Mode update dox to reflect this update test git-svn-id: http://skia.googlecode.com/svn/trunk@1121 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkXfermode.h | 16 ++++---- src/core/SkXfermode.cpp | 94 ++++++++++++++++++----------------------------- tests/XfermodeTest.cpp | 25 ++++++++++--- 3 files changed, 61 insertions(+), 74 deletions(-) diff --git a/include/core/SkXfermode.h b/include/core/SkXfermode.h index 155d3f8..506f336 100644 --- a/include/core/SkXfermode.h +++ b/include/core/SkXfermode.h @@ -115,10 +115,10 @@ public: kLastMode = kExclusion_Mode }; - /** If the xfermode is one of the modes in the Mode enum, then asMode() - returns true and sets (if not null) mode accordingly. - This is a better version of IsMode(), which is only able to report - about modes that are expressible as coefficients. + /** + * If the xfermode is one of the modes in the Mode enum, then asMode() + * returns true and sets (if not null) mode accordingly. Otherwise it + * returns false and ignores the mode parameter. */ virtual bool asMode(Mode* mode); @@ -138,10 +138,9 @@ public: */ static SkXfermodeProc16 GetProc16(Mode mode, SkColor srcColor); - /** If the specified xfermode advertises itself as one of the porterduff - modes (via SkXfermode::Coeff), return true and if not null, set mode - to the corresponding porterduff mode. If it is not recognized as a one, - return false and ignore the mode parameter. + /** + * The same as calling xfermode->asMode(mode), except that this also checks + * if the xfermode is NULL, and if so, treats its as kSrcOver_Mode. */ static bool IsMode(SkXfermode*, Mode* mode); @@ -189,7 +188,6 @@ public: // overrides from SkFlattenable virtual Factory getFactory() { return CreateProc; } virtual void flatten(SkFlattenableWriteBuffer&); - virtual bool asMode(SkXfermode::Mode* mode); protected: SkProcXfermode(SkFlattenableReadBuffer&); diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp index 9d54bf6..87d6078 100644 --- a/src/core/SkXfermode.cpp +++ b/src/core/SkXfermode.cpp @@ -469,7 +469,7 @@ bool SkXfermode::asCoeff(Coeff* src, Coeff* dst) { } bool SkXfermode::asMode(Mode* mode) { - return IsMode(this, mode); + return false; } SkPMColor SkXfermode::xferColor(SkPMColor src, SkPMColor dst) { @@ -706,28 +706,31 @@ void SkProcXfermode::flatten(SkFlattenableWriteBuffer& buffer) { buffer.writeFunctionPtr((void*)fProc); } -bool SkProcXfermode::asMode(SkXfermode::Mode* mode) { - for (size_t i = 0; i < SK_ARRAY_COUNT(gProcCoeffs); i++) { - if (gProcCoeffs[i].fProc == fProc) { - if (mode) { - *mode = static_cast(i); - } - return true; - } - } - return false; -} - /////////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////// class SkProcCoeffXfermode : public SkProcXfermode { public: - SkProcCoeffXfermode(SkXfermodeProc proc, Coeff sc, Coeff dc) - : INHERITED(proc), fSrcCoeff(sc), fDstCoeff(dc) { + SkProcCoeffXfermode(const ProcCoeff& rec, Mode mode) + : INHERITED(rec.fProc) { + fMode = mode; + // these may be valid, or may be CANNOT_USE_COEFF + fSrcCoeff = rec.fSC; + fDstCoeff = rec.fDC; } - + + virtual bool asMode(Mode* mode) { + if (mode) { + *mode = fMode; + } + return true; + } + virtual bool asCoeff(Coeff* sc, Coeff* dc) { + if (CANNOT_USE_COEFF == fSrcCoeff) { + return false; + } + if (sc) { *sc = fSrcCoeff; } @@ -740,6 +743,7 @@ public: virtual Factory getFactory() { return CreateProc; } virtual void flatten(SkFlattenableWriteBuffer& buffer) { this->INHERITED::flatten(buffer); + buffer.write32(fMode); buffer.write32(fSrcCoeff); buffer.write32(fDstCoeff); } @@ -747,11 +751,13 @@ public: protected: SkProcCoeffXfermode(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) { + fMode = (SkXfermode::Mode)buffer.readU32(); fSrcCoeff = (Coeff)buffer.readU32(); fDstCoeff = (Coeff)buffer.readU32(); } private: + Mode fMode; Coeff fSrcCoeff, fDstCoeff; static SkFlattenable* CreateProc(SkFlattenableReadBuffer& buffer) { @@ -764,8 +770,7 @@ private: class SkClearXfermode : public SkProcCoeffXfermode { public: - SkClearXfermode() : SkProcCoeffXfermode(clear_modeproc, - kZero_Coeff, kZero_Coeff) {} + SkClearXfermode(const ProcCoeff& rec) : SkProcCoeffXfermode(rec, kClear_Mode) {} virtual void xfer32(SK_RESTRICT SkPMColor dst[], const SK_RESTRICT SkPMColor[], int count, @@ -819,8 +824,7 @@ private: class SkSrcXfermode : public SkProcCoeffXfermode { public: - SkSrcXfermode() : SkProcCoeffXfermode(src_modeproc, - kOne_Coeff, kZero_Coeff) {} + SkSrcXfermode(const ProcCoeff& rec) : SkProcCoeffXfermode(rec, kSrc_Mode) {} virtual void xfer32(SK_RESTRICT SkPMColor dst[], const SK_RESTRICT SkPMColor src[], int count, @@ -878,8 +882,7 @@ private: class SkDstInXfermode : public SkProcCoeffXfermode { public: - SkDstInXfermode() : SkProcCoeffXfermode(dstin_modeproc, - kZero_Coeff, kSA_Coeff) {} + SkDstInXfermode(const ProcCoeff& rec) : SkProcCoeffXfermode(rec, kDstIn_Mode) {} virtual void xfer32(SK_RESTRICT SkPMColor dst[], const SK_RESTRICT SkPMColor src[], int count, @@ -915,8 +918,7 @@ private: class SkDstOutXfermode : public SkProcCoeffXfermode { public: - SkDstOutXfermode() : SkProcCoeffXfermode(dstout_modeproc, - kZero_Coeff, kISA_Coeff) {} + SkDstOutXfermode(const ProcCoeff& rec) : SkProcCoeffXfermode(rec, kDstOut_Mode) {} virtual void xfer32(SK_RESTRICT SkPMColor dst[], const SK_RESTRICT SkPMColor src[], int count, @@ -957,29 +959,21 @@ SkXfermode* SkXfermode::Create(Mode mode) { SkASSERT(SK_ARRAY_COUNT(gProcCoeffs) == kModeCount); SkASSERT((unsigned)mode < kModeCount); + const ProcCoeff& rec = gProcCoeffs[mode]; + switch (mode) { case kClear_Mode: - return SkNEW(SkClearXfermode); + return SkNEW_ARGS(SkClearXfermode, (rec)); case kSrc_Mode: - return SkNEW(SkSrcXfermode); + return SkNEW_ARGS(SkSrcXfermode, (rec)); case kSrcOver_Mode: return NULL; case kDstIn_Mode: - return SkNEW(SkDstInXfermode); + return SkNEW_ARGS(SkDstInXfermode, (rec)); case kDstOut_Mode: - return SkNEW(SkDstOutXfermode); - // use the table - default: { - const ProcCoeff& rec = gProcCoeffs[mode]; - if ((unsigned)rec.fSC < SkXfermode::kCoeffCount && - (unsigned)rec.fDC < SkXfermode::kCoeffCount) { - return SkNEW_ARGS(SkProcCoeffXfermode, (rec.fProc, - rec.fSC, - rec.fDC)); - } else { - return SkNEW_ARGS(SkProcXfermode, (rec.fProc)); - } - } + return SkNEW_ARGS(SkDstOutXfermode, (rec)); + default: + return SkNEW_ARGS(SkProcCoeffXfermode, (rec, mode)); } } @@ -990,25 +984,7 @@ bool SkXfermode::IsMode(SkXfermode* xfer, Mode* mode) { } return true; } - - SkXfermode::Coeff sc, dc; - if (xfer->asCoeff(&sc, &dc)) { - SkASSERT((unsigned)sc < (unsigned)SkXfermode::kCoeffCount); - SkASSERT((unsigned)dc < (unsigned)SkXfermode::kCoeffCount); - - const ProcCoeff* rec = gProcCoeffs; - for (size_t i = 0; i < SK_ARRAY_COUNT(gProcCoeffs); i++) { - if (rec[i].fSC == sc && rec[i].fDC == dc) { - if (mode) { - *mode = static_cast(i); - } - return true; - } - } - } - - // no coefficients, or not found in our table - return false; + return xfer->asMode(mode); } SkXfermodeProc SkXfermode::GetProc(Mode mode) { diff --git a/tests/XfermodeTest.cpp b/tests/XfermodeTest.cpp index 6ab6cd7..b552ce0 100644 --- a/tests/XfermodeTest.cpp +++ b/tests/XfermodeTest.cpp @@ -6,16 +6,27 @@ SkPMColor bogusXfermodeProc(SkPMColor src, SkPMColor dst) { return 42; } +#define ILLEGAL_MODE ((SkXfermode::Mode)-1) + static void test_asMode(skiatest::Reporter* reporter) { for (int mode = 0; mode <= SkXfermode::kLastMode; mode++) { SkXfermode* xfer = SkXfermode::Create((SkXfermode::Mode) mode); - SkXfermode::Mode reportedMode = (SkXfermode::Mode) -1; - REPORTER_ASSERT(reporter, - xfer != NULL || mode == SkXfermode::kSrcOver_Mode); + + SkXfermode::Mode reportedMode = ILLEGAL_MODE; + REPORTER_ASSERT(reporter, reportedMode != mode); + + // test IsMode + REPORTER_ASSERT(reporter, SkXfermode::IsMode(xfer, &reportedMode)); + REPORTER_ASSERT(reporter, reportedMode == mode); + + // repeat that test, but with asMode instead if (xfer) { - REPORTER_ASSERT(reporter, xfer->asMode(&reportedMode)); - REPORTER_ASSERT(reporter, reportedMode == mode); - xfer->unref(); + reportedMode = (SkXfermode::Mode) -1; + REPORTER_ASSERT(reporter, xfer->asMode(&reportedMode)); + REPORTER_ASSERT(reporter, reportedMode == mode); + xfer->unref(); + } else { + REPORTER_ASSERT(reporter, SkXfermode::kSrcOver_Mode == mode); } } @@ -23,6 +34,8 @@ static void test_asMode(skiatest::Reporter* reporter) { SkXfermode::Mode reportedMode = (SkXfermode::Mode) -1; REPORTER_ASSERT(reporter, !bogusXfer->asMode(&reportedMode)); REPORTER_ASSERT(reporter, reportedMode == -1); + REPORTER_ASSERT(reporter, !SkXfermode::IsMode(bogusXfer, &reportedMode)); + REPORTER_ASSERT(reporter, reportedMode == -1); bogusXfer->unref(); } -- 2.7.4