Merge Replay and Quilt tasks, adding in all BBH implementations.

Replay isn't that helpful of a test any more now that we have the more
stringent Quilt tests.  Quilt was missing bounding box hierarchies, though,
while Replay was sort of testing RTree (pointlessly, as it was drawing without
any clip).  Now Quilt does everything, testing RTree, QuadTree, and TileGrid.

Quilt mode now falls back to drawing all at once (i.e. Replay) for GMs that
don't tile perfectly.  Still a TODO to make this check more flexible than exact
pixel matches.

Two GMs fail when using a BBH:
  - imageresizetiled
  - resizeimagefilter
We think we're not adjusting the bounds of save layers by their paint.
This is probably a bug, but one to be fixed separately from adding new tests.

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

Author: mtklein@chromium.org

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

diff --git a/dm/DMCpuGMTask.cpp b/dm/DMCpuGMTask.cpp
index 2252a1d..912ca9e 100644 (file)
--- a/dm/DMCpuGMTask.cpp
+++ b/dm/DMCpuGMTask.cpp
@@ -2,7 +2,6 @@
 #include "DMExpectationsTask.h"
 #include "DMPipeTask.h"
 #include "DMQuiltTask.h"
-#include "DMReplayTask.h"
 #include "DMSerializeTask.h"
 #include "DMUtil.h"
 #include "DMWriteTask.h"
@@ -39,13 +38,12 @@ void CpuGMTask::draw() {
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kCrossProcess_Mode);
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kSharedAddress_Mode);
 
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kNormal_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kNoBBH_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kRTree_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kQuadTree_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kTileGrid_Mode);
     SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kSkRecord_Mode);
 
-    SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kNormal_Mode);
-    SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kRTree_Mode);
-    SPAWN(ReplayTask, fGMFactory(NULL), bitmap, ReplayTask::kSkRecord_Mode);
-
     SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kNormal_Mode);
     SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kSkRecord_Mode);
 

diff --git a/dm/DMQuiltTask.cpp b/dm/DMQuiltTask.cpp
index b0f36c0..04f5999 100644 (file)
--- a/dm/DMQuiltTask.cpp
+++ b/dm/DMQuiltTask.cpp
@@ -2,15 +2,15 @@
 #include "DMUtil.h"
 #include "DMWriteTask.h"
 
+#include "SkBBHFactory.h"
 #include "SkCommandLineFlags.h"
 #include "SkPicture.h"
 #include "SkThreadPool.h"
 
-DEFINE_bool(quilt, true, "If true, draw into a quilt of small tiles and compare.");
+DEFINE_bool(quilt, true, "If true, draw GM via a picture into a quilt of small tiles and compare.");
 DEFINE_int32(quiltTile, 256, "Dimension of (square) quilt tile.");
-DEFINE_bool(quiltThreaded, true, "If true, draw quilt tiles with multiple threads.");
 
