SkQP: better error report workflow
* Generate report in app after running all tests.
* Add script to unpack backup.ab file.
* Remove unused index file.
* Streamline instructions in README.md.
Change-Id: I44c80b17332eb4496ee31578287b691bd646d71a
Reviewed-on: https://skia-review.googlesource.com/86742
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
diff --git a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
index 1cf3aef..464c9e2 100644
--- a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
+++ b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
@@ -24,6 +24,7 @@
private native void nInit(AssetManager assetManager, String dataDir);
private native float nExecuteGM(int gm, int backend) throws SkQPException;
private native String[] nExecuteUnitTest(int test);
+ private native void nMakeReport();
private AssetManager mAssetManager;
private String[] mGMs;
@@ -128,6 +129,7 @@
}
notifier.fireTestFinished(desc);
}
+ this.nMakeReport();
}
}
diff --git a/tools/skqp/README.md b/tools/skqp/README.md
index d3156a0..3b311da 100644
--- a/tools/skqp/README.md
+++ b/tools/skqp/README.md
@@ -35,7 +35,6 @@
5. Generate the validation model data:
- rm -rf platform_tools/android/apps/skqp/src/main/assets/gmkb
go get go.skia.org/infra/golden/go/search
go run tools/skqp/make_gmkb.go ~/Downloads/meta.json \
platform_tools/android/apps/skqp/src/main/assets/gmkb
@@ -52,12 +51,11 @@
adb push out/${arch}-rel/skqp /data/local/tmp/
adb shell "cd /data/local/tmp; ./skqp gmkb report"
-2. Produce a one-page error report if there are errors:
+2. Get the error report if there are errors:
- rm -rf /tmp/report
if adb shell test -d /data/local/tmp/report; then
adb pull /data/local/tmp/report /tmp/
- tools/skqp/make_report.py /tmp/report
+ tools/skqp/sysopen.py /tmp/report/report.html
fi
Run as an APK
@@ -72,11 +70,8 @@
2. Retrieve the report if there are any errors:
- rm -rf /tmp/skqp
- mkdir /tmp/skqp
- adb backup -f /tmp/skqp/backup.ab org.skia.skqp
- dd if=/tmp/skqp/backup.ab bs=24 skip=1 | tools/skqp/inflate.py | \
- ( cd /tmp/skqp; tar x )
- rm /tmp/skqp/backup.ab
- tools/skqp/make_report.py /tmp/skqp/apps/org.skia.skqp/f
+ adb backup -f /tmp/skqp.ab org.skia.skqp
+ # Must unlock phone and verify backup.
+ tools/skqp/extract_report.py /tmp/skqp.ab
+
diff --git a/tools/skqp/extract_report.py b/tools/skqp/extract_report.py
new file mode 100755
index 0000000..5fc3bf4
--- /dev/null
+++ b/tools/skqp/extract_report.py
@@ -0,0 +1,29 @@
+#! /usr/bin/env python2
+# Copyright 2017 Google Inc.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import StringIO
+import os
+import sys
+import sysopen
+import tarfile
+import tempfile
+import zlib
+
+if __name__ == '__main__':
+ if len(sys.argv) != 2:
+ print 'usage: %s FILE.ab\n' % sys.argv[0]
+ exit (1)
+ with open(sys.argv[1], 'rb') as f:
+ f.read(24)
+ t = tarfile.open(fileobj=StringIO.StringIO(zlib.decompress(f.read())))
+ d = tempfile.mkdtemp(prefix='skqp_')
+ t.extractall(d)
+ p = os.path.join(d, 'apps/org.skia.skqp/f/skqp_report/report.html')
+ assert os.path.isfile(p)
+ print p
+ sysopen.sysopen(p)
+
+
+
diff --git a/tools/skqp/gm_knowledge.cpp b/tools/skqp/gm_knowledge.cpp
index bc2ca82..fb2134f 100644
--- a/tools/skqp/gm_knowledge.cpp
+++ b/tools/skqp/gm_knowledge.cpp
@@ -8,18 +8,23 @@
#include "gm_knowledge.h"
#include <cfloat>
+#include <cstdlib>
#include <fstream>
+#include <mutex>
#include <sstream>
#include <string>
#include <vector>
#include "../../src/core/SkStreamPriv.h"
+#include "../../src/core/SkTSort.h"
#include "SkBitmap.h"
#include "SkCodec.h"
#include "SkOSFile.h"
+#include "SkOSPath.h"
#include "SkPngEncoder.h"
#include "SkStream.h"
+
#include "skqp_asset_manager.h"
#define PATH_MAX_PNG "max.png"
@@ -29,27 +34,6 @@
#define PATH_REPORT "report.html"
////////////////////////////////////////////////////////////////////////////////
-inline void path_join_append(std::ostringstream* o) { }
-
-template<class... Types>
-void path_join_append(std::ostringstream* o, const char* v, Types... args) {
- constexpr char kPathSeparator[] = "/";
- *o << kPathSeparator << v;
- path_join_append(o, args...);
-}
-
-template<class... Types>
-std::string path_join(const char* v, Types... args) {
- std::ostringstream o;
- o << v;
- path_join_append(&o, args...);
- return o.str();
-}
-template<class... Types>
-std::string path_join(const std::string& v, Types... args) {
- return path_join(v.c_str(), args...);
-}
-////////////////////////////////////////////////////////////////////////////////
static int get_error(uint32_t value, uint32_t value_max, uint32_t value_min) {
int error = 0;
@@ -125,7 +109,24 @@
return bitmap;
}
+namespace {
+struct Run {
+ SkString fBackend;
+ SkString fGM;
+ int fMaxerror;
+ int fBadpixels;
+};
+} // namespace
+
+static std::vector<Run> gErrors;
+static std::mutex gMutex;
+
namespace gmkb {
+bool IsGoodGM(const char* name, skqp::AssetManager* assetManager) {
+ return asset_exists(assetManager, SkOSPath::Join(name, PATH_MAX_PNG).c_str())
+ && asset_exists(assetManager, SkOSPath::Join(name, PATH_MIN_PNG).c_str());
+}
+
// Assumes that for each GM foo, asset_manager has files foo/{max,min}.png
float Check(const uint32_t* pixels,
int width,
@@ -135,13 +136,12 @@
skqp::AssetManager* assetManager,
const char* report_directory_path,
Error* error_out) {
- using std::string;
if (width <= 0 || height <= 0) {
return set_error_code(error_out, Error::kBadInput);
}
size_t N = (unsigned)width * (unsigned)height;
- string max_path = path_join(name, PATH_MAX_PNG);
- string min_path = path_join(name, PATH_MIN_PNG);
+ SkString max_path = SkOSPath::Join(name, PATH_MAX_PNG);
+ SkString min_path = SkOSPath::Join(name, PATH_MIN_PNG);
SkBitmap max_image = ReadPngRgba8888FromFile(assetManager, max_path.c_str());
if (max_image.isNull()) {
return set_error_code(error_out, Error::kBadData);
@@ -175,11 +175,11 @@
if (!backend) {
backend = "skia";
}
- string report_directory = path_join(report_directory_path, backend);
+ SkString report_directory = SkOSPath::Join(report_directory_path, backend);
sk_mkdir(report_directory.c_str());
- string report_subdirectory = path_join(report_directory, name);
+ SkString report_subdirectory = SkOSPath::Join(report_directory.c_str(), name);
sk_mkdir(report_subdirectory.c_str());
- string error_path = path_join(report_subdirectory, PATH_IMG_PNG);
+ SkString error_path = SkOSPath::Join(report_subdirectory.c_str(), PATH_IMG_PNG);
SkAssertResult(WritePixmapToFile(rgba8888_to_pixmap(pixels, width, height),
error_path.c_str()));
SkBitmap errorBitmap;
@@ -189,18 +189,52 @@
int error = get_error(pixels[i], max_pixels[i], min_pixels[i]);
errors[i] = error > 0 ? 0xFF000000 + (unsigned)error : 0x00000000;
}
- error_path = path_join(report_subdirectory, PATH_ERR_PNG);
+ error_path = SkOSPath::Join(report_subdirectory.c_str(), PATH_ERR_PNG);
SkAssertResult(WritePixmapToFile(to_pixmap(errorBitmap), error_path.c_str()));
- auto report_path = path_join(report_subdirectory, PATH_REPORT);
- auto rdir = path_join("..", "..", backend, name);
+ SkString report_path = SkOSPath::Join(report_subdirectory.c_str(), PATH_REPORT);
- auto max_path_out = path_join(report_subdirectory, PATH_MAX_PNG);
- auto min_path_out = path_join(report_subdirectory, PATH_MIN_PNG);
+ SkString max_path_out = SkOSPath::Join(report_subdirectory.c_str(), PATH_MAX_PNG);
+ SkString min_path_out = SkOSPath::Join(report_subdirectory.c_str(), PATH_MIN_PNG);
(void)copy(assetManager, max_path.c_str(), max_path_out.c_str());
(void)copy(assetManager, min_path.c_str(), min_path_out.c_str());
+ std::lock_guard<std::mutex> lock(gMutex);
+ gErrors.push_back(Run{SkString(backend), SkString(name), badness, badPixelCount});
+ }
+ if (error_out) {
+ *error_out = Error::kNone;
+ }
+ return (float)badness;
+}
+
+bool MakeReport(const char* report_directory_path) {
+ SkFILEWStream out(SkOSPath::Join(report_directory_path, PATH_REPORT).c_str());
+ if (!out.isValid()) {
+ return false;
+ }
+ out.writeText(
+ "<!doctype html>\n"
+ "<html lang=\"en\">\n"
+ "<head>\n"
+ "<meta charset=\"UTF-8\">\n"
+ "<title>SkQP Report</title>\n"
+ "<style>\n"
+ "img { max-width:48%; border:1px green solid; }\n"
+ "</style>\n"
+ "</head>\n"
+ "<body>\n"
+ "<h1>SkQP Report</h1>\n"
+ "<hr>\n");
+ std::lock_guard<std::mutex> lock(gMutex);
+ for (const Run& run : gErrors) {
+ const SkString& backend = run.fBackend;
+ const SkString& gm = run.fGM;
+ int maxerror = run.fMaxerror;
+ int badpixels = run.fBadpixels;
+ SkString rdir = SkOSPath::Join(backend.c_str(), gm.c_str());
SkString text = SkStringPrintf(
+ "<h2>%s</h2>\n"
"backend: %s\n<br>\n"
"gm name: %s\n<br>\n"
"maximum error: %d\n<br>\n"
@@ -210,21 +244,13 @@
"<a href=\"%s/" PATH_ERR_PNG "\">"
"<img src=\"%s/" PATH_ERR_PNG "\" alt='err'></a>\n<br>\n"
"<a href=\"%s/" PATH_MAX_PNG "\">max</a>\n<br>\n"
- "<a href=\"%s/" PATH_MIN_PNG "\">min</a>\n<hr>\n",
- backend, name, badness, badPixelCount,
- rdir.c_str(), rdir.c_str(), rdir.c_str(), rdir.c_str(), rdir.c_str(), rdir.c_str());
- SkFILEWStream(report_path.c_str()).write(text.c_str(), text.size());
+ "<a href=\"%s/" PATH_MIN_PNG "\">min</a>\n<hr>\n\n",
+ rdir.c_str(), backend.c_str(), gm.c_str(), maxerror, badpixels,
+ rdir.c_str(), rdir.c_str(), rdir.c_str(),
+ rdir.c_str(), rdir.c_str(), rdir.c_str());
+ out.write(text.c_str(), text.size());
}
- if (error_out) {
- *error_out = Error::kNone;
- }
- return (float)badness;
-}
-
-bool IsGoodGM(const char* name, skqp::AssetManager* assetManager) {
- std::string max_path = path_join(name, PATH_MAX_PNG);
- std::string min_path = path_join(name, PATH_MIN_PNG);
- return asset_exists(assetManager, max_path.c_str())
- && asset_exists(assetManager, min_path.c_str());
+ out.writeText("</body>\n</html>\n");
+ return true;
}
} // namespace gmkb
diff --git a/tools/skqp/gm_knowledge.h b/tools/skqp/gm_knowledge.h
index 4aca00e..25399c4 100644
--- a/tools/skqp/gm_knowledge.h
+++ b/tools/skqp/gm_knowledge.h
@@ -66,6 +66,12 @@
*/
bool IsGoodGM(const char* name, skqp::AssetManager* assetManager);
+/**
+Call this after running all checks.
+
+@param report_directory_path locatation to write report to.
+*/
+bool MakeReport(const char* report_directory_path);
} // namespace gmkb
#endif // gm_knowledge_DEFINED
diff --git a/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp b/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
index 9c9c9df..22d2c03 100644
--- a/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
+++ b/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
@@ -13,6 +13,7 @@
#include <android/asset_manager_jni.h>
#include "gm_runner.h"
+#include "gm_knowledge.h"
#include "skqp_asset_manager.h"
#include "SkStream.h"
@@ -22,6 +23,7 @@
JNIEXPORT jfloat JNICALL Java_org_skia_skqp_SkQPRunner_nExecuteGM(JNIEnv*, jobject, jint, jint);
JNIEXPORT jobjectArray JNICALL Java_org_skia_skqp_SkQPRunner_nExecuteUnitTest(JNIEnv*, jobject,
jint);
+JNIEXPORT void JNICALL Java_org_skia_skqp_SkQPRunner_nMakeReport(JNIEnv*, jobject);
} // extern "C"
////////////////////////////////////////////////////////////////////////////////
@@ -128,7 +130,7 @@
jassert(env, gAssetManager.fMgr);
const char* dataDirString = env->GetStringUTFChars(dataDir, nullptr);
- gReportDirectory = dataDirString;
+ gReportDirectory = std::string(dataDirString) + "/skqp_report";
env->ReleaseStringUTFChars(dataDir, dataDirString);
gBackends = gm_runner::GetSupportedBackends();
@@ -191,5 +193,14 @@
return (jobjectArray)env->NewGlobalRef(array);
}
+void Java_org_skia_skqp_SkQPRunner_nMakeReport(JNIEnv*, jobject) {
+ std::string reportDirectoryPath;
+ {
+ std::lock_guard<std::mutex> lock(gMutex);
+ reportDirectoryPath = gReportDirectory;
+ }
+ (void)gmkb::MakeReport(reportDirectoryPath.c_str());
+}
+
////////////////////////////////////////////////////////////////////////////////
diff --git a/tools/skqp/make_gmkb.go b/tools/skqp/make_gmkb.go
index b8b67a3..f839375 100644
--- a/tools/skqp/make_gmkb.go
+++ b/tools/skqp/make_gmkb.go
@@ -174,8 +174,11 @@
}
input := os.Args[1]
output := os.Args[2]
- err := os.MkdirAll(output, os.ModePerm)
- if err != nil && !os.IsExist(err) {
+ // output is removed and replaced with a clean directory.
+ if err := os.RemoveAll(output); err != nil && !os.IsNotExist(err) {
+ log.Fatal(err)
+ }
+ if err := os.MkdirAll(output, os.ModePerm); err != nil && !os.IsExist(err) {
log.Fatal(err)
}
@@ -185,12 +188,6 @@
}
sort.Sort(ExportTestRecordArray(records))
- index, err := os.Create(path.Join(output, "index.txt"))
- if err != nil {
- log.Fatal(err)
- }
- defer index.Close()
-
var wg sync.WaitGroup
for _, record := range records {
if in(record.TestName, blacklist) {
@@ -212,11 +209,6 @@
}
fmt.Printf("\r%-60s", testName)
}(record.TestName, goodUrls, output)
-
- _, err = fmt.Fprintf(index, "%s\n", record.TestName)
- if err != nil {
- log.Fatal(err)
- }
}
wg.Wait()
fmt.Printf("\r%60s\n", "")
diff --git a/tools/skqp/make_report.py b/tools/skqp/make_report.py
deleted file mode 100755
index 4345575..0000000
--- a/tools/skqp/make_report.py
+++ /dev/null
@@ -1,41 +0,0 @@
-#! /usr/bin/env python
-
-# Copyright 2017 Google Inc.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-import glob
-import os
-import re
-import sys
-
-import sysopen
-
-if len(sys.argv) == 2:
- if not os.path.isdir(sys.argv[1]):
- exit(1)
- os.chdir(sys.argv[1])
-
-head = '''<!doctype html>
-<html lang="en">
-<head>
-<meta charset="UTF-8">
-<title>SkQP Report</title>
-<style>
-img { max-width:48%; border:1px green solid; }
-</style>
-</head>
-<body>
-<h1>SkQP Report</h1>
-<hr>
-'''
-
-reg = re.compile('="../../')
-with open('report.html', 'w') as o:
- o.write(head)
- for x in glob.iglob('*/*/report.html'):
- with open(x, 'r') as f:
- o.write(reg.sub('="', f.read()))
- o.write('</body>\n</html>\n')
-
-sysopen.sysopen('report.html')
diff --git a/tools/skqp/skqp.cpp b/tools/skqp/skqp.cpp
index b9864cd..fbd4c1e 100644
--- a/tools/skqp/skqp.cpp
+++ b/tools/skqp/skqp.cpp
@@ -5,6 +5,7 @@
* found in the LICENSE file.
*/
+#include "gm_knowledge.h"
#include "gm_runner.h"
#ifdef __clang__
@@ -136,5 +137,7 @@
gReportDirectoryPath = argv[2];
}
register_skia_tests();
- return RUN_ALL_TESTS();
+ int ret = RUN_ALL_TESTS();
+ (void)gmkb::MakeReport(gReportDirectoryPath.c_str());
+ return ret;
}