Support serialization in SkRecord-backed SkPictures.

Update DM to test SkRecord through SkPictureRecorder API.

BUG=skia:
R=robertphillips@google.com, mtklein@google.com

Author: mtklein@chromium.org

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

13 files changed, 133 insertions, 152 deletions

diff --git a/dm/DMCpuGMTask.cpp b/dm/DMCpuGMTask.cpp
index 8bec52250b..d45da5166c 100644
--- a/dm/DMCpuGMTask.cpp
+++ b/dm/DMCpuGMTask.cpp
@@ -2,7 +2,6 @@
 #include "DMExpectationsTask.h"
 #include "DMPipeTask.h"
 #include "DMQuiltTask.h"
-#include "DMRecordTask.h"
 #include "DMReplayTask.h"
 #include "DMSerializeTask.h"
 #include "DMUtil.h"
@@ -39,12 +38,15 @@ void CpuGMTask::draw() {
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kInProcess_Mode);
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kCrossProcess_Mode);
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kSharedAddress_Mode);
+
     SPAWN(QuiltTask, fGMFactory(NULL), bitmap);
-    SPAWN(RecordTask, fGMFactory(NULL), bitmap, RecordTask::kOptimize_Mode);
-    SPAWN(RecordTask, fGMFactory(NULL), bitmap, RecordTask::kNoOptimize_Mode);
+
     SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kNormal_Mode);
     SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kRTree_Mode);
-    SPAWN(SerializeTask, fGMFactory(NULL), bitmap);
+    SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kSkRecord_Mode);
+
+    SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kNormal_Mode);
+    SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kSkRecord_Mode);
 
     SPAWN(WriteTask, bitmap);
 #undef SPAWN
diff --git a/dm/DMRecordTask.cpp b/dm/DMRecordTask.cpp
deleted file mode 100644
index 28633f20da..0000000000
--- a/dm/DMRecordTask.cpp
+++ /dev/null
@@ -1,63 +0,0 @@
-#include "DMRecordTask.h"
-#include "DMUtil.h"
-#include "DMWriteTask.h"
-#include "SkCommandLineFlags.h"
-#include "SkRecord.h"
-#include "SkRecordDraw.h"
-#include "SkRecordOpts.h"
-#include "SkRecorder.h"
-
-DEFINE_bool(skr, true, "If true, run SKR tests.");
-
-namespace DM {
-
-RecordTask::RecordTask(const Task& parent, skiagm::GM* gm, SkBitmap reference, Mode mode)
-    : CpuTask(parent)
-    , fOptimize(mode == kOptimize_Mode)
-    , fName(UnderJoin(parent.name().c_str(), fOptimize ? "skr" : "skr-noopt"))
-    , fGM(gm)
-    , fReference(reference)
-    {}
-
-RecordTask::RecordTask(const Task& parent, SkPicture* pic, SkBitmap reference, Mode mode)
-    : CpuTask(parent)
-    , fOptimize(mode == kOptimize_Mode)
-    , fName(UnderJoin(parent.name().c_str(), fOptimize ? "skr" : "skr-noopt"))
-    , fPicture(SkRef(pic))
-    , fReference(reference)
-    {}
-
-void RecordTask::draw() {
-    // Record into an SkRecord.
-    SkRecord record;
-    SkRecorder recorder(&record, fReference.width(), fReference.height());
-
-    if (fGM.get()) {
-        recorder.concat(fGM->getInitialTransform());
-        fGM->draw(&recorder);
-    } else {
-        fPicture->draw(&recorder);
-    }
-
-
-    if (fOptimize) {
-        SkRecordOptimize(&record);
-    }
-
-    // Draw the SkRecord back into a bitmap.
-    SkBitmap bitmap;
-    AllocatePixels(fReference, &bitmap);
-    SkCanvas target(bitmap);
-    SkRecordDraw(record, &target);
-
-    if (!BitmapsEqual(bitmap, fReference)) {
-        this->fail();
-        this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
-    }
-}
-
-bool RecordTask::shouldSkip() const {
-    return !FLAGS_skr;
-}
-
-}  // namespace DM
diff --git a/dm/DMRecordTask.h b/dm/DMRecordTask.h
deleted file mode 100644
index 1420724bb0..0000000000
--- a/dm/DMRecordTask.h
+++ /dev/null
@@ -1,39 +0,0 @@
-#ifndef DMRecordTask_DEFINED
-#define DMRecordTask_DEFINED
-
-#include "DMTask.h"
-#include "SkBitmap.h"
-#include "SkPicture.h"
-#include "SkString.h"
-#include "SkTemplates.h"
-#include "gm.h"
-
-// Records a GM or SKP through an SkRecord, draws it, and compares against the reference bitmap.
-
-namespace DM {
-
-class RecordTask : public CpuTask {
-
-public:
-    enum Mode {
-        kNoOptimize_Mode,
-        kOptimize_Mode,
-    };
-    RecordTask(const Task& parent, skiagm::GM*, SkBitmap reference, Mode);
-    RecordTask(const Task& parent, SkPicture*,  SkBitmap reference, Mode);
-
-    virtual void draw() SK_OVERRIDE;
-    virtual bool shouldSkip() const SK_OVERRIDE;
-    virtual SkString name() const SK_OVERRIDE { return fName; }
-
-private:
-    bool fOptimize;
-    const SkString fName;
-    SkAutoTUnref<SkPicture> fPicture;
-    SkAutoTDelete<skiagm::GM> fGM;
-    const SkBitmap fReference;
-};
-
-}  // namespace DM
-
-#endif  // DMRecordTask_DEFINED
diff --git a/dm/DMReplayTask.cpp b/dm/DMReplayTask.cpp
index 86838795d2..7fd58cef6a 100644
--- a/dm/DMReplayTask.cpp
+++ b/dm/DMReplayTask.cpp
@@ -8,6 +8,10 @@
 
 DEFINE_bool(replay, true, "If true, run picture replay tests.");
 DEFINE_bool(rtree,  true, "If true, run picture replay tests with an rtree.");
