Tick off some TODOs:
- support fRecord in copy constructor
- support SkDrawPictureCallback
Moved SkDrawPictureCallback to its own header so
SkRecordDraw can include it without pulling in all of
SkPicture.
Adding an SkAutoSaveRestore to SkRecordDraw was the easiest
way to match the balance guarantees of the callback, and
probably not a bad idea in general. Updated its tests.
BUG=skia:
R=robertphillips@google.com
Review URL: https://codereview.chromium.org/349973008
diff --git a/include/core/SkDrawPictureCallback.h b/include/core/SkDrawPictureCallback.h
new file mode 100644
index 0000000..e86a227
--- /dev/null
+++ b/include/core/SkDrawPictureCallback.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkDrawPictureCallback_DEFINED
+#define SkDrawPictureCallback_DEFINED
+
+/**
+ * Subclasses of this can be passed to canvas.drawPicture(). During the drawing
+ * of the picture, this callback will periodically be invoked. If its
+ * abortDrawing() returns true, then picture playback will be interrupted.
+ *
+ * The resulting drawing is undefined, as there is no guarantee how often the
+ * callback will be invoked. If the abort happens inside some level of nested
+ * calls to save(), restore will automatically be called to return the state
+ * to the same level it was before the drawPicture call was made.
+ */
+class SK_API SkDrawPictureCallback {
+public:
+ SkDrawPictureCallback() {}
+ virtual ~SkDrawPictureCallback() {}
+
+ virtual bool abortDrawing() = 0;
+};
+
+#endif//SkDrawPictureCallback_DEFINED
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 04b2138b..a53617f 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -11,6 +11,7 @@
#define SkPicture_DEFINED
#include "SkBitmap.h"
+#include "SkDrawPictureCallback.h"
#include "SkImageDecoder.h"
#include "SkRefCnt.h"
@@ -21,7 +22,6 @@
class SkBBHFactory;
class SkBBoxHierarchy;
class SkCanvas;
-class SkDrawPictureCallback;
class SkData;
class SkPicturePlayback;
class SkPictureRecord;
@@ -314,22 +314,4 @@
SkAutoTDelete<SkRecord> fRecord;
};
-/**
- * Subclasses of this can be passed to canvas.drawPicture. During the drawing
- * of the picture, this callback will periodically be invoked. If its
- * abortDrawing() returns true, then picture playback will be interrupted.
- *
- * The resulting drawing is undefined, as there is no guarantee how often the
- * callback will be invoked. If the abort happens inside some level of nested
- * calls to save(), restore will automatically be called to return the state
- * to the same level it was before the drawPicture call was made.
- */
-class SK_API SkDrawPictureCallback {
-public:
- SkDrawPictureCallback() {}
- virtual ~SkDrawPictureCallback() {}
-
- virtual bool abortDrawing() = 0;
-};
-
#endif
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index c4e4204..2d054c6 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -15,6 +15,7 @@
#include "SkBitmapDevice.h"
#include "SkCanvas.h"
#include "SkChunkAlloc.h"
+#include "SkDrawPictureCallback.h"
#include "SkPaintPriv.h"
#include "SkPicture.h"
#include "SkRegion.h"
@@ -34,6 +35,7 @@
#include "SkRecord.h"
#include "SkRecordDraw.h"
+#include "SkRecorder.h"
template <typename T> int SafeCount(const T* obj) {
return obj ? obj->count() : 0;
@@ -146,29 +148,42 @@
fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (record, info, deepCopyOps)));
}
-// fRecord TODO
+// The simplest / safest way to copy an SkRecord is to replay it into a new one.
+static SkRecord* copy(const SkRecord& src, int width, int height) {
+ SkRecord* dst = SkNEW(SkRecord);
+ SkRecorder recorder(dst, width, height);
+ SkRecordDraw(src, &recorder);
+ return dst;
+}
+
+// fRecord OK
SkPicture::SkPicture(const SkPicture& src) : INHERITED() {
this->needsNewGenID();
fWidth = src.fWidth;
fHeight = src.fHeight;
- if (src.fPlayback.get()) {
+ if (NULL != src.fPlayback.get()) {
fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback)));
- fUniqueID = src.uniqueID(); // need to call method to ensure != 0
+ fUniqueID = src.uniqueID(); // need to call method to ensure != 0
+ }
+
+ if (NULL != src.fRecord.get()) {
+ fRecord.reset(copy(*src.fRecord, fWidth, fHeight));
+ fUniqueID = src.uniqueID(); // need to call method to ensure != 0
}
}
// fRecord OK
SkPicture::~SkPicture() {}
-// fRecord TODO
+// fRecord TODO, fix by deleting this method
SkPicture* SkPicture::clone() const {
SkPicture* clonedPicture = SkNEW(SkPicture);
this->clone(clonedPicture, 1);
return clonedPicture;
}
-// fRecord TODO
+// fRecord TODO, fix by deleting this method
void SkPicture::clone(SkPicture* pictures, int count) const {
SkPictCopyInfo copyInfo;
@@ -293,8 +308,7 @@
fPlayback->draw(*canvas, callback);
}
if (NULL != fRecord.get()) {
- // TODO: support SkDrawPictureCallback
- SkRecordDraw(*fRecord, canvas);
+ SkRecordDraw(*fRecord, canvas, callback);
}
}
@@ -495,7 +509,7 @@
}
#ifdef SK_BUILD_FOR_ANDROID
-// fRecord TODO
+// fRecord TODO, fix by switching Android to SkDrawPictureCallback, then deleting this method
void SkPicture::abortPlayback() {
if (NULL == fPlayback.get()) {
return;
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 1688710..c3f24b9 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -6,6 +6,7 @@
*/
#include <new>
#include "SkBBoxHierarchy.h"
+#include "SkDrawPictureCallback.h"
#include "SkPicturePlayback.h"
#include "SkPictureRecord.h"
#include "SkPictureStateTree.h"
@@ -1355,7 +1356,7 @@
&& 0 == sampleCount);
bool ret = suitableForDash &&
- (fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
+ (fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
< kNumAAConcavePaths;
if (!ret && NULL != reason) {
if (!suitableForDash) {
@@ -1364,7 +1365,7 @@
} else {
*reason = "Too many non dashed path effects.";
}
- } else if ((fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
+ } else if ((fContentInfo.numAAConcavePaths() - fContentInfo.numAAHairlineConcavePaths())
>= kNumAAConcavePaths)
*reason = "Too many anti-aliased concave paths.";
else
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp
index 2bf7076..e4c49e4 100644
--- a/src/core/SkRecordDraw.cpp
+++ b/src/core/SkRecordDraw.cpp
@@ -7,8 +7,12 @@
#include "SkRecordDraw.h"
-void SkRecordDraw(const SkRecord& record, SkCanvas* canvas) {
+void SkRecordDraw(const SkRecord& record, SkCanvas* canvas, SkDrawPictureCallback* callback) {
+ SkAutoCanvasRestore saveRestore(canvas, true /*save now, restore at exit*/);
for (SkRecords::Draw draw(canvas); draw.index() < record.count(); draw.next()) {
+ if (NULL != callback && callback->abortDrawing()) {
+ return;
+ }
record.visit<void>(draw.index(), draw);
}
}
diff --git a/src/core/SkRecordDraw.h b/src/core/SkRecordDraw.h
index 359679a..92b94c4 100644
--- a/src/core/SkRecordDraw.h
+++ b/src/core/SkRecordDraw.h
@@ -10,9 +10,10 @@
#include "SkRecord.h"
#include "SkCanvas.h"
+#include "SkDrawPictureCallback.h"
// Draw an SkRecord into an SkCanvas. A convenience wrapper around SkRecords::Draw.
-void SkRecordDraw(const SkRecord&, SkCanvas*);
+void SkRecordDraw(const SkRecord&, SkCanvas*, SkDrawPictureCallback* = NULL);
namespace SkRecords {
diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp
index b13b3a1..5c63b53 100644
--- a/tests/RecordDrawTest.cpp
+++ b/tests/RecordDrawTest.cpp
@@ -9,6 +9,7 @@
#include "RecordTestUtils.h"
#include "SkDebugCanvas.h"
+#include "SkDrawPictureCallback.h"
#include "SkRecord.h"
#include "SkRecordOpts.h"
#include "SkRecordDraw.h"
@@ -26,8 +27,51 @@
canvas->drawPosTextH(text, len, xpos, y, SkPaint());
}
-// Rerecord into another SkRecord using full SkCanvas semantics,
-// tracking clips and allowing SkRecordDraw's quickReject() calls to work.
+class JustOneDraw : public SkDrawPictureCallback {
+public:
+ JustOneDraw() : fCalls(0) {}
+
+ virtual bool abortDrawing() SK_OVERRIDE { return fCalls++ > 0; }
+private:
+ int fCalls;
+};
+
+DEF_TEST(RecordDraw_Abort, r) {
+ // Record two commands.
+ SkRecord record;
+ SkRecorder recorder(&record, W, H);
+ recorder.drawRect(SkRect::MakeWH(200, 300), SkPaint());
+ recorder.clipRect(SkRect::MakeWH(100, 200));
+
+ SkRecord rerecord;
+ SkRecorder canvas(&rerecord, W, H);
+
+ JustOneDraw callback;
+ SkRecordDraw(record, &canvas, &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);
+}
+
+DEF_TEST(RecordDraw_Unbalanced, r) {
+ SkRecord record;
+ SkRecorder recorder(&record, W, H);
+ recorder.save(); // We won't balance this, but SkRecordDraw will for us.
+
+ SkRecord rerecord;
+ SkRecorder canvas(&rerecord, W, H);
+ SkRecordDraw(record, &canvas);
+
+ 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);
+}
+
+// Rerecord into another SkRecord with a clip.
static void record_clipped(const SkRecord& record, SkRect clip, SkRecord* clipped) {
SkRecorder recorder(clipped, W, H);
recorder.clipRect(clip);
@@ -46,8 +90,11 @@
SkRecord clipped;
record_clipped(record, SkRect::MakeLTRB(20, 20, 200, 200), &clipped);
- // clipRect and the first drawPosTextH.
- REPORTER_ASSERT(r, 2 == clipped.count());
+ REPORTER_ASSERT(r, 4 == clipped.count());
+ assert_type<SkRecords::ClipRect> (r, clipped, 0);
+ assert_type<SkRecords::Save> (r, clipped, 1);
+ assert_type<SkRecords::DrawPosTextH>(r, clipped, 2);
+ assert_type<SkRecords::Restore> (r, clipped, 3);
}
DEF_TEST(RecordDraw_Culling, r) {
@@ -70,9 +117,15 @@
SkRecord clipped;
record_clipped(record, SkRect::MakeLTRB(20, 20, 200, 200), &clipped);
- // We'll keep the clipRect call from above, and the outer two drawRects, and the push/pop pair.
- // If culling weren't working, we'd see 8 commands recorded here.
- REPORTER_ASSERT(r, 5 == clipped.count());
+ // If culling weren't working, we'd see 3 more commands recorded here.
+ REPORTER_ASSERT(r, 7 == clipped.count());
+ assert_type<SkRecords::ClipRect>(r, clipped, 0);
+ assert_type<SkRecords::Save> (r, clipped, 1);
+ assert_type<SkRecords::PushCull>(r, clipped, 2);
+ assert_type<SkRecords::DrawRect>(r, clipped, 3);
+ assert_type<SkRecords::DrawRect>(r, clipped, 4);
+ assert_type<SkRecords::PopCull> (r, clipped, 5);
+ assert_type<SkRecords::Restore> (r, clipped, 6);
}
DEF_TEST(RecordDraw_SetMatrixClobber, r) {
@@ -91,6 +144,11 @@
translateCanvas.setMatrix(translate);
SkRecordDraw(scaleRecord, &translateCanvas);
+ REPORTER_ASSERT(r, 4 == translateRecord.count());
+ assert_type<SkRecords::SetMatrix>(r, translateRecord, 0);
+ assert_type<SkRecords::Save> (r, translateRecord, 1);
+ assert_type<SkRecords::SetMatrix>(r, translateRecord, 2);
+ assert_type<SkRecords::Restore> (r, translateRecord, 3);
// When we look at translateRecord now, it should have its first +20,+20 translate,
// then a 2x,3x scale that's been concatted with that +20,+20 translate.
@@ -98,7 +156,7 @@
setMatrix = assert_type<SkRecords::SetMatrix>(r, translateRecord, 0);
REPORTER_ASSERT(r, setMatrix->matrix == translate);
- setMatrix = assert_type<SkRecords::SetMatrix>(r, translateRecord, 1);
+ setMatrix = assert_type<SkRecords::SetMatrix>(r, translateRecord, 2);
SkMatrix expected = scale;
expected.postConcat(translate);
REPORTER_ASSERT(r, setMatrix->matrix == expected);
