Start to rework DM JSON handling.
DM's striking off into its own JSON world. This gets strawman implementations
in place for writing and reading a JSON file mapping test name to hashes.
For what it's worth, I basically want to change _all_ these pieces,
- MD5 is slow and we can replace it with something faster,
- JSON schema needs room to grow more data,
- it'd be nice to hash once instead of twice when reading and writing,
- this code wants lots of refactoring,
but this gives us a starting platform to work on these bits at our leisure.
E.x. file for now:
mtklein@mtklein ~/skia (dm)> cat good/dm.json
{
"3x3bitmaprect_565" : "fc70d985fbfbe70e3a3c9dc626d4f5bc",
"3x3bitmaprect_8888" : "df1591dde35907399734ea19feb76663",
"3x3bitmaprect_gpu" : "df1591dde35907399734ea19feb76663",
"aaclip_565" : "1862798689b838a7ab0dc0652b9ace3a",
"aaclip_8888" : "47bb314329f0ce243f1d83fd583decb7",
"aaclip_gpu" : "75f72412d0ef4815770202d297246e7d",
...
BUG=skia:
R=jcgregorio@google.com, stephana@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/546873002
diff --git a/dm/DM.cpp b/dm/DM.cpp
index b330445..2be3794 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -6,6 +6,7 @@
#include "SkCommonFlags.h"
#include "SkForceLinking.h"
#include "SkGraphics.h"
+#include "SkOSFile.h"
#include "SkPicture.h"
#include "SkString.h"
#include "SkTaskGroup.h"
@@ -208,19 +209,17 @@
GrGLStandard gpuAPI = get_gl_standard();
+ SkAutoTDelete<DM::Expectations> expectations(SkNEW(DM::Expectations));
+ if (FLAGS_expectations.count() > 0) {
+ expectations.reset(DM::WriteTask::Expectations::Create(FLAGS_expectations[0]));
+ if (!expectations.get()) {
+ return 1;
+ }
+ }
+
SkTDArray<GMRegistry::Factory> gms;
- SkAutoTDelete<DM::Expectations> expectations(SkNEW(DM::NoExpectations));
if (FLAGS_gms) {
append_matching_factories<GM>(GMRegistry::Head(), &gms);
-
- if (FLAGS_expectations.count() > 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)));
- }
- }
}
SkTDArray<TestRegistry::Factory> tests;
@@ -241,6 +240,8 @@
kick_off_skps(skps, &reporter, &tasks);
tasks.wait();
+ DM::WriteTask::DumpJson();
+
SkDebugf("\n");
#ifdef SK_DEBUG
if (FLAGS_portableFonts && FLAGS_reportUsedChars) {
diff --git a/dm/DMExpectations.h b/dm/DMExpectations.h
index 238d1c5..7993a55 100644
--- a/dm/DMExpectations.h
+++ b/dm/DMExpectations.h
@@ -2,7 +2,6 @@
#define DMExpectations_DEFINED
#include "DMTask.h"
-#include "gm_expectations.h"
namespace DM {
@@ -10,35 +9,9 @@
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);
+ virtual bool check(const Task& task, SkBitmap bitmap) const {
+ return true;
}
-
-private:
- skiagm::JsonExpectationsSource fGMExpectations;
};
} // namespace DM
diff --git a/dm/DMWriteTask.cpp b/dm/DMWriteTask.cpp
index 5a07e46..2a129a5 100644
--- a/dm/DMWriteTask.cpp
+++ b/dm/DMWriteTask.cpp
@@ -4,13 +4,12 @@
#include "SkColorPriv.h"
#include "SkCommonFlags.h"
#include "SkImageEncoder.h"
+#include "SkMD5.h"
#include "SkMallocPixelRef.h"
+#include "SkOSFile.h"
#include "SkStream.h"
#include "SkString.h"
-DEFINE_bool(writePngOnly, false, "If true, don't encode raw bitmap after .png data. "
- "This means -r won't work, but skdiff will still work fine.");
-
namespace DM {
// Splits off the last N suffixes of name (splitting on _) and appends them to out.
@@ -27,23 +26,33 @@
return consumed;
}
-inline static SkString find_gm_name(const Task& parent, SkTArray<SkString>* suffixList) {
+inline static SkString find_base_name(const Task& parent, SkTArray<SkString>* suffixList) {
const int suffixes = parent.depth() + 1;
const SkString& name = parent.name();
const int totalSuffixLength = split_suffixes(suffixes, name.c_str(), suffixList);
return SkString(name.c_str(), name.size() - totalSuffixLength);
}
+struct JsonData {
+ SkString name;
+ SkMD5::Digest md5;
+};
+SkTArray<JsonData> gJsonData;
+SK_DECLARE_STATIC_MUTEX(gJsonDataLock);
+
WriteTask::WriteTask(const Task& parent, SkBitmap bitmap)
: CpuTask(parent)
- , fGmName(find_gm_name(parent, &fSuffixes))
+ , fFullName(parent.name())
+ , fBaseName(find_base_name(parent, &fSuffixes))
, fBitmap(bitmap)
, fData(NULL)
- , fExtension(".png") {}
+ , fExtension(".png") {
+}
WriteTask::WriteTask(const Task& parent, SkStreamAsset *data, const char* ext)
: CpuTask(parent)
- , fGmName(find_gm_name(parent, &fSuffixes))
+ , fFullName(parent.name())
+ , fBaseName(find_base_name(parent, &fSuffixes))
, fData(data)
, fExtension(ext) {
SkASSERT(fData.get());
@@ -56,79 +65,26 @@
}
}
-namespace {
-
-// One file that first contains a .png of an SkBitmap, then its raw pixels.
-// We use this custom format to avoid premultiplied/unpremultiplied pixel conversions.
-struct PngAndRaw {
- static bool Encode(SkBitmap bitmap, const char* path) {
- SkFILEWStream stream(path);
- if (!stream.isValid()) {
- SkDebugf("Can't write %s.\n", path);
- return false;
- }
-
- // Write a PNG first for humans and other tools to look at.
- if (!SkImageEncoder::EncodeStream(&stream, bitmap, SkImageEncoder::kPNG_Type, 100)) {
- SkDebugf("Can't encode a PNG.\n");
- return false;
- }
- if (FLAGS_writePngOnly) {
- return true;
- }
-
- // Pad out so the raw pixels start 4-byte aligned.
- const uint32_t maxPadding = 0;
- const size_t pos = stream.bytesWritten();
- stream.write(&maxPadding, SkAlign4(pos) - pos);
-
- // Then write our secret raw pixels that only DM reads.
- SkAutoLockPixels lock(bitmap);
- return stream.write(bitmap.getPixels(), bitmap.getSize());
- }
-
- // This assumes bitmap already has allocated pixels of the correct size.
- static bool Decode(const char* path, SkImageInfo info, SkBitmap* bitmap) {
- SkAutoTUnref<SkData> data(SkData::NewFromFileName(path));
- if (!data) {
- SkDebugf("Can't read %s.\n", path);
- return false;
- }
-
- // The raw pixels are at the end of the file. We'll skip the encoded PNG at the front.
- const size_t rowBytes = info.minRowBytes(); // Assume densely packed.
- const size_t bitmapBytes = info.getSafeSize(rowBytes);
- if (data->size() < bitmapBytes) {
- SkDebugf("%s is too small to contain the bitmap we're looking for.\n", path);
- return false;
- }
-
- const size_t offset = data->size() - bitmapBytes;
- SkAutoTUnref<SkData> subset(
- SkData::NewSubset(data, offset, bitmapBytes));
- SkAutoTUnref<SkPixelRef> pixels(
- SkMallocPixelRef::NewWithData(
- info, rowBytes, NULL/*ctable*/, subset));
- SkASSERT(pixels);
-
- bitmap->setInfo(info, rowBytes);
- bitmap->setPixelRef(pixels);
- return true;
- }
-};
-
-// Does not take ownership of data.
-bool save_data_to_file(SkStreamAsset* data, const char* path) {
- data->rewind();
+static bool save_bitmap_to_file(SkBitmap bitmap, const char* path) {
SkFILEWStream stream(path);
- if (!stream.isValid() || !stream.writeStream(data, data->getLength())) {
- SkDebugf("Can't write %s.\n", path);
+ if (!stream.isValid() ||
+ !SkImageEncoder::EncodeStream(&stream, bitmap, SkImageEncoder::kPNG_Type, 100)) {
+ SkDebugf("Can't write a PNG to %s.\n", path);
return false;
}
return true;
}
-} // namespace
+// Does not take ownership of data.
+static bool save_data_to_file(SkStreamAsset* data, const char* path) {
+ data->rewind();
+ SkFILEWStream stream(path);
+ if (!stream.isValid() || !stream.writeStream(data, data->getLength())) {
+ SkDebugf("Can't write data to %s.\n", path);
+ return false;
+ }
+ return true;
+}
void WriteTask::draw() {
SkString dir(FLAGS_writePath[0]);
@@ -143,11 +99,29 @@
this->makeDirOrFail(dir);
}
- SkString path = SkOSPath::Join(dir.c_str(), fGmName.c_str());
+ // FIXME: MD5 is really slow. Let's use a different hash.
+ SkMD5 hasher;
+ if (fData.get()) {
+ hasher.write(fData->getMemoryBase(), fData->getLength());
+ } else {
+ SkAutoLockPixels lock(fBitmap);
+ hasher.write(fBitmap.getPixels(), fBitmap.getSize());
+ }
+
+ JsonData entry;
+ entry.name = fFullName;
+ hasher.finish(entry.md5);
+
+ {
+ SkAutoMutexAcquire lock(&gJsonDataLock);
+ gJsonData.push_back(entry);
+ }
+
+ SkString path = SkOSPath::Join(dir.c_str(), fBaseName.c_str());
path.append(fExtension);
const bool ok = fData.get() ? save_data_to_file(fData.get(), path.c_str())
- : PngAndRaw::Encode(fBitmap, path.c_str());
+ : save_bitmap_to_file(fBitmap, path.c_str());
if (!ok) {
this->fail();
}
@@ -158,7 +132,7 @@
for (int i = 0; i < fSuffixes.count(); i++) {
name.appendf("%s/", fSuffixes[i].c_str());
}
- name.append(fGmName.c_str());
+ name.append(fBaseName.c_str());
return name;
}
@@ -166,38 +140,82 @@
return FLAGS_writePath.isEmpty();
}
-static SkString path_to_expected_image(const char* root, const Task& task) {
- SkString filename = task.name();
+WriteTask::Expectations* WriteTask::Expectations::Create(const char* path) {
+ if (!FLAGS_writePath.isEmpty() && 0 == strcmp(FLAGS_writePath[0], path)) {
+ SkDebugf("We seem to be reading and writing %s concurrently. This won't work.\n", path);
+ return NULL;
+ }
- // 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());
+ SkString jsonPath;
+ if (sk_isdir(path)) {
+ jsonPath = SkOSPath::Join(path, "dm.json");
+ } else {
+ jsonPath.set(path);
+ }
- // We'll look in root/suffix for images.
- const SkString dir = SkOSPath::Join(root, suffixes[0].c_str());
+ SkAutoDataUnref json(SkData::NewFromFileName(jsonPath.c_str()));
+ if (NULL == json.get()) {
+ SkDebugf("Can't read %s!\n", jsonPath.c_str());
+ return NULL;
+ }
- // Remove the suffix and tack on a .png.
- filename.remove(filename.size() - suffixLength, suffixLength);
- filename.append(".png");
-
- return SkOSPath::Join(dir.c_str(), filename.c_str());
+ SkAutoTDelete<Expectations> expectations(SkNEW(Expectations));
+ Json::Reader reader;
+ const char* begin = (const char*)json->bytes();
+ const char* end = begin + json->size();
+ if (!reader.parse(begin, end, expectations->fJson)) {
+ SkDebugf("Can't read %s as JSON!\n", jsonPath.c_str());
+ return NULL;
+ }
+ return expectations.detach();
}
bool WriteTask::Expectations::check(const Task& task, SkBitmap bitmap) const {
- if (!FLAGS_writePath.isEmpty() && 0 == strcmp(FLAGS_writePath[0], fRoot)) {
- SkDebugf("We seem to be reading and writing %s concurrently. This won't work.\n", fRoot);
- return false;
+ const SkString name = task.name();
+ if (fJson[name.c_str()].isNull()) {
+ return true; // No expectations.
}
- const SkString path = path_to_expected_image(fRoot, task);
- SkBitmap expected;
- if (!PngAndRaw::Decode(path.c_str(), bitmap.info(), &expected)) {
- return false;
+ const char* md5Ascii = fJson[name.c_str()].asCString();
+ uint8_t md5[16];
+
+ for (int j = 0; j < 16; j++) {
+ sscanf(md5Ascii + (j*2), "%02hhx", md5 + j);
}
- return BitmapsEqual(expected, bitmap);
+ SkMD5 hasher;
+ {
+ SkAutoLockPixels lock(bitmap);
+ hasher.write(bitmap.getPixels(), bitmap.getSize());
+ }
+ SkMD5::Digest digest;
+ hasher.finish(digest);
+
+ return 0 == memcmp(md5, digest.data, 16);
+}
+
+void WriteTask::DumpJson() {
+ if (FLAGS_writePath.isEmpty()) {
+ return;
+ }
+
+ // FIXME: This JSON format is a complete MVP strawman.
+ Json::Value root;
+ {
+ SkAutoMutexAcquire lock(&gJsonDataLock);
+ for (int i = 0; i < gJsonData.count(); i++) {
+ char md5Ascii[32];
+ for (int j = 0; j < 16; j++) {
+ sprintf(md5Ascii + (j*2), "%02x", gJsonData[i].md5.data[j]);
+ }
+ root[gJsonData[i].name.c_str()] = md5Ascii;
+ }
+ }
+
+ SkString path = SkOSPath::Join(FLAGS_writePath[0], "dm.json");
+ SkFILEWStream stream(path.c_str());
+ stream.writeText(Json::StyledWriter().write(root).c_str());
+ stream.flush();
}
} // namespace DM
diff --git a/dm/DMWriteTask.h b/dm/DMWriteTask.h
index 15e1300..fad9262 100644
--- a/dm/DMWriteTask.h
+++ b/dm/DMWriteTask.h
@@ -4,6 +4,8 @@
#include "DMExpectations.h"
#include "DMTask.h"
#include "SkBitmap.h"
+#include "SkJSONCPP.h"
+#include "SkStream.h"
#include "SkString.h"
#include "SkTArray.h"
@@ -27,19 +29,22 @@
virtual bool shouldSkip() const SK_OVERRIDE;
virtual SkString name() const SK_OVERRIDE;
- // Reads image files WriteTask wrote under root and compares them with bitmap.
+ // Reads JSON file WriteTask wrote under root and compares the bitmap with checksums inside.
class Expectations : public DM::Expectations {
public:
- explicit Expectations(const char* root) : fRoot(root) {}
-
+ static Expectations* Create(const char*);
bool check(const Task& task, SkBitmap bitmap) const SK_OVERRIDE;
private:
- const char* fRoot;
+ Expectations() {}
+ Json::Value fJson;
};
+ static void DumpJson();
+
private:
SkTArray<SkString> fSuffixes;
- const SkString fGmName;
+ const SkString fFullName;
+ const SkString fBaseName;
const SkBitmap fBitmap;
SkAutoTDelete<SkStreamAsset> fData;
const char* fExtension;