From a25c0619b5ef81c32e127722660cd38753a6bcd4 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 19 Oct 2020 15:18:45 -0400 Subject: [PATCH] [canvaskit] Make gm test wrapper not omit known pngs We just make a global set of known png digests and expect that the test harness will load this with the appropriate data. Bug: skia:10812 Change-Id: I8e1fc987d36cc57386167410514803f8c3b90a69 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/328379 Reviewed-by: Chris Dalton --- modules/canvaskit/gm_bindings.cpp | 50 +++++++++++++++++++------------ 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/modules/canvaskit/gm_bindings.cpp b/modules/canvaskit/gm_bindings.cpp index ccd7f2f0535d7..66d55c7cbbf94 100644 --- a/modules/canvaskit/gm_bindings.cpp +++ b/modules/canvaskit/gm_bindings.cpp @@ -5,6 +5,7 @@ * found in the LICENSE file. */ +#include #include #include #include @@ -71,6 +72,12 @@ static sk_sp MakeGrContext(EMSCRIPTEN_WEBGL_CONTEXT_HANDLE cont return dContext; } +static std::set gKnownDigests; + +static void LoadKnownDigest(std::string md5) { + gKnownDigests.insert(md5); +} + /** * Runs the given GM and returns a JS object. If the GM was successful, the object will have the * following properties: @@ -140,31 +147,36 @@ static JSObject RunGM(sk_sp ctx, std::string name) { md5.appendf("%02x", digest.data[i]); } - // We do not need to include the keys because they are optional - they are not read by Gold. - CommandLineFlags::StringArray empty; - SkDynamicMemoryWStream stream; - // TODO(kjlubick) make emission of PNGs optional and make it so we can check the hash against - // the list of known digests to not emit it. This will hopefully speed tests up. - hashAndEncode->encodePNG(&stream, md5.c_str(), empty, empty); - - auto data = stream.detachAsData(); - - // This is the cleanest way to create a new Uint8Array with a copy of the data that is not - // in the WASM heap. kjlubick tried returning a pointer inside an SkData, but that lead to some - // use after free issues. By making the copy using the JS transliteration, we don't risk the - // SkData object being cleaned up before we make the copy. - Uint8Array pngData = emscripten::val( - // https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#memory-views - typed_memory_view(data->size(), data->bytes()) - ).call("slice"); // slice with no args makes a copy of the memory view. - - result.set("png", pngData); + auto ok = gKnownDigests.find(md5.c_str()); + if (ok == gKnownDigests.end()) { + // We only need to decode the image if it is "interesting", that is, we have not written it + // before to disk and uploaded it to gold. + SkDynamicMemoryWStream stream; + // We do not need to include the keys because they are optional - they are not read by Gold. + CommandLineFlags::StringArray empty; + hashAndEncode->encodePNG(&stream, md5.c_str(), empty, empty); + + auto data = stream.detachAsData(); + + // This is the cleanest way to create a new Uint8Array with a copy of the data that is not + // in the WASM heap. kjlubick tried returning a pointer inside an SkData, but that lead to + // some use after free issues. By making the copy using the JS transliteration, we don't + // risk the SkData object being cleaned up before we make the copy. + Uint8Array pngData = emscripten::val( + // https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#memory-views + typed_memory_view(data->size(), data->bytes()) + ).call("slice"); // slice with no args makes a copy of the memory view. + + result.set("png", pngData); + gKnownDigests.emplace(md5.c_str()); + } result.set("hash", md5.c_str()); return result; } EMSCRIPTEN_BINDINGS(GMs) { function("ListGMs", &ListGMs); + function("LoadKnownDigest", &LoadKnownDigest); function("MakeGrContext", &MakeGrContext); function("RunGM", &RunGM);