+DEFINE_bool(skr,    true, "If true, run picture replay tests with SkRecord backend.");
+
+static const char* kSuffixes[] = { "replay", "rtree", "skr" };
+static const bool* kEnabled[]  = { &FLAGS_replay, &FLAGS_rtree, &FLAGS_skr };
 
 namespace DM {
 
@@ -16,22 +20,23 @@ ReplayTask::ReplayTask(const Task& parent,
                        SkBitmap reference,
                        Mode mode)
     : CpuTask(parent)
-    , fUseRTree(mode == kRTree_Mode)
-    , fName(UnderJoin(parent.name().c_str(), fUseRTree ? "rtree" : "replay"))
+    , fMode(mode)
+    , fName(UnderJoin(parent.name().c_str(), kSuffixes[mode]))
     , fGM(gm)
     , fReference(reference)
     {}
 
 void ReplayTask::draw() {
     SkAutoTDelete<SkBBHFactory> factory;
-    if (fUseRTree) {
+    if (kRTree_Mode == fMode) {
         factory.reset(SkNEW(SkRTreeFactory));
     }
-    SkAutoTUnref<SkPicture> recorded(RecordPicture(fGM.get(), 0, factory.get()));
+    SkAutoTUnref<SkPicture> recorded(
+            RecordPicture(fGM.get(), factory.get(), kSkRecord_Mode == fMode));
 
     SkBitmap bitmap;
     AllocatePixels(fReference, &bitmap);
-    DrawPicture(recorded, &bitmap);
+    DrawPicture(*recorded, &bitmap);
     if (!BitmapsEqual(bitmap, fReference)) {
         this->fail();
         this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
@@ -42,14 +47,7 @@ bool ReplayTask::shouldSkip() const {
     if (fGM->getFlags() & skiagm::GM::kSkipPicture_Flag) {
         return true;
     }
-
-    if (FLAGS_rtree && fUseRTree) {
-        return false;
-    }
-    if (FLAGS_replay && !fUseRTree) {
-        return false;
-    }
-    return true;
+    return !*kEnabled[fMode];
 }
 
 }  // namespace DM
diff --git a/dm/DMReplayTask.h b/dm/DMReplayTask.h
index de28172000..f2ed89d147 100644
--- a/dm/DMReplayTask.h
+++ b/dm/DMReplayTask.h
@@ -17,6 +17,7 @@ public:
     enum Mode {
         kNormal_Mode,
         kRTree_Mode,
+        kSkRecord_Mode,
     };
     ReplayTask(const Task& parent,  // ReplayTask must be a child task.  Pass its parent here.
                skiagm::GM*,         // GM to run through a picture.  Takes ownership.
@@ -28,7 +29,7 @@ public:
     virtual SkString name() const SK_OVERRIDE { return fName; }
 
 private:
-    const bool fUseRTree;
+    const Mode fMode;
     const SkString fName;
     SkAutoTDelete<skiagm::GM> fGM;
     const SkBitmap fReference;
diff --git a/dm/DMSKPTask.cpp b/dm/DMSKPTask.cpp
index 760138f7c9..68fbb60717 100644
--- a/dm/DMSKPTask.cpp
+++ b/dm/DMSKPTask.cpp
@@ -1,23 +1,59 @@
-#include "DMRecordTask.h"
 #include "DMSKPTask.h"
 #include "DMUtil.h"
 #include "DMWriteTask.h"
 
+#include "SkCommandLineFlags.h"
+#include "SkPictureRecorder.h"
+
+DECLARE_bool(skr);  // in DMReplayTask.cpp
+
 namespace DM {
 
-SKPTask::SKPTask(Reporter* r, TaskRunner* tr, SkPicture* pic, SkString filename)
+// Test that an SkPicture plays back the same when re-recorded into an
+// SkPicture backed by SkRecord.
+class SkrComparisonTask : public CpuTask {
+public:
+    SkrComparisonTask(const Task& parent, const SkPicture* picture, SkBitmap reference)
+        : CpuTask(parent)
+        , fPicture(picture)
+        , fReference(reference)
+        , fName(UnderJoin(parent.name().c_str(), "skr")) {}
+
+    virtual bool shouldSkip() const SK_OVERRIDE { return !FLAGS_skr; }
+    virtual SkString name() const SK_OVERRIDE { return fName; }
+
+    virtual void draw() SK_OVERRIDE {
+        SkPictureRecorder recorder;
+        fPicture->draw(recorder.EXPERIMENTAL_beginRecording(fPicture->width(), fPicture->height()));
+        SkAutoTDelete<const SkPicture> skrPicture(recorder.endRecording());
+
+        SkBitmap bitmap;
+        AllocatePixels(kN32_SkColorType, fPicture->width(), fPicture->height(), &bitmap);
+        DrawPicture(*skrPicture, &bitmap);
+
+        if (!BitmapsEqual(fReference, bitmap)) {
+            this->fail();
+            this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
+        }
+    }
+
+private:
+    SkAutoTUnref<const SkPicture> fPicture;
+    const SkBitmap fReference;
+    const SkString fName;
+};
+
+
+SKPTask::SKPTask(Reporter* r, TaskRunner* tr, const SkPicture* pic, SkString filename)
     : CpuTask(r, tr), fPicture(SkRef(pic)), fName(FileToTaskName(filename)) {}
 
 void SKPTask::draw() {
     SkBitmap bitmap;
     AllocatePixels(kN32_SkColorType, fPicture->width(), fPicture->height(), &bitmap);
-    DrawPicture(fPicture, &bitmap);
+    DrawPicture(*fPicture, &bitmap);
 
-    this->spawnChild(SkNEW_ARGS(RecordTask,
-                                (*this, fPicture, bitmap, RecordTask::kNoOptimize_Mode)));
-    this->spawnChild(SkNEW_ARGS(RecordTask,
-                                (*this, fPicture, bitmap, RecordTask::kOptimize_Mode)));
     this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
+    this->spawnChild(SkNEW_ARGS(SkrComparisonTask, (*this, fPicture.get(), bitmap)));
 }
 
 }  // namespace DM
diff --git a/dm/DMSKPTask.h b/dm/DMSKPTask.h
index ef465af06a..b8a288e037 100644
--- a/dm/DMSKPTask.h
+++ b/dm/DMSKPTask.h
@@ -14,14 +14,14 @@ namespace DM {
 
 class SKPTask : public CpuTask {
 public:
-    SKPTask(Reporter*, TaskRunner*, SkPicture*, SkString name);
+    SKPTask(Reporter*, TaskRunner*, const SkPicture*, SkString name);
 
     virtual void draw() SK_OVERRIDE;
     virtual bool shouldSkip() const SK_OVERRIDE { return false; }
     virtual SkString name() const SK_OVERRIDE { return fName; }
 
 private:
-    SkAutoTUnref<SkPicture> fPicture;
+    SkAutoTUnref<const SkPicture> fPicture;
     const SkString fName;
 };
 
diff --git a/dm/DMSerializeTask.cpp b/dm/DMSerializeTask.cpp
index 62c45b3f04..86fbff33db 100644
--- a/dm/DMSerializeTask.cpp
+++ b/dm/DMSerializeTask.cpp
@@ -7,20 +7,27 @@
 #include "SkPixelRef.h"
 
 DEFINE_bool(serialize, true, "If true, run picture serialization tests.");
+DECLARE_bool(skr);  // in DMReplayTask.cpp
+
+static const char* kSuffixes[] = { "serialize", "serialize_skr" };
+static const bool* kEnabled[]  = { &FLAGS_serialize, &FLAGS_skr };
 
 namespace DM {
 
 SerializeTask::SerializeTask(const Task& parent,
                              skiagm::GM* gm,
-                             SkBitmap reference)
+                             SkBitmap reference,
+                             SerializeTask::Mode mode)
     : CpuTask(parent)
-    , fName(UnderJoin(parent.name().c_str(), "serialize"))
+    , fMode(mode)
+    , fName(UnderJoin(parent.name().c_str(), kSuffixes[mode]))
     , fGM(gm)
     , fReference(reference)
     {}
 
 void SerializeTask::draw() {
-    SkAutoTUnref<SkPicture> recorded(RecordPicture(fGM.get()));
+    SkAutoTUnref<SkPicture> recorded(
+        RecordPicture(fGM.get(), NULL/*no BBH*/, kSkRecord_Mode == fMode));
 
     SkDynamicMemoryWStream wStream;
     recorded->serialize(&wStream, NULL);
@@ -29,7 +36,7 @@ void SerializeTask::draw() {
 
     SkBitmap bitmap;
     AllocatePixels(fReference, &bitmap);
-    DrawPicture(reconstructed, &bitmap);
+    DrawPicture(*reconstructed, &bitmap);
     if (!BitmapsEqual(bitmap, fReference)) {
         this->fail();
         this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
@@ -37,7 +44,10 @@ void SerializeTask::draw() {
 }
 
 bool SerializeTask::shouldSkip() const {
-    return !FLAGS_serialize || fGM->getFlags() & skiagm::GM::kSkipPicture_Flag;
+    if (fGM->getFlags() & skiagm::GM::kSkipPicture_Flag) {
+        return true;
+    }
+    return !*kEnabled[fMode];
 }
 
 }  // namespace DM
diff --git a/dm/DMSerializeTask.h b/dm/DMSerializeTask.h
index 1f8b83632b..d06ebeb5f7 100644
--- a/dm/DMSerializeTask.h
+++ b/dm/DMSerializeTask.h
@@ -14,15 +14,21 @@ namespace DM {
 class SerializeTask : public CpuTask {
 
 public:
+    enum Mode {
+        kNormal_Mode,
+        kSkRecord_Mode,
+    };
     SerializeTask(const Task& parent,
                   skiagm::GM*,
-                  SkBitmap reference);
+                  SkBitmap reference,
+                  Mode mode);
 
     virtual void draw() SK_OVERRIDE;
     virtual bool shouldSkip() const SK_OVERRIDE;
     virtual SkString name() const SK_OVERRIDE { return fName; }
 
 private:
+    const Mode fMode;
     const SkString fName;
     SkAutoTDelete<skiagm::GM> fGM;
     const SkBitmap fReference;
diff --git a/dm/DMUtil.cpp b/dm/DMUtil.cpp
index 94fdd6ec88..92ce38172e 100644
--- a/dm/DMUtil.cpp
+++ b/dm/DMUtil.cpp
@@ -20,10 +20,13 @@ SkString FileToTaskName(SkString filename) {
     return filename;
 }
 
-SkPicture* RecordPicture(skiagm::GM* gm, uint32_t recordFlags, SkBBHFactory* factory) {
-    const SkISize size = gm->getISize();
+SkPicture* RecordPicture(skiagm::GM* gm, SkBBHFactory* factory, bool skr) {
+    const int w = gm->getISize().width(),
+              h = gm->getISize().height();
     SkPictureRecorder recorder;
-    SkCanvas* canvas = recorder.beginRecording(size.width(), size.height(), factory, recordFlags);
+
+    SkCanvas* canvas = skr ? recorder.EXPERIMENTAL_beginRecording(w, h, factory)
+                           : recorder.             beginRecording(w, h, factory);
     canvas->concat(gm->getInitialTransform());
     gm->draw(canvas);
     canvas->flush();
@@ -39,11 +42,10 @@ void AllocatePixels(const SkBitmap& reference, SkBitmap* bitmap) {
     AllocatePixels(reference.colorType(), reference.width(), reference.height(), bitmap);
 }
 
-void DrawPicture(SkPicture* picture, SkBitmap* bitmap) {
-    SkASSERT(picture != NULL);
+void DrawPicture(const SkPicture& picture, SkBitmap* bitmap) {
     SkASSERT(bitmap != NULL);
     SkCanvas canvas(*bitmap);
-    canvas.drawPicture(picture);
+    canvas.drawPicture(&picture);
     canvas.flush();
 }
 
diff --git a/dm/DMUtil.h b/dm/DMUtil.h
index b99fe0c01a..c202b048b0 100644
--- a/dm/DMUtil.h
+++ b/dm/DMUtil.h
@@ -18,10 +18,10 @@ SkString UnderJoin(const char* a, const char* b);
 // "foo_bar.skp" -> "foo-bar_skp"
 SkString FileToTaskName(SkString);
 
-// Draw gm to picture.  Passes recordFlags to SkPictureRecorder::beginRecording().
+// Draw gm to picture.  If skr is true, uses EXPERIMENTAL_beginRecording().
 SkPicture* RecordPicture(skiagm::GM* gm,
-                         uint32_t recordFlags = 0,
-                         SkBBHFactory* factory = NULL);
+                         SkBBHFactory* factory = NULL,
+                         bool skr = false);
 
 // Allocate an empty bitmap with this size and depth.
 void AllocatePixels(SkColorType, int w, int h, SkBitmap* bitmap);
@@ -29,7 +29,7 @@ void AllocatePixels(SkColorType, int w, int h, SkBitmap* bitmap);
 void AllocatePixels(const SkBitmap& reference, SkBitmap* bitmap);
 
 // Draw picture to bitmap.
-void DrawPicture(SkPicture* picture, SkBitmap* bitmap);
+void DrawPicture(const SkPicture& picture, SkBitmap* bitmap);
 
 // What is the maximum component difference in these bitmaps?
 unsigned MaxComponentDifference(const SkBitmap& a, const SkBitmap& b);
diff --git a/gyp/dm.gyp b/gyp/dm.gyp
index 202757ebf6..d36f18bbf8 100644
--- a/gyp/dm.gyp
+++ b/gyp/dm.gyp
@@ -36,7 +36,6 @@
             '../dm/DMPDFTask.cpp',
             '../dm/DMPipeTask.cpp',
             '../dm/DMQuiltTask.cpp',
-            '../dm/DMRecordTask.cpp',
             '../dm/DMReplayTask.cpp',
             '../dm/DMReporter.cpp',
             '../dm/DMSKPTask.cpp',
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index a678047455..75160b8be4 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -10,6 +10,7 @@
 #include "SkPictureFlat.h"
 #include "SkPicturePlayback.h"
 #include "SkPictureRecord.h"
+#include "SkPictureRecorder.h"
 
 #include "SkBBHFactory.h"
 #include "SkBitmapDevice.h"
@@ -156,6 +157,14 @@ static SkRecord* copy(const SkRecord& src, int width, int height) {
     return dst;
 }
 
+// Create an SkPicturePlayback-backed SkPicture from an SkRecord.
+// This for compatibility with serialization code only.  This is not cheap.
+static SkPicture* backport(const SkRecord& src, int width, int height) {
+    SkPictureRecorder recorder;
+    SkRecordDraw(src, recorder.beginRecording(width, height));
+    return recorder.endRecording();
+}
+
 // fRecord OK
 SkPicture::SkPicture(const SkPicture& src) : INHERITED() {
     this->needsNewGenID();
@@ -436,15 +445,25 @@ void SkPicture::createHeader(SkPictInfo* info) const {
     }
 }
 
-// fRecord TODO
+// fRecord OK
 void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
+    const SkPicturePlayback* playback = fPlayback.get();
+
+    // If we're a new-format picture, backport to old format for serialization.
+    SkAutoTDelete<SkPicture> oldFormat;
+    if (NULL == playback && NULL != fRecord.get()) {
+        oldFormat.reset(backport(*fRecord, fWidth, fHeight));
+        playback = oldFormat->fPlayback.get();
+        SkASSERT(NULL != playback);
+    }
+
     SkPictInfo info;
     this->createHeader(&info);
     stream->write(&info, sizeof(info));
 
-    if (NULL != fPlayback.get()) {
+    if (NULL != playback) {
         stream->writeBool(true);
-        fPlayback->serialize(stream, encoder);
+        playback->serialize(stream, encoder);
     } else {
         stream->writeBool(false);
     }
@@ -462,15 +481,25 @@ void SkPicture::WriteTagSize(SkWStream* stream, uint32_t tag,  size_t size) {
     stream->write32(SkToU32(size));
 }
 
-// fRecord TODO
+// fRecord OK
 void SkPicture::flatten(SkWriteBuffer& buffer) const {
+    const SkPicturePlayback* playback = fPlayback.get();
+
+    // If we're a new-format picture, backport to old format for serialization.
+    SkAutoTDelete<SkPicture> oldFormat;
+    if (NULL == playback && NULL != fRecord.get()) {
+        oldFormat.reset(backport(*fRecord, fWidth, fHeight));
+        playback = oldFormat->fPlayback.get();
+        SkASSERT(NULL != playback);
+    }
+
     SkPictInfo info;
     this->createHeader(&info);
     buffer.writeByteArray(&info, sizeof(info));
 
-    if (NULL != fPlayback.get()) {
+    if (NULL != playback) {
         buffer.writeBool(true);
-        fPlayback->flatten(buffer);
+        playback->flatten(buffer);
     } else {
         buffer.writeBool(false);
     }