Defer saves() until they're needed
authorreed <reed@google.com>
Thu, 11 Dec 2014 15:07:37 +0000 (07:07 -0800)
committerCommit bot <commit-bot@chromium.org>
Thu, 11 Dec 2014 15:07:38 +0000 (07:07 -0800)
patch from issue 759443006 at patchset 40001 (http://crrev.com/759443006#ps40001)

BUG=skia:

Review URL: https://codereview.chromium.org/767333002

include/core/SkCanvas.h
src/core/SkCanvas.cpp
src/core/SkPictureRecord.cpp
tests/PictureBBHTest.cpp
tests/RecordDrawTest.cpp
tests/RecordOptsTest.cpp
tests/RecordPatternTest.cpp
tests/RecordReplaceDrawTest.cpp
tests/RecordTestUtils.h

index c4ec76a..d28336f 100644 (file)
@@ -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);
index 083a8ed..294562a 100644 (file)
@@ -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);
 }
 
index 778abb5..67c4229 100644 (file)
@@ -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
 }
 
index bc712f5..562d9b5 100644 (file)
@@ -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"? <reed>
+    REPORTER_ASSERT(r, dstPic->approximateOpCount() == 4 || dstPic->approximateOpCount() == 2);
 }
 
 DEF_TEST(PictureBBH_Clear, r) {
index 2d9e90d..e830af4 100644 (file)
@@ -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<SkRecords::DrawPaint>(r, record, 0);
+    assert_type<SkRecords::Save>     (r, record, 1);
+    assert_type<SkRecords::SetMatrix>(r, record, 2);
+    assert_type<SkRecords::Restore>  (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<SkRecords::Save>    (r, rerecord, 0);
-    assert_type<SkRecords::DrawRect>(r, rerecord, 1);
-    assert_type<SkRecords::Restore> (r, rerecord, 2);
+    REPORTER_ASSERT(r, 1 == count_instances_of_type<SkRecords::DrawRect>(rerecord));
+    REPORTER_ASSERT(r, 0 == count_instances_of_type<SkRecords::ClipRect>(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<SkRecords::Save>    (r, rerecord, 0);
-    assert_type<SkRecords::Save>    (r, rerecord, 1);
-    assert_type<SkRecords::Restore> (r, rerecord, 2);
-    assert_type<SkRecords::Restore> (r, rerecord, 3);
+    int save_count = count_instances_of_type<SkRecords::Save>(rerecord);
+    int restore_count = count_instances_of_type<SkRecords::Save>(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<SkRecords::Save>     (r, rerecord, 0);
-    assert_type<SkRecords::DrawRect> (r, rerecord, 1);
-    assert_type<SkRecords::Restore>  (r, rerecord, 2);
-
-    const SkRecords::DrawRect* drawRect = assert_type<SkRecords::DrawRect>(r, rerecord, 1);
+    REPORTER_ASSERT(r, 1 == count_instances_of_type<SkRecords::DrawRect>(rerecord));
+    int index = find_first_instances_of_type<SkRecords::DrawRect>(rerecord);
+    const SkRecords::DrawRect* drawRect = assert_type<SkRecords::DrawRect>(r, rerecord, index);
     REPORTER_ASSERT(r, drawRect->rect == r2);
 }
 
index c5c4471..cdc0350 100644 (file)
 
 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<SkRecords::NoOp>(2);  // NoOps should be allowed.
+    record.replace<SkRecords::NoOp>(1);  // NoOps should be allowed.
 
     SkRecordNoopSaveRestores(&record);
 
-    assert_type<SkRecords::NoOp>(r, record, 0);
-    assert_type<SkRecords::DrawRect>(r, record, 1);
-    assert_type<SkRecords::NoOp>(r, record, 2);
-    assert_type<SkRecords::DrawRect>(r, record, 3);
-    assert_type<SkRecords::NoOp>(r, record, 4);
+    REPORTER_ASSERT(r, 2 == count_instances_of_type<SkRecords::DrawRect>(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<SkRecords::NoOp>(r, record, index);
     }
 }
@@ -86,10 +79,22 @@ DEF_TEST(RecordOpts_SaveSaveLayerRestoreRestore, r) {
     recorder.restore();
 
     SkRecordNoopSaveRestores(&record);
-    assert_type<SkRecords::Save>     (r, record, 0);
-    assert_type<SkRecords::SaveLayer>(r, record, 1);
-    assert_type<SkRecords::Restore>  (r, record, 2);
-    assert_type<SkRecords::Restore>  (r, record, 3);
+    switch (record.count()) {
+        case 4:
+            assert_type<SkRecords::Save>     (r, record, 0);
+            assert_type<SkRecords::SaveLayer>(r, record, 1);
+            assert_type<SkRecords::Restore>  (r, record, 2);
+            assert_type<SkRecords::Restore>  (r, record, 3);
+            break;
+        case 2:
+            assert_type<SkRecords::SaveLayer>(r, record, 0);
+            assert_type<SkRecords::Restore>  (r, record, 1);
+            break;
+        case 0:
+            break;
+        default:
+            REPORTER_ASSERT(r, false);
+    }
 }
 
 static void assert_savelayer_restore(skiatest::Reporter* r,
index 5f4d006..1f5ce2c 100644 (file)
@@ -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<Is<Save>, IsDraw, Is<Restore> > 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<Save>()    != NULL);
-    REPORTER_ASSERT(r, pattern.second<SkPaint>()->getColor() == 0xEEAA8822);
-    REPORTER_ASSERT(r, pattern.third<Restore>() != NULL);
-
-    REPORTER_ASSERT(r, pattern.match(&record, 6));
-    REPORTER_ASSERT(r, pattern.first<Save>()    != NULL);
-    REPORTER_ASSERT(r, pattern.second<SkPaint>()->getColor() == 0xFACEFACE);
-    REPORTER_ASSERT(r, pattern.third<Restore>() != 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));
 }
index f1ebf82..e0e9466 100644 (file)
@@ -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<SkRecords::Save>(r, rerecord, 0);
-    assert_type<SkRecords::DrawRect>(r, rerecord, 1);
-    assert_type<SkRecords::Restore>(r, rerecord, 2);
+    switch (rerecord.count()) {
+        case 3:
+            assert_type<SkRecords::Save>(r, rerecord, 0);
+            assert_type<SkRecords::DrawRect>(r, rerecord, 1);
+            assert_type<SkRecords::Restore>(r, rerecord, 2);
+            break;
+        case 1:
+            assert_type<SkRecords::DrawRect>(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<SkRecords::Save>(r, rerecord, 0);
-    assert_type<SkRecords::Save>(r, rerecord, 1);
-    assert_type<SkRecords::Restore>(r, rerecord, 2);
-    assert_type<SkRecords::Restore>(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<SkRecords::Save>(r, rerecord, 0);
-    assert_type<SkRecords::Save>(r, rerecord, 1);
-    assert_type<SkRecords::SetMatrix>(r, rerecord, 2);
-    assert_type<SkRecords::DrawBitmapRectToRect>(r, rerecord, 3);
-    assert_type<SkRecords::Restore>(r, rerecord, 4);
-    assert_type<SkRecords::DrawRect>(r, rerecord, 5);
-    assert_type<SkRecords::Restore>(r, rerecord, 6);
+    int recount = rerecord.count();
+    REPORTER_ASSERT(r, 5 == recount || 7 == recount);
+
+    int index = 0;
+    if (7 == recount) {
+        assert_type<SkRecords::Save>(r, rerecord, 0);
+        index += 1;
+    }
+    assert_type<SkRecords::Save>(r, rerecord, index + 0);
+    assert_type<SkRecords::SetMatrix>(r, rerecord, index + 1);
+    assert_type<SkRecords::DrawBitmapRectToRect>(r, rerecord, index + 2);
+    assert_type<SkRecords::Restore>(r, rerecord, index + 3);
+    assert_type<SkRecords::DrawRect>(r, rerecord, index + 4);
+    if (7 == recount) {
+        assert_type<SkRecords::Restore>(r, rerecord, 6);
+    }
 }
 
 DEF_GPUTEST(RecordReplaceDraw, r, factory) { 
index 0575b83..4bab8e4 100644 (file)
@@ -28,4 +28,28 @@ static const T* assert_type(skiatest::Reporter* r, const SkRecord& record, unsig
     return reader.ptr;
 }
 
+template <typename DrawT> struct MatchType {
+    template <typename T> int operator()(const T&) { return 0; }
+    int operator()(const DrawT&) { return 1; }
+};
+
+template <typename DrawT> int count_instances_of_type(const SkRecord& record) {
+    MatchType<DrawT> matcher;
+    int counter = 0;
+    for (unsigned i = 0; i < record.count(); i++) {
+        counter += record.visit<int>(i, matcher);
+    }
+    return counter;
+}
+
+template <typename DrawT> int find_first_instances_of_type(const SkRecord& record) {
+    MatchType<DrawT> matcher;
+    for (unsigned i = 0; i < record.count(); i++) {
+        if (record.visit<int>(i, matcher)) {
+            return i;
+        }
+    }
+    return -1;
+}
+
 #endif//RecordTestUtils_DEFINED