From: reed Date: Thu, 11 Dec 2014 15:07:37 +0000 (-0800) Subject: Defer saves() until they're needed X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~4482 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2ff1fcede1e9525285c5de1f35fb2dcb0fab32bd;p=platform%2Fupstream%2FlibSkiaSharp.git Defer saves() until they're needed patch from issue 759443006 at patchset 40001 (http://crrev.com/759443006#ps40001) BUG=skia: Review URL: https://codereview.chromium.org/767333002 --- diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index c4ec76a..d28336f 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -1265,6 +1265,7 @@ private: const SkSurfaceProps fProps; + int fSaveCount; // value returned by getSaveCount() int fSaveLayerCount; // number of successful saveLayer calls int fCullCount; // number of active culls @@ -1281,6 +1282,9 @@ private: bool fDeviceCMDirty; // cleared by updateDeviceCMCache() void updateDeviceCMCache(); + void doSave(); + void checkForDeferredSave(); + friend class SkDrawIter; // needs setupDrawForLayerDevice() friend class AutoDrawLooper; friend class SkLua; // needs top layer size and offset @@ -1334,12 +1338,12 @@ private: void internalDrawBitmapNine(const SkBitmap& bitmap, const SkIRect& center, const SkRect& dst, const SkPaint* paint); void internalDrawPaint(const SkPaint& paint); - int internalSaveLayer(const SkRect* bounds, const SkPaint* paint, - SaveFlags, bool justForImageFilter, SaveLayerStrategy strategy); + void internalSaveLayer(const SkRect* bounds, const SkPaint* paint, + SaveFlags, bool justForImageFilter, SaveLayerStrategy strategy); void internalDrawDevice(SkBaseDevice*, int x, int y, const SkPaint*); // shared by save() and saveLayer() - int internalSave(); + void internalSave(); void internalRestore(); static void DrawRect(const SkDraw& draw, const SkPaint& paint, const SkRect& r, SkScalar textSize); diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 083a8ed..294562a 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -170,8 +170,6 @@ private: */ class SkCanvas::MCRec { public: - SkRasterClip fRasterClip; - SkMatrix fMatrix; SkDrawFilter* fFilter; // the current filter (or null) DeviceCM* fLayer; /* If there are any layers in the stack, this points to the top-most @@ -180,22 +178,26 @@ public: reference counted, since the real owner is either our fLayer field, or a previous one in a lower level.) */ - DeviceCM* fTopLayer; + DeviceCM* fTopLayer; + SkRasterClip fRasterClip; + SkMatrix fMatrix; + int fDeferredSaveCount; MCRec(bool conservativeRasterClip) : fRasterClip(conservativeRasterClip) { - fMatrix.reset(); fFilter = NULL; fLayer = NULL; fTopLayer = NULL; + fMatrix.reset(); + fDeferredSaveCount = 0; // don't bother initializing fNext inc_rec(); } - MCRec(const MCRec& prev) : fRasterClip(prev.fRasterClip) { - fMatrix = prev.fMatrix; + MCRec(const MCRec& prev) : fRasterClip(prev.fRasterClip), fMatrix(prev.fMatrix) { fFilter = SkSafeRef(prev.fFilter); fLayer = NULL; fTopLayer = prev.fTopLayer; + fDeferredSaveCount = 0; // don't bother initializing fNext inc_rec(); @@ -413,6 +415,7 @@ SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) { fAllowSoftClip = true; fAllowSimplifyClip = false; fDeviceCMDirty = true; + fSaveCount = 1; fSaveLayerCount = 0; fCullCount = 0; fMetaData = NULL; @@ -785,21 +788,54 @@ void SkCanvas::updateDeviceCMCache() { /////////////////////////////////////////////////////////////////////////////// +void SkCanvas::checkForDeferredSave() { + if (fMCRec->fDeferredSaveCount > 0) { + fMCRec->fDeferredSaveCount -= 1; + this->doSave(); + } +} + int SkCanvas::getSaveCount() const { - return fMCStack.count(); +#ifdef SK_DEBUG + int count = 0; + SkDeque::Iter iter(fMCStack, SkDeque::Iter::kFront_IterStart); + for (;;) { + const MCRec* rec = (const MCRec*)iter.next(); + if (!rec) { + break; + } + count += 1 + rec->fDeferredSaveCount; + } + SkASSERT(count == fSaveCount); +#endif + return fSaveCount; } int SkCanvas::save() { + fSaveCount += 1; + fMCRec->fDeferredSaveCount += 1; + return this->getSaveCount() - 1; // return our prev value +} + +void SkCanvas::doSave() { this->willSave(); - return this->internalSave(); + this->internalSave(); } void SkCanvas::restore() { - // check for underflow - if (fMCStack.count() > 1) { - this->willRestore(); - this->internalRestore(); - this->didRestore(); + if (fMCRec->fDeferredSaveCount > 0) { + SkASSERT(fSaveCount > 1); + fSaveCount -= 1; + fMCRec->fDeferredSaveCount -= 1; + } else { + // check for underflow + if (fMCStack.count() > 1) { + this->willRestore(); + SkASSERT(fSaveCount > 1); + fSaveCount -= 1; + this->internalRestore(); + this->didRestore(); + } } } @@ -815,16 +851,12 @@ void SkCanvas::restoreToCount(int count) { } } -int SkCanvas::internalSave() { - int saveCount = this->getSaveCount(); // record this before the actual save - +void SkCanvas::internalSave() { MCRec* newTop = (MCRec*)fMCStack.push_back(); new (newTop) MCRec(*fMCRec); // balanced in restore() fMCRec = newTop; fClipStack.save(); - - return saveCount; } static bool bounds_affects_clip(SkCanvas::SaveFlags flags) { @@ -881,16 +913,19 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags, int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint) { SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag); - return this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy); + fSaveCount += 1; + this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy); + return this->getSaveCount() - 1; } -int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, - SaveFlags flags) { +int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags) { SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags); - return this->internalSaveLayer(bounds, paint, flags, false, strategy); + fSaveCount += 1; + this->internalSaveLayer(bounds, paint, flags, false, strategy); + return this->getSaveCount() - 1; } -int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags, +void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags, bool justForImageFilter, SaveLayerStrategy strategy) { #ifndef SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG flags |= kClipToLayer_SaveFlag; @@ -898,19 +933,19 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save // do this before we create the layer. We don't call the public save() since // that would invoke a possibly overridden virtual - int count = this->internalSave(); + this->internalSave(); fDeviceCMDirty = true; SkIRect ir; if (!this->clipRectBounds(bounds, flags, &ir, paint ? paint->getImageFilter() : NULL)) { - return count; + return; } // FIXME: do willSaveLayer() overriders returning kNoLayer_SaveLayerStrategy really care about // the clipRectBounds() call above? if (kNoLayer_SaveLayerStrategy == strategy) { - return count; + return; } // Kill the imagefilter if our device doesn't allow it @@ -919,7 +954,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save if (!this->getTopDevice()->allowImageFilter(paint->getImageFilter())) { if (justForImageFilter) { // early exit if the layer was just for the imageFilter - return count; + return; } SkPaint* p = lazyP.set(*paint); p->setImageFilter(NULL); @@ -934,7 +969,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save SkBaseDevice* device = this->getTopDevice(); if (NULL == device) { SkDebugf("Unable to find device for layer."); - return count; + return; } SkBaseDevice::Usage usage = SkBaseDevice::kSaveLayer_Usage; @@ -945,7 +980,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save fProps.pixelGeometry())); if (NULL == device) { SkDebugf("Unable to create device for layer."); - return count; + return; } device->setOrigin(ir.fLeft, ir.fTop); @@ -958,7 +993,6 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save fMCRec->fTopLayer = layer; // this field is NOT an owner of layer fSaveLayerCount += 1; - return count; } int SkCanvas::saveLayerAlpha(const SkRect* bounds, U8CPU alpha) { @@ -1115,6 +1149,7 @@ void SkCanvas::validateCull(const SkIRect& devCull) { #endif void SkCanvas::pushCull(const SkRect& cullRect) { + this->checkForDeferredSave(); ++fCullCount; this->onPushCull(cullRect); @@ -1286,6 +1321,7 @@ void SkCanvas::concat(const SkMatrix& matrix) { return; } + this->checkForDeferredSave(); fDeviceCMDirty = true; fCachedLocalClipBoundsDirty = true; fMCRec->fMatrix.preConcat(matrix); @@ -1294,6 +1330,7 @@ void SkCanvas::concat(const SkMatrix& matrix) { } void SkCanvas::setMatrix(const SkMatrix& matrix) { + this->checkForDeferredSave(); fDeviceCMDirty = true; fCachedLocalClipBoundsDirty = true; fMCRec->fMatrix = matrix; @@ -1310,6 +1347,7 @@ void SkCanvas::resetMatrix() { ////////////////////////////////////////////////////////////////////////////// void SkCanvas::clipRect(const SkRect& rect, SkRegion::Op op, bool doAA) { + this->checkForDeferredSave(); ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle; this->onClipRect(rect, op, edgeStyle); } @@ -1367,6 +1405,7 @@ static void rasterclip_path(SkRasterClip* rc, const SkCanvas* canvas, const SkPa } void SkCanvas::clipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) { + this->checkForDeferredSave(); ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle; if (rrect.isRect()) { this->onClipRect(rrect.getBounds(), op, edgeStyle); @@ -1402,6 +1441,7 @@ void SkCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle } void SkCanvas::clipPath(const SkPath& path, SkRegion::Op op, bool doAA) { + this->checkForDeferredSave(); ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle; SkRect r; if (!path.isInverseFillType() && path.isRect(&r)) { @@ -1484,6 +1524,7 @@ void SkCanvas::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edg } void SkCanvas::clipRegion(const SkRegion& rgn, SkRegion::Op op) { + this->checkForDeferredSave(); this->onClipRegion(rgn, op); } diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp index 778abb5..67c4229 100644 --- a/src/core/SkPictureRecord.cpp +++ b/src/core/SkPictureRecord.cpp @@ -324,10 +324,13 @@ void SkPictureRecord::fillRestoreOffsetPlaceholdersForCurrentStackLevel(uint32_t } #ifdef SK_DEBUG - // assert that the final offset value points to a save verb - uint32_t opSize; - DrawType drawOp = peek_op_and_size(&fWriter, -offset, &opSize); - SkASSERT(SAVE == drawOp || SAVE_LAYER == drawOp); + // offset of 0 has been disabled, so we skip it + if (offset > 0) { + // assert that the final offset value points to a save verb + uint32_t opSize; + DrawType drawOp = peek_op_and_size(&fWriter, -offset, &opSize); + SkASSERT(SAVE == drawOp || SAVE_LAYER == drawOp); + } #endif } diff --git a/tests/PictureBBHTest.cpp b/tests/PictureBBHTest.cpp index bc712f5..562d9b5 100644 --- a/tests/PictureBBHTest.cpp +++ b/tests/PictureBBHTest.cpp @@ -119,7 +119,8 @@ static void test_clear(skiatest::Reporter* r, SkBBHFactory* factory) { // Should be Clip - Save - Clear - Restore. // Buggy implementations might return 1 (just Clip) or 3 (Clip - Save - Restore). - REPORTER_ASSERT(r, dstPic->approximateOpCount() == 4); + // TODO: can we just search that it contains "clear"? + REPORTER_ASSERT(r, dstPic->approximateOpCount() == 4 || dstPic->approximateOpCount() == 2); } DEF_TEST(PictureBBH_Clear, r) { diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp index 2d9e90d..e830af4 100644 --- a/tests/RecordDrawTest.cpp +++ b/tests/RecordDrawTest.cpp @@ -30,6 +30,33 @@ private: int fCalls; }; +DEF_TEST(RecordDraw_LazySaves, r) { + // Record two commands. + SkRecord record; + SkRecorder recorder(&record, W, H); + + REPORTER_ASSERT(r, 0 == record.count()); + recorder.save(); + REPORTER_ASSERT(r, 0 == record.count()); // the save was not recorded (yet) + recorder.drawColor(SK_ColorRED); + REPORTER_ASSERT(r, 1 == record.count()); + recorder.scale(2, 2); + REPORTER_ASSERT(r, 3 == record.count()); // now we see the save + recorder.restore(); + REPORTER_ASSERT(r, 4 == record.count()); + + assert_type(r, record, 0); + assert_type (r, record, 1); + assert_type(r, record, 2); + assert_type (r, record, 3); + + recorder.save(); + recorder.save(); + recorder.restore(); + recorder.restore(); + REPORTER_ASSERT(r, 4 == record.count()); +} + DEF_TEST(RecordDraw_Abort, r) { // Record two commands. SkRecord record; @@ -43,26 +70,23 @@ DEF_TEST(RecordDraw_Abort, r) { JustOneDraw callback; SkRecordDraw(record, &canvas, NULL, NULL, 0, NULL/*bbh*/, &callback); - REPORTER_ASSERT(r, 3 == rerecord.count()); - assert_type (r, rerecord, 0); - assert_type(r, rerecord, 1); - assert_type (r, rerecord, 2); + REPORTER_ASSERT(r, 1 == count_instances_of_type(rerecord)); + REPORTER_ASSERT(r, 0 == count_instances_of_type(rerecord)); } DEF_TEST(RecordDraw_Unbalanced, r) { SkRecord record; SkRecorder recorder(&record, W, H); recorder.save(); // We won't balance this, but SkRecordDraw will for us. + recorder.scale(2, 2); SkRecord rerecord; SkRecorder canvas(&rerecord, W, H); SkRecordDraw(record, &canvas, NULL, NULL, 0, NULL/*bbh*/, NULL/*callback*/); - REPORTER_ASSERT(r, 4 == rerecord.count()); - assert_type (r, rerecord, 0); - assert_type (r, rerecord, 1); - assert_type (r, rerecord, 2); - assert_type (r, rerecord, 3); + int save_count = count_instances_of_type(rerecord); + int restore_count = count_instances_of_type(rerecord); + REPORTER_ASSERT(r, save_count == restore_count); } DEF_TEST(RecordDraw_SetMatrixClobber, r) { @@ -193,12 +217,9 @@ DEF_TEST(RecordDraw_PartialStartStop, r) { SkRecorder canvas(&rerecord, kWidth, kHeight); SkRecordPartialDraw(record, &canvas, NULL, 0, 1, 2, SkMatrix::I()); // replay just drawRect of r2 - REPORTER_ASSERT(r, 3 == rerecord.count()); - assert_type (r, rerecord, 0); - assert_type (r, rerecord, 1); - assert_type (r, rerecord, 2); - - const SkRecords::DrawRect* drawRect = assert_type(r, rerecord, 1); + REPORTER_ASSERT(r, 1 == count_instances_of_type(rerecord)); + int index = find_first_instances_of_type(rerecord); + const SkRecords::DrawRect* drawRect = assert_type(r, rerecord, index); REPORTER_ASSERT(r, drawRect->rect == r2); } diff --git a/tests/RecordOptsTest.cpp b/tests/RecordOptsTest.cpp index c5c4471..cdc0350 100644 --- a/tests/RecordOptsTest.cpp +++ b/tests/RecordOptsTest.cpp @@ -16,26 +16,19 @@ static const int W = 1920, H = 1080; -DEF_TEST(RecordOpts_NoopDrawSaveRestore, r) { +DEF_TEST(RecordOpts_NoopDraw, r) { SkRecord record; SkRecorder recorder(&record, W, H); - // The save and restore are pointless if there's only draw commands in the middle. - recorder.save(); - recorder.drawRect(SkRect::MakeWH(200, 200), SkPaint()); - recorder.drawRect(SkRect::MakeWH(300, 300), SkPaint()); - recorder.drawRect(SkRect::MakeWH(100, 100), SkPaint()); - recorder.restore(); + recorder.drawRect(SkRect::MakeWH(200, 200), SkPaint()); + recorder.drawRect(SkRect::MakeWH(300, 300), SkPaint()); + recorder.drawRect(SkRect::MakeWH(100, 100), SkPaint()); - record.replace(2); // NoOps should be allowed. + record.replace(1); // NoOps should be allowed. SkRecordNoopSaveRestores(&record); - assert_type(r, record, 0); - assert_type(r, record, 1); - assert_type(r, record, 2); - assert_type(r, record, 3); - assert_type(r, record, 4); + REPORTER_ASSERT(r, 2 == count_instances_of_type(record)); } DEF_TEST(RecordOpts_SingleNoopSaveRestore, r) { @@ -70,7 +63,7 @@ DEF_TEST(RecordOpts_NoopSaveRestores, r) { recorder.restore(); SkRecordNoopSaveRestores(&record); - for (unsigned index = 0; index < 8; index++) { + for (unsigned index = 0; index < record.count(); index++) { assert_type(r, record, index); } } @@ -86,10 +79,22 @@ DEF_TEST(RecordOpts_SaveSaveLayerRestoreRestore, r) { recorder.restore(); SkRecordNoopSaveRestores(&record); - assert_type (r, record, 0); - assert_type(r, record, 1); - assert_type (r, record, 2); - assert_type (r, record, 3); + switch (record.count()) { + case 4: + assert_type (r, record, 0); + assert_type(r, record, 1); + assert_type (r, record, 2); + assert_type (r, record, 3); + break; + case 2: + assert_type(r, record, 0); + assert_type (r, record, 1); + break; + case 0: + break; + default: + REPORTER_ASSERT(r, false); + } } static void assert_savelayer_restore(skiatest::Reporter* r, diff --git a/tests/RecordPatternTest.cpp b/tests/RecordPatternTest.cpp index 5f4d006..1f5ce2c 100644 --- a/tests/RecordPatternTest.cpp +++ b/tests/RecordPatternTest.cpp @@ -75,56 +75,19 @@ DEF_TEST(RecordPattern_Star, r) { SkRecord record; SkRecorder recorder(&record, 1920, 1200); - - recorder.save(); - recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 0)); + int index = 0; recorder.save(); recorder.clipRect(SkRect::MakeWH(300, 200)); recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 2)); + REPORTER_ASSERT(r, pattern.match(&record, index)); + index += 3; recorder.save(); recorder.clipRect(SkRect::MakeWH(300, 200)); recorder.clipRect(SkRect::MakeWH(100, 100)); recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 5)); -} - -DEF_TEST(RecordPattern_IsDraw, r) { - Pattern3, IsDraw, Is > pattern; - - SkRecord record; - SkRecorder recorder(&record, 1920, 1200); - - recorder.save(); - recorder.clipRect(SkRect::MakeWH(300, 200)); - recorder.restore(); - - REPORTER_ASSERT(r, !pattern.match(&record, 0)); - - SkPaint paint; - - recorder.save(); - paint.setColor(0xEEAA8822); - recorder.drawRect(SkRect::MakeWH(300, 200), paint); - recorder.restore(); - - recorder.save(); - paint.setColor(0xFACEFACE); - recorder.drawPaint(paint); - recorder.restore(); - - REPORTER_ASSERT(r, pattern.match(&record, 3)); - REPORTER_ASSERT(r, pattern.first() != NULL); - REPORTER_ASSERT(r, pattern.second()->getColor() == 0xEEAA8822); - REPORTER_ASSERT(r, pattern.third() != NULL); - - REPORTER_ASSERT(r, pattern.match(&record, 6)); - REPORTER_ASSERT(r, pattern.first() != NULL); - REPORTER_ASSERT(r, pattern.second()->getColor() == 0xFACEFACE); - REPORTER_ASSERT(r, pattern.third() != NULL); + REPORTER_ASSERT(r, pattern.match(&record, index)); } DEF_TEST(RecordPattern_Complex, r) { @@ -136,54 +99,39 @@ DEF_TEST(RecordPattern_Complex, r) { SkRecord record; SkRecorder recorder(&record, 1920, 1200); + unsigned start, begin, end; - recorder.save(); - recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 0) == 2); - - recorder.save(); - recorder.save(); - recorder.restore(); - recorder.restore(); - REPORTER_ASSERT(r, !pattern.match(&record, 2)); - REPORTER_ASSERT(r, pattern.match(&record, 3) == 5); - + start = record.count(); recorder.save(); recorder.clipRect(SkRect::MakeWH(300, 200)); recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 6) == 9); + REPORTER_ASSERT(r, pattern.match(&record, 0) == record.count()); + end = start; + REPORTER_ASSERT(r, pattern.search(&record, &begin, &end)); + REPORTER_ASSERT(r, begin == start); + REPORTER_ASSERT(r, end == record.count()); + start = record.count(); recorder.save(); recorder.clipRect(SkRect::MakeWH(300, 200)); recorder.drawRect(SkRect::MakeWH(100, 3000), SkPaint()); recorder.restore(); - REPORTER_ASSERT(r, !pattern.match(&record, 9)); + REPORTER_ASSERT(r, !pattern.match(&record, start)); + end = start; + REPORTER_ASSERT(r, !pattern.search(&record, &begin, &end)); + start = record.count(); recorder.save(); recorder.pushCull(SkRect::MakeWH(300, 200)); recorder.clipRect(SkRect::MakeWH(300, 200)); recorder.clipRect(SkRect::MakeWH(100, 400)); recorder.popCull(); recorder.restore(); - REPORTER_ASSERT(r, pattern.match(&record, 13) == 19); - - // Same as above, but using pattern.search to step through matches. - unsigned begin, end = 0; - REPORTER_ASSERT(r, pattern.search(&record, &begin, &end)); - REPORTER_ASSERT(r, begin == 0); - REPORTER_ASSERT(r, end == 2); - - REPORTER_ASSERT(r, pattern.search(&record, &begin, &end)); - REPORTER_ASSERT(r, begin == 3); - REPORTER_ASSERT(r, end == 5); - - REPORTER_ASSERT(r, pattern.search(&record, &begin, &end)); - REPORTER_ASSERT(r, begin == 6); - REPORTER_ASSERT(r, end == 9); - + REPORTER_ASSERT(r, pattern.match(&record, start) == record.count()); + end = start; REPORTER_ASSERT(r, pattern.search(&record, &begin, &end)); - REPORTER_ASSERT(r, begin == 13); - REPORTER_ASSERT(r, end == 19); + REPORTER_ASSERT(r, begin == start); + REPORTER_ASSERT(r, end == record.count()); REPORTER_ASSERT(r, !pattern.search(&record, &begin, &end)); } diff --git a/tests/RecordReplaceDrawTest.cpp b/tests/RecordReplaceDrawTest.cpp index f1ebf82..e0e9466 100644 --- a/tests/RecordReplaceDrawTest.cpp +++ b/tests/RecordReplaceDrawTest.cpp @@ -52,10 +52,18 @@ DEF_TEST(RecordReplaceDraw_Abort, r) { JustOneDraw callback; GrRecordReplaceDraw(pic, &canvas, NULL, SkMatrix::I(), &callback); - REPORTER_ASSERT(r, 3 == rerecord.count()); - assert_type(r, rerecord, 0); - assert_type(r, rerecord, 1); - assert_type(r, rerecord, 2); + switch (rerecord.count()) { + case 3: + assert_type(r, rerecord, 0); + assert_type(r, rerecord, 1); + assert_type(r, rerecord, 2); + break; + case 1: + assert_type(r, rerecord, 0); + break; + default: + REPORTER_ASSERT(r, false); + } } // Make sure GrRecordReplaceDraw balances unbalanced saves @@ -68,7 +76,7 @@ DEF_TEST(RecordReplaceDraw_Unbalanced, r) { // We won't balance this, but GrRecordReplaceDraw will for us. canvas->save(); - + canvas->scale(2, 2); pic.reset(recorder.endRecording()); } @@ -77,11 +85,8 @@ DEF_TEST(RecordReplaceDraw_Unbalanced, r) { GrRecordReplaceDraw(pic, &canvas, NULL, SkMatrix::I(), NULL/*callback*/); - REPORTER_ASSERT(r, 4 == rerecord.count()); - assert_type(r, rerecord, 0); - assert_type(r, rerecord, 1); - assert_type(r, rerecord, 2); - assert_type(r, rerecord, 3); + // ensure rerecord is balanced (in this case by checking that the count is even) + REPORTER_ASSERT(r, (rerecord.count() & 1) == 0); } // Test out the layer replacement functionality with and w/o a BBH @@ -127,14 +132,22 @@ void test_replacements(skiatest::Reporter* r, GrContext* context, bool useBBH) { SkRecorder canvas(&rerecord, kWidth, kHeight); GrRecordReplaceDraw(pic, &canvas, layerCache, SkMatrix::I(), NULL/*callback*/); - REPORTER_ASSERT(r, 7 == rerecord.count()); - assert_type(r, rerecord, 0); - assert_type(r, rerecord, 1); - assert_type(r, rerecord, 2); - assert_type(r, rerecord, 3); - assert_type(r, rerecord, 4); - assert_type(r, rerecord, 5); - assert_type(r, rerecord, 6); + int recount = rerecord.count(); + REPORTER_ASSERT(r, 5 == recount || 7 == recount); + + int index = 0; + if (7 == recount) { + assert_type(r, rerecord, 0); + index += 1; + } + assert_type(r, rerecord, index + 0); + assert_type(r, rerecord, index + 1); + assert_type(r, rerecord, index + 2); + assert_type(r, rerecord, index + 3); + assert_type(r, rerecord, index + 4); + if (7 == recount) { + assert_type(r, rerecord, 6); + } } DEF_GPUTEST(RecordReplaceDraw, r, factory) { diff --git a/tests/RecordTestUtils.h b/tests/RecordTestUtils.h index 0575b83..4bab8e4 100644 --- a/tests/RecordTestUtils.h +++ b/tests/RecordTestUtils.h @@ -28,4 +28,28 @@ static const T* assert_type(skiatest::Reporter* r, const SkRecord& record, unsig return reader.ptr; } +template struct MatchType { + template int operator()(const T&) { return 0; } + int operator()(const DrawT&) { return 1; } +}; + +template int count_instances_of_type(const SkRecord& record) { + MatchType matcher; + int counter = 0; + for (unsigned i = 0; i < record.count(); i++) { + counter += record.visit(i, matcher); + } + return counter; +} + +template int find_first_instances_of_type(const SkRecord& record) { + MatchType matcher; + for (unsigned i = 0; i < record.count(); i++) { + if (record.visit(i, matcher)) { + return i; + } + } + return -1; +} + #endif//RecordTestUtils_DEFINED