Add support for reading a directory of images with --expectations (-r).

DM writes out its images in a hierarchy that's a little different than GM,
so this can't read GM's output.  But it can read its own, written with -w.

Example usage:
  $ out/Release/dm -w /tmp/baseline
  $ out/Release/dm -r /tmp/baseline -w /tmp/new
  (and optionally)
  $ mkdir /tmp/diff; out/Release/skdiff /tmp/baseline /tmp/new /tmp/diff

GM's IndividualImageExpectationsSource and Expectations are a little too eager
about decoding and hashing the expected images, so I took the opportunity to
add DM::Expectations that mostly replaces skiagm::ExpectationsSource and
skiagm::Expectations in DM.  It mainly exists to move the image decoding and
comparison off the main thread, which would otherwise be a major speed
bottleneck.

I tried to use skiagm code where possible.  One notable place where I differed
is in this new feature.  When -r is a directory of images, DM does no hashing.
It considerably faster to read the expected file into an SkBitmap and do a
byte-for-byte comparison than to hash the two bitmaps and check those.

The example usage above isn't quite working 100% yet.  Expectations on some GMs
fail, even with no binary change.  I haven't pinned down whether this is due to
  - a bug in DM
  - flaky GMs
  - unthreadsafe GMs
  - flaky image decoding
  - unthreadsafe image decoding
  - something else
but I intend to.  Leon, Derek and I have suspected PNG decoding isn't
threadsafe, but are as yet unable to prove it.

I also seem to be able to cause malloc to fail on my laptop if I run too many
configs at once, though I never seem to be using more than ~1G of RAM.  Will
track that down too.

BUG=
R=reed@google.com, bsalomon@google.com

Author: mtklein@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@12596 2bbb7eff-a529-9590-31e7-b0007b416f81

13 files changed, 172 insertions, 81 deletions

diff --git a/dm/DM.cpp b/dm/DM.cpp
index 274ec2a815..65fccc7611 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -14,18 +14,18 @@
 #include "DMTaskRunner.h"
 #include "DMCpuTask.h"
 #include "DMGpuTask.h"
+#include "DMWriteTask.h"
 
 #include <string.h>
 
 using skiagm::GM;
 using skiagm::GMRegistry;
-using skiagm::Expectations;
-using skiagm::ExpectationsSource;
-using skiagm::JsonExpectationsSource;
 
 DEFINE_int32(cpuThreads, -1, "Threads for CPU work. Default NUM_CPUS.");
 DEFINE_int32(gpuThreads, 1, "Threads for GPU work.");
