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
diff --git a/dm/DM.cpp b/dm/DM.cpp
index 274ec2a..65fccc7 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 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 @@
}
}
-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 @@
}
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 0d9dce7..0000000
--- 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/DMChecksumTask.h b/dm/DMChecksumTask.h
deleted file mode 100644
index b0afac9..0000000
--- a/dm/DMChecksumTask.h
+++ /dev/null
@@ -1,30 +0,0 @@
-#ifndef DMChecksumTask_DEFINED
-#define DMChecksumTask_DEFINED
-
-#include "DMTask.h"
-#include "SkBitmap.h"
-#include "SkString.h"
-#include "gm_expectations.h"
-
-namespace DM {
-
-// ChecksumTask compares an SkBitmap against some Expectations.
-// Moving this off the GPU threadpool is a nice (~30%) runtime win.
-class ChecksumTask : public Task {
-public:
- ChecksumTask(const Task& parent, skiagm::Expectations, SkBitmap);
-
- virtual void draw() SK_OVERRIDE;
- virtual bool usesGpu() const SK_OVERRIDE { return false; }
- virtual bool shouldSkip() const SK_OVERRIDE { return false; }
- virtual SkString name() const SK_OVERRIDE { return fName; }
-
-private:
- const SkString fName;
- const skiagm::Expectations fExpectations;
- const SkBitmap fBitmap;
-};
-
-} // namespace DM
-
-#endif // DMChecksumTask_DEFINED
diff --git a/dm/DMCpuTask.cpp b/dm/DMCpuTask.cpp
index 3f51c8a..c538f0a 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 @@
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 @@
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 998ed7b..c1ee715 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 @@
CpuTask(const char* name,
Reporter*,
TaskRunner*,
- const skiagm::ExpectationsSource&,
+ const Expectations&,
skiagm::GMRegistry::Factory,
SkBitmap::Config);
@@ -35,7 +33,7 @@
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 0000000..238d1c5
--- /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 0000000..cb92486
--- /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/DMExpectationsTask.h b/dm/DMExpectationsTask.h
new file mode 100644
index 0000000..cf76fc8
--- /dev/null
+++ b/dm/DMExpectationsTask.h
@@ -0,0 +1,30 @@
+#ifndef DMExpectationsTask_DEFINED
+#define DMExpectationsTask_DEFINED
+
+#include "DMExpectations.h"
+#include "DMTask.h"
+#include "SkBitmap.h"
+#include "SkString.h"
+
+namespace DM {
+
+// ExpectationsTask compares an SkBitmap against some Expectations.
+// Moving this off the GPU threadpool is a nice (~30%) runtime win.
+class ExpectationsTask : public Task {
+public:
+ ExpectationsTask(const Task& parent, const Expectations&, SkBitmap);
+
+ virtual void draw() SK_OVERRIDE;
+ virtual bool usesGpu() const SK_OVERRIDE { return false; }
+ virtual bool shouldSkip() const SK_OVERRIDE { return false; }
+ virtual SkString name() const SK_OVERRIDE { return fName; }
+
+private:
+ const SkString fName;
+ const Expectations& fExpectations;
+ const SkBitmap fBitmap;
+};
+
+} // namespace DM
+
+#endif // DMExpectationsTask_DEFINED
diff --git a/dm/DMGpuTask.cpp b/dm/DMGpuTask.cpp
index d4c4254..c0502ee 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 @@
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 @@
: 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 @@
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 87c530b..aa350c9 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 @@
GpuTask(const char* name,
Reporter*,
TaskRunner*,
- const skiagm::ExpectationsSource&,
+ const Expectations&,
skiagm::GMRegistry::Factory,
SkBitmap::Config,
GrContextFactory::GLContextType,
@@ -34,7 +34,7 @@
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 6cf6c22..52efda3 100644
--- a/dm/DMUtil.cpp
+++ b/dm/DMUtil.cpp
@@ -10,11 +10,6 @@
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 5f22df0..b887d82 100644
--- a/dm/DMUtil.h
+++ b/dm/DMUtil.h
@@ -12,9 +12,6 @@
// 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 c86381f..11b9fd3 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 @@
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 @@
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 @@
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 82a26bc..49a5c74 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 @@
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;
diff --git a/gyp/dm.gyp b/gyp/dm.gyp
index 8fa59c5..e56cf16 100644
--- a/gyp/dm.gyp
+++ b/gyp/dm.gyp
@@ -20,8 +20,8 @@
'includes': [ 'gmslides.gypi' ],
'sources': [
'../dm/DM.cpp',
- '../dm/DMChecksumTask.cpp',
'../dm/DMCpuTask.cpp',
+ '../dm/DMExpectationsTask.cpp',
'../dm/DMGpuTask.cpp',
'../dm/DMPipeTask.cpp',
'../dm/DMReplayTask.cpp',