-static const char* kSuffixes[] = { "quilt", "quilt_skr" };
+static const char* kSuffixes[] = { "nobbh", "rtree", "quadtree", "tilegrid", "skr" };
 
 namespace DM {
 
@@ -53,27 +53,58 @@ private:
 };
 
 void QuiltTask::draw() {
-    SkAutoTUnref<SkPicture> recorded(
-            RecordPicture(fGM.get(), NULL/*bbh factory*/, kSkRecord_Mode == fMode));
+    SkAutoTDelete<SkBBHFactory> factory;
+    switch (fMode) {
+        case kRTree_Mode:
+            factory.reset(SkNEW(SkRTreeFactory));
+            break;
+        case kQuadTree_Mode:
+            factory.reset(SkNEW(SkQuadTreeFactory));
+            break;
+        case kTileGrid_Mode: {
+            const SkTileGridFactory::TileGridInfo tiles = {
+                { FLAGS_quiltTile, FLAGS_quiltTile },
+                /*overlap: */{0, 0},
+                /*offset:  */{0, 0},
+            };
+            factory.reset(SkNEW_ARGS(SkTileGridFactory, (tiles)));
+            break;
+        }
 
-    SkBitmap full;
-    AllocatePixels(fReference, &full);
+        case kNoBBH_Mode:
+        case kSkRecord_Mode:
+            break;
+    }
 
-    int threads = 0;
-    if (kSkRecord_Mode == fMode || FLAGS_quiltThreaded) {
-        threads = SkThreadPool::kThreadPerCore;
+    // A couple GMs draw wrong when using a bounding box hierarchy.
+    // This almost certainly means we have a bug to fix, but for now
+    // just draw without a bounding box hierarchy.
+    if (fGM->getFlags() & skiagm::GM::kNoBBH_Flag) {
+        factory.reset(NULL);
     }
-    SkThreadPool pool(threads);
 
-    for (int y = 0; y < tiles_needed(full.height(), FLAGS_quiltTile); y++) {
-        for (int x = 0; x < tiles_needed(full.width(), FLAGS_quiltTile); x++) {
-            // Deletes itself when done.
-            pool.add(new Tile(x, y, *recorded, &full));
+    SkAutoTUnref<const SkPicture> recorded(
+            RecordPicture(fGM.get(), factory.get(), kSkRecord_Mode == fMode));
+
+    SkBitmap full;
+    AllocatePixels(fReference, &full);
+
+    if (fGM->getFlags() & skiagm::GM::kSkipTiled_Flag) {
+        // Some GMs don't draw exactly the same when tiled.  Draw them in one go.
+        SkCanvas canvas(full);
+        recorded->draw(&canvas);
+        canvas.flush();
+    } else {
+        // Draw tiles in parallel into the same bitmap, simulating aggressive impl-side painting.
+        SkThreadPool pool(SkThreadPool::kThreadPerCore);
+        for (int y = 0; y < tiles_needed(full.height(), FLAGS_quiltTile); y++) {
+            for (int x = 0; x < tiles_needed(full.width(), FLAGS_quiltTile); x++) {
+                // Deletes itself when done.
+                pool.add(new Tile(x, y, *recorded, &full));
+            }
         }
     }
 
-    pool.wait();
-
     if (!BitmapsEqual(full, fReference)) {
         this->fail();
         this->spawnChild(SkNEW_ARGS(WriteTask, (*this, full)));
@@ -84,9 +115,6 @@ bool QuiltTask::shouldSkip() const {
     if (fGM->getFlags() & skiagm::GM::kSkipPicture_Flag) {
         return true;
     }
-    if (fGM->getFlags() & skiagm::GM::kSkipTiled_Flag) {
-        return true;
-    }
     return !FLAGS_quilt;
 }
 

diff --git a/dm/DMQuiltTask.h b/dm/DMQuiltTask.h
index 5f65662..1bc6a27 100644 (file)
--- a/dm/DMQuiltTask.h
+++ b/dm/DMQuiltTask.h
@@ -15,8 +15,11 @@ class QuiltTask : public CpuTask {
 
 public:
     enum Mode {
-        kNormal_Mode,
-        kSkRecord_Mode,
+        kNoBBH_Mode,
+        kRTree_Mode,
+        kQuadTree_Mode,
+        kTileGrid_Mode,
+        kSkRecord_Mode,  // Currently uses no BBH.
     };
 
     QuiltTask(const Task& parent,  // QuiltTask must be a child task.  Pass its parent here.

diff --git a/dm/DMReplayTask.cpp b/dm/DMReplayTask.cpp
deleted file mode 100644 (file)
index 7fd58ce..0000000
--- a/dm/DMReplayTask.cpp
+++ /dev/null
@@ -1,53 +0,0 @@
-#include "DMReplayTask.h"
-#include "DMWriteTask.h"
-#include "DMUtil.h"
-
-#include "SkBBHFactory.h"
-#include "SkCommandLineFlags.h"
-#include "SkPicture.h"
-
-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 {
-
-ReplayTask::ReplayTask(const Task& parent,
-                       skiagm::GM* gm,
-                       SkBitmap reference,
-                       Mode mode)
-    : CpuTask(parent)
-    , fMode(mode)
-    , fName(UnderJoin(parent.name().c_str(), kSuffixes[mode]))
-    , fGM(gm)
-    , fReference(reference)
-    {}
-
-void ReplayTask::draw() {
-    SkAutoTDelete<SkBBHFactory> factory;
-    if (kRTree_Mode == fMode) {
-        factory.reset(SkNEW(SkRTreeFactory));
-    }
-    SkAutoTUnref<SkPicture> recorded(
-            RecordPicture(fGM.get(), factory.get(), kSkRecord_Mode == fMode));
-
-    SkBitmap bitmap;
-    AllocatePixels(fReference, &bitmap);
-    DrawPicture(*recorded, &bitmap);
-    if (!BitmapsEqual(bitmap, fReference)) {
-        this->fail();
-        this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
-    }
-}
-
-bool ReplayTask::shouldSkip() const {
-    if (fGM->getFlags() & skiagm::GM::kSkipPicture_Flag) {
-        return true;
-    }
-    return !*kEnabled[fMode];
-}
-
-}  // namespace DM

diff --git a/dm/DMReplayTask.h b/dm/DMReplayTask.h
deleted file mode 100644 (file)
index f2ed89d..0000000
--- a/dm/DMReplayTask.h
+++ /dev/null
@@ -1,40 +0,0 @@
-#ifndef DMReplayTask_DEFINED
-#define DMReplayTask_DEFINED
-
-#include "DMTask.h"
-#include "SkBitmap.h"
-#include "SkString.h"
-#include "SkTemplates.h"
-#include "gm.h"
-
-// Records a GM through an SkPicture, draws it, and compares against the reference bitmap.
-
-namespace DM {
-
-class ReplayTask : public CpuTask {
-
-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.
-               SkBitmap reference,  // Bitmap to compare picture replay results to.
-               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;
-};
-
-}  // namespace DM
-
-#endif  // DMReplayTask_DEFINED

diff --git a/dm/DMSKPTask.cpp b/dm/DMSKPTask.cpp
index 68fbb60..295499b 100644 (file)
--- a/dm/DMSKPTask.cpp
+++ b/dm/DMSKPTask.cpp
@@ -5,7 +5,7 @@
 #include "SkCommandLineFlags.h"
 #include "SkPictureRecorder.h"
 
-DECLARE_bool(skr);  // in DMReplayTask.cpp
+DEFINE_bool(skr, true, "Test that SKPs draw the same when re-recorded with SkRecord backend.");
 
 namespace DM {
 

diff --git a/gm/gm.h b/gm/gm.h
index ac9da26..e48f772 100644 (file)
--- a/gm/gm.h
+++ b/gm/gm.h
@@ -45,6 +45,8 @@ namespace skiagm {
             kGPUOnly_Flag               = 1 << 9,
 
             kAsBench_Flag               = 1 << 10, // Run the GM as a benchmark in the bench tool
+
+            kNoBBH_Flag                 = 1 << 11, // May draw wrong using a bounding-box hierarchy
         };
 
         enum Mode {

diff --git a/gm/imageresizetiled.cpp b/gm/imageresizetiled.cpp
index ff02020..ec60274 100644 (file)
--- a/gm/imageresizetiled.cpp
+++ b/gm/imageresizetiled.cpp
@@ -22,6 +22,8 @@ public:
     }
 
 protected:
+    virtual uint32_t onGetFlags() const SK_OVERRIDE { return kNoBBH_Flag; }
+
     virtual SkString onShortName() SK_OVERRIDE {
         return SkString("imageresizetiled");
     }

diff --git a/gm/resizeimagefilter.cpp b/gm/resizeimagefilter.cpp
index e87dff0..96a6118 100644 (file)
--- a/gm/resizeimagefilter.cpp
+++ b/gm/resizeimagefilter.cpp
@@ -21,6 +21,8 @@ public:
     }
 
 protected:
+    virtual uint32_t onGetFlags() const SK_OVERRIDE { return kNoBBH_Flag; }
+
     virtual SkString onShortName() {
         return SkString("resizeimagefilter");
     }

diff --git a/gyp/dm.gyp b/gyp/dm.gyp
index d36f18b..a2ef037 100644 (file)
--- a/gyp/dm.gyp
+++ b/gyp/dm.gyp
@@ -36,7 +36,6 @@
             '../dm/DMPDFTask.cpp',
             '../dm/DMPipeTask.cpp',
             '../dm/DMQuiltTask.cpp',
-            '../dm/DMReplayTask.cpp',
             '../dm/DMReporter.cpp',
             '../dm/DMSKPTask.cpp',
             '../dm/DMSerializeTask.cpp',