-DEFINE_string(expectations, "", "Compare generated images against JSON expectations at this path.");
+DEFINE_string2(expectations, r, "",
+               "If a directory, compare generated images against images under this path. "
+               "If a file, compare generated images against JSON expectations at this path.");
 DEFINE_string(resources, "resources", "Path to resources directory.");
 DEFINE_string(match, "",  "[~][^]substring[$] [...] of GM name to run.\n"
                           "Multiple matches may be separated by spaces.\n"
@@ -50,7 +50,7 @@ static SkString lowercase(SkString s) {
 
 static void kick_off_tasks(const SkTDArray<GMRegistry::Factory>& gms,
                            const SkTArray<SkString>& configs,
-                           const ExpectationsSource& expectations,
+                           const DM::Expectations& expectations,
                            DM::Reporter* reporter,
                            DM::TaskRunner* tasks) {
     const SkBitmap::Config _565 = SkBitmap::kRGB_565_Config;
@@ -107,13 +107,6 @@ static void report_failures(const DM::Reporter& reporter) {
     }
 }
 
-class NoExpectations : public ExpectationsSource {
-public:
-    Expectations get(const char* /*testName*/) const SK_OVERRIDE {
-        return Expectations();
-    }
-};
-
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
     SkGraphics::Init();
@@ -134,9 +127,14 @@ int tool_main(int argc, char** argv) {
     }
     SkDebugf("%d GMs x %d configs\n", gms.count(), configs.count());
 
-    SkAutoTUnref<ExpectationsSource> expectations(SkNEW(NoExpectations));
+    SkAutoTDelete<DM::Expectations> expectations(SkNEW(DM::NoExpectations));
     if (FLAGS_expectations.count() > 0) {
-        expectations.reset(SkNEW_ARGS(JsonExpectationsSource, (FLAGS_expectations[0])));
+        const char* path = FLAGS_expectations[0];
+        if (sk_isdir(path)) {
+            expectations.reset(SkNEW_ARGS(DM::WriteTask::Expectations, (path)));
+        } else {
+            expectations.reset(SkNEW_ARGS(DM::JsonExpectations, (path)));
+        }
     }
 
     DM::Reporter reporter;
diff --git a/dm/DMChecksumTask.cpp b/dm/DMChecksumTask.cpp
deleted file mode 100644
index 0d9dce7fd1..0000000000
--- a/dm/DMChecksumTask.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-#include "DMChecksumTask.h"
-#include "DMUtil.h"
-
-namespace DM {
-
-ChecksumTask::ChecksumTask(const Task& parent,
-                           skiagm::Expectations expectations,
-                           SkBitmap bitmap)
-    : Task(parent)
-    , fName(parent.name())  // Masquerade as parent so failures are attributed to it.
-    , fExpectations(expectations)
-    , fBitmap(bitmap)
-    {}
-
-void ChecksumTask::draw() {
-    if (fExpectations.ignoreFailure() || fExpectations.empty()) {
-        return;
-    }
-
-    const skiagm::GmResultDigest digest(fBitmap);
-    if (!fExpectations.match(digest)) {
-        this->fail();
-    }
-}
-
-}  // namespace DM
diff --git a/dm/DMCpuTask.cpp b/dm/DMCpuTask.cpp
index 3f51c8a318..c538f0a6e4 100644
--- a/dm/DMCpuTask.cpp
+++ b/dm/DMCpuTask.cpp
@@ -1,5 +1,5 @@
 #include "DMCpuTask.h"
-#include "DMChecksumTask.h"
+#include "DMExpectationsTask.h"
 #include "DMPipeTask.h"
 #include "DMReplayTask.h"
 #include "DMSerializeTask.h"
@@ -12,14 +12,14 @@ namespace DM {
 CpuTask::CpuTask(const char* name,
                  Reporter* reporter,
                  TaskRunner* taskRunner,
-                 const skiagm::ExpectationsSource& expectations,
+                 const Expectations& expectations,
                  skiagm::GMRegistry::Factory gmFactory,
                  SkBitmap::Config config)
     : Task(reporter, taskRunner)
     , fGMFactory(gmFactory)
     , fGM(fGMFactory(NULL))
     , fName(UnderJoin(fGM->shortName(), name))
-    , fExpectations(expectations.get(Png(fName).c_str()))
+    , fExpectations(expectations)
     , fConfig(config)
     {}
 
@@ -33,7 +33,7 @@ void CpuTask::draw() {
     canvas.flush();
 
 #define SPAWN(ChildTask, ...) this->spawnChild(SkNEW_ARGS(ChildTask, (*this, __VA_ARGS__)))
-    SPAWN(ChecksumTask, fExpectations, bitmap);
+    SPAWN(ExpectationsTask, fExpectations, bitmap);
 
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, false, false);
     SPAWN(PipeTask, fGMFactory(NULL), bitmap, true, false);
diff --git a/dm/DMCpuTask.h b/dm/DMCpuTask.h
index 998ed7ba4d..c1ee7152c9 100644
--- a/dm/DMCpuTask.h
+++ b/dm/DMCpuTask.h
@@ -1,6 +1,7 @@
 #ifndef DMCpuTask_DEFINED
 #define DMCpuTask_DEFINED
 
+#include "DMExpectations.h"
 #include "DMReporter.h"
 #include "DMTask.h"
 #include "DMTaskRunner.h"
@@ -8,12 +9,9 @@
 #include "SkString.h"
 #include "SkTemplates.h"
 #include "gm.h"
-#include "gm_expectations.h"
 
 // This is the main entry point for drawing GMs with the CPU.  Commandline
 // flags control whether this kicks off various comparison tasks when done.
-// Currently:
-//   --replay: spawn a DMReplayTask to record into a picture, draw the picture, and compare.
 
 namespace DM {
 
@@ -22,7 +20,7 @@ public:
     CpuTask(const char* name,
             Reporter*,
             TaskRunner*,
-            const skiagm::ExpectationsSource&,
+            const Expectations&,
             skiagm::GMRegistry::Factory,
             SkBitmap::Config);
 
@@ -35,7 +33,7 @@ private:
     skiagm::GMRegistry::Factory fGMFactory;
     SkAutoTDelete<skiagm::GM> fGM;
     const SkString fName;
-    const skiagm::Expectations fExpectations;
+    const Expectations& fExpectations;
     const SkBitmap::Config fConfig;
 };
 
diff --git a/dm/DMExpectations.h b/dm/DMExpectations.h
new file mode 100644
index 0000000000..238d1c5bea
--- /dev/null
+++ b/dm/DMExpectations.h
@@ -0,0 +1,46 @@
+#ifndef DMExpectations_DEFINED
+#define DMExpectations_DEFINED
+
+#include "DMTask.h"
+#include "gm_expectations.h"
+
+namespace DM {
+
+struct Expectations {
+    virtual ~Expectations() {}
+
+    // Return true if bitmap is the correct output for task, else false.
+    virtual bool check(const Task& task, SkBitmap bitmap) const = 0;
+};
+
+class NoExpectations : public Expectations {
+public:
+    NoExpectations() {}
+    bool check(const Task&, SkBitmap) const SK_OVERRIDE { return true; }
+};
+
+class JsonExpectations : public Expectations {
+public:
+    explicit JsonExpectations(const char* path) : fGMExpectations(path) {}
+
+    bool check(const Task& task, SkBitmap bitmap) const SK_OVERRIDE {
+        SkString filename = task.name();
+        filename.append(".png");
+        const skiagm::Expectations expectations = fGMExpectations.get(filename.c_str());
+
+        if (expectations.ignoreFailure() || expectations.empty()) {
+            return true;
+        }
+
+        // Delay this calculation as long as possible.  It's expensive.
+        const skiagm::GmResultDigest digest(bitmap);
+        return expectations.match(digest);
+    }
+
+private:
+    skiagm::JsonExpectationsSource fGMExpectations;
+};
+
+}  // namespace DM
+
+#endif // DMExpectations_DEFINED
diff --git a/dm/DMExpectationsTask.cpp b/dm/DMExpectationsTask.cpp
new file mode 100644
index 0000000000..cb92486269
--- /dev/null
+++ b/dm/DMExpectationsTask.cpp
@@ -0,0 +1,21 @@
+#include "DMExpectationsTask.h"
+#include "DMUtil.h"
+
+namespace DM {
+
+ExpectationsTask::ExpectationsTask(const Task& parent,
+                                   const Expectations& expectations,
+                                   SkBitmap bitmap)
+    : Task(parent)
+    , fName(parent.name())  // Masquerade as parent so failures are attributed to it.
+    , fExpectations(expectations)
+    , fBitmap(bitmap)
+    {}
+
+void ExpectationsTask::draw() {
+    if (!fExpectations.check(*this, fBitmap)) {
+        this->fail();
+    }
+}
+
+}  // namespace DM
diff --git a/dm/DMChecksumTask.h b/dm/DMExpectationsTask.h
index b0afac9467..cf76fc8bdf 100644
--- a/dm/DMChecksumTask.h
+++ b/dm/DMExpectationsTask.h
@@ -1,18 +1,18 @@
-#ifndef DMChecksumTask_DEFINED
-#define DMChecksumTask_DEFINED
+#ifndef DMExpectationsTask_DEFINED
+#define DMExpectationsTask_DEFINED
 
+#include "DMExpectations.h"
 #include "DMTask.h"
 #include "SkBitmap.h"
 #include "SkString.h"
-#include "gm_expectations.h"
 
 namespace DM {
 
-// ChecksumTask compares an SkBitmap against some Expectations.
+// ExpectationsTask compares an SkBitmap against some Expectations.
 // Moving this off the GPU threadpool is a nice (~30%) runtime win.
-class ChecksumTask : public Task {
+class ExpectationsTask : public Task {
 public:
-    ChecksumTask(const Task& parent, skiagm::Expectations, SkBitmap);
+    ExpectationsTask(const Task& parent, const Expectations&, SkBitmap);
 
     virtual void draw() SK_OVERRIDE;
     virtual bool usesGpu() const SK_OVERRIDE { return false; }
@@ -21,10 +21,10 @@ public:
 
 private:
     const SkString fName;
-    const skiagm::Expectations fExpectations;
+    const Expectations& fExpectations;
     const SkBitmap fBitmap;
 };
 
 }  // namespace DM
 
-#endif  // DMChecksumTask_DEFINED
+#endif  // DMExpectationsTask_DEFINED
diff --git a/dm/DMGpuTask.cpp b/dm/DMGpuTask.cpp
index d4c4254c62..c0502eeed5 100644
--- a/dm/DMGpuTask.cpp
+++ b/dm/DMGpuTask.cpp
@@ -1,6 +1,6 @@
 #include "DMGpuTask.h"
 
-#include "DMChecksumTask.h"
+#include "DMExpectationsTask.h"
 #include "DMUtil.h"
 #include "DMWriteTask.h"
 #include "SkCommandLineFlags.h"
@@ -12,7 +12,7 @@ namespace DM {
 GpuTask::GpuTask(const char* name,
                  Reporter* reporter,
                  TaskRunner* taskRunner,
-                 const skiagm::ExpectationsSource& expectations,
+                 const Expectations& expectations,
                  skiagm::GMRegistry::Factory gmFactory,
                  SkBitmap::Config config,
                  GrContextFactory::GLContextType contextType,
@@ -20,7 +20,7 @@ GpuTask::GpuTask(const char* name,
     : Task(reporter, taskRunner)
     , fGM(gmFactory(NULL))
     , fName(UnderJoin(fGM->shortName(), name))
-    , fExpectations(expectations.get(Png(fName).c_str()))
+    , fExpectations(expectations)
     , fConfig(config)
     , fContextType(contextType)
     , fSampleCount(sampleCount)
@@ -60,7 +60,7 @@ void GpuTask::draw() {
     gr->printCacheStats();
 #endif
 
-    this->spawnChild(SkNEW_ARGS(ChecksumTask, (*this, fExpectations, bitmap)));
+    this->spawnChild(SkNEW_ARGS(ExpectationsTask, (*this, fExpectations, bitmap)));
     this->spawnChild(SkNEW_ARGS(WriteTask, (*this, bitmap)));
 }
 
diff --git a/dm/DMGpuTask.h b/dm/DMGpuTask.h
index 87c530b825..aa350c9d99 100644
--- a/dm/DMGpuTask.h
+++ b/dm/DMGpuTask.h
@@ -1,6 +1,7 @@
 #ifndef DMGpuTask_DEFINED
 #define DMGpuTask_DEFINED
 
+#include "DMExpectations.h"
 #include "DMReporter.h"
 #include "DMTask.h"
 #include "DMTaskRunner.h"
@@ -9,7 +10,6 @@
 #include "SkString.h"
 #include "SkTemplates.h"
 #include "gm.h"
-#include "gm_expectations.h"
 
 // This is the main entry point for drawing GMs with the GPU.
 
@@ -20,7 +20,7 @@ public:
     GpuTask(const char* name,
             Reporter*,
             TaskRunner*,
-            const skiagm::ExpectationsSource&,
+            const Expectations&,
             skiagm::GMRegistry::Factory,
             SkBitmap::Config,
             GrContextFactory::GLContextType,
@@ -34,7 +34,7 @@ public:
 private:
     SkAutoTDelete<skiagm::GM> fGM;
     const SkString fName;
-    const skiagm::Expectations fExpectations;
+    const Expectations& fExpectations;
     const SkBitmap::Config fConfig;
     const GrContextFactory::GLContextType fContextType;
     const int fSampleCount;
diff --git a/dm/DMUtil.cpp b/dm/DMUtil.cpp
index 6cf6c22e2a..52efda3d32 100644
--- a/dm/DMUtil.cpp
+++ b/dm/DMUtil.cpp
@@ -10,11 +10,6 @@ SkString UnderJoin(const char* a, const char* b) {
     return s;
 }
 
-SkString Png(SkString s) {
-    s.appendf(".png");
-    return s;
-}
-
 void RecordPicture(skiagm::GM* gm, SkPicture* picture, uint32_t recordFlags) {
     const SkISize size = gm->getISize();
     SkCanvas* canvas = picture->beginRecording(size.width(), size.height(), recordFlags);
diff --git a/dm/DMUtil.h b/dm/DMUtil.h
index 5f22df0878..b887d828d4 100644
--- a/dm/DMUtil.h
+++ b/dm/DMUtil.h
@@ -12,9 +12,6 @@ namespace DM {
 // UnderJoin("a", "b") -> "a_b"
 SkString UnderJoin(const char* a, const char* b);
 
-// Png("a") -> "a.png"
-SkString Png(SkString s);
-
 // Draw gm to picture.  Passes recordFlags to SkPicture::beginRecording().
 void RecordPicture(skiagm::GM* gm, SkPicture* picture, uint32_t recordFlags = 0);
 
diff --git a/dm/DMWriteTask.cpp b/dm/DMWriteTask.cpp
index c86381f78a..11b9fd3470 100644
--- a/dm/DMWriteTask.cpp
+++ b/dm/DMWriteTask.cpp
@@ -2,6 +2,7 @@
 
 #include "DMUtil.h"
 #include "SkCommandLineFlags.h"
+#include "SkImageDecoder.h"
 #include "SkImageEncoder.h"
 #include "SkString.h"
 
@@ -9,18 +10,25 @@ DEFINE_string2(writePath, w, "", "If set, write GMs here as .pngs.");
 
 namespace DM {
 
-WriteTask::WriteTask(const Task& parent, SkBitmap bitmap) : Task(parent), fBitmap(bitmap) {
-    const int suffixes = parent.depth() + 1;
-    const char* name = parent.name().c_str();
+// Splits off the last N suffixes of name (splitting on _) and appends them to out.
+// Returns the total number of characters consumed.
+static int split_suffixes(int N, const char* name, SkTArray<SkString>* out) {
     SkTArray<SkString> split;
     SkStrSplit(name, "_", &split);
-    int totalSuffixLength = 0;
-    for (int i = 0; i < suffixes; i++) {
+    int consumed = 0;
+    for (int i = 0; i < N; i++) {
         // We're splitting off suffixes from the back to front.
-        fSuffixes.push_back(split[split.count()-i-1]);
-        totalSuffixLength += fSuffixes.back().size() + 1;
+        out->push_back(split[split.count()-i-1]);
+        consumed += out->back().size() + 1;  // Add one for the _.
     }
-    fGmName.set(name, strlen(name)-totalSuffixLength);
+    return consumed;
+}
+
+WriteTask::WriteTask(const Task& parent, SkBitmap bitmap) : Task(parent), fBitmap(bitmap) {
+    const int suffixes = parent.depth() + 1;
+    const SkString& name = parent.name();
+    const int totalSuffixLength = split_suffixes(suffixes, name.c_str(), &fSuffixes);
+    fGmName.set(name.c_str(), name.size()-totalSuffixLength);
 }
 
 void WriteTask::makeDirOrFail(SkString dir) {
@@ -36,7 +44,9 @@ void WriteTask::draw() {
         dir = SkOSPath::SkPathJoin(dir.c_str(), fSuffixes[i].c_str());
         this->makeDirOrFail(dir);
     }
-    if (!SkImageEncoder::EncodeFile(Png(SkOSPath::SkPathJoin(dir.c_str(), fGmName.c_str())).c_str(),
+    SkString path = SkOSPath::SkPathJoin(dir.c_str(), fGmName.c_str());
+    path.append(".png");
+    if (!SkImageEncoder::EncodeFile(path.c_str(),
                                     fBitmap,
                                     SkImageEncoder::kPNG_Type,
                                     100/*quality*/)) {
@@ -57,4 +67,44 @@ bool WriteTask::shouldSkip() const {
     return FLAGS_writePath.isEmpty();
 }
 
+static SkString path_to_expected_image(const char* root, const Task& task) {
+    SkString filename = task.name();
+
+    // We know that all names passed in here belong to top-level Tasks, which have a single suffix
+    // (8888, 565, gpu, etc.) indicating what subdirectory to look in.
+    SkTArray<SkString> suffixes;
+    const int suffixLength = split_suffixes(1, filename.c_str(), &suffixes);
+    SkASSERT(1 == suffixes.count());
+
+    // We'll look in root/suffix for images.
+    const SkString dir = SkOSPath::SkPathJoin(root, suffixes[0].c_str());
+
+    // Remove the suffix and tack on a .png.
+    filename.remove(filename.size() - suffixLength, suffixLength);
+    filename.append(".png");
+
+    //SkDebugf("dir %s, filename %s\n", dir.c_str(), filename.c_str());
+
+    return SkOSPath::SkPathJoin(dir.c_str(), filename.c_str());
+}
+
+bool WriteTask::Expectations::check(const Task& task, SkBitmap bitmap) const {
+    const SkString path = path_to_expected_image(fRoot, task);
+
+    SkBitmap expected;
+    if (SkImageDecoder::DecodeFile(path.c_str(), &expected)) {
+        if (expected.config() != bitmap.config()) {
+            SkBitmap converted;
+            SkAssertResult(expected.copyTo(&converted, bitmap.config()));
+            expected.swap(converted);
+        }
+        SkASSERT(expected.config() == bitmap.config());
+        return BitmapsEqual(expected, bitmap);
+    }
+
+    // Couldn't read the file, etc.
+    SkDebugf("Problem decoding %s to SkBitmap.\n", path.c_str());
+    return false;
+}
+
 }  // namespace DM
diff --git a/dm/DMWriteTask.h b/dm/DMWriteTask.h
index 82a26bc928..49a5c746a6 100644
--- a/dm/DMWriteTask.h
+++ b/dm/DMWriteTask.h
@@ -1,11 +1,13 @@
 #ifndef DMWriteTask_DEFINED
 #define DMWriteTask_DEFINED
 
+#include "DMExpectations.h"
 #include "DMTask.h"
 #include "SkBitmap.h"
 #include "SkString.h"
 #include "SkTArray.h"
 
+
 // Writes a bitmap to a file.
 
 namespace DM {
@@ -21,6 +23,16 @@ public:
     virtual bool shouldSkip() const SK_OVERRIDE;
     virtual SkString name() const SK_OVERRIDE;
 
+    // Reads image files WriteTask wrote under root and compares them with bitmap.
+    class Expectations : public DM::Expectations {
+    public:
+        explicit Expectations(const char* root) : fRoot(root) {}
+
+        bool check(const Task& task, SkBitmap bitmap) const SK_OVERRIDE;
+    private:
+        const char* fRoot;
+    };
+
 private:
     SkTArray<SkString> fSuffixes;
     SkString fGmName;