SkQP: replace blacklist with: DoNotExecuteInExperimentalMode and NoScoreInCompatibilityTestMode

Also clean up some things, fix docs, whitelist.

Change-Id: I2818d973978ffe1b8ce0cc9c69f8d91ab4a0ef22
Reviewed-on: https://skia-review.googlesource.com/91805
Reviewed-by: Hal Canary <halcanary@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>

9 files changed, 136 insertions, 125 deletions

diff --git a/tools/skqp/README.md b/tools/skqp/README.md
index 8b61174f2e..0a0c8db671 100644
--- a/tools/skqp/README.md
+++ b/tools/skqp/README.md
@@ -19,7 +19,7 @@ How To Use SkQP on your Android device:
 3.  Configure and build Skia for your device's architecture:
 
         arch='arm64'  # Also valid: 'arm', 'x68', 'x64'
-        android_ndk="${HOME}/ndk"  # Or wherever you installed the NDK.
+        android_ndk="${HOME}/android-ndk"  # Or wherever you installed the NDK.
         mkdir -p out/${arch}-rel
         cat > out/${arch}-rel/args.gn << EOF
             ndk = "$android_ndk"
@@ -37,7 +37,7 @@ How To Use SkQP on your Android device:
 
 5.  Generate the validation model data:
 
-        go get go.skia.org/infra/golden/go/search
+        go get -u 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
 
@@ -47,18 +47,16 @@ Run as an executable
 1.  Build the SkQP program, load files on the device, and run skqp:
 
         ninja -C out/${arch}-rel skqp
-        adb shell "cd /data/local/tmp; rm -rf gmkb report"
-        adb push platform_tools/android/apps/skqp/src/main/assets/gmkb \
-            /data/local/tmp/
+        adb shell "cd /data/local/tmp; rm -rf skqp_assets report"
+        adb push platform_tools/android/apps/skqp/src/main/assets \
+            /data/local/tmp/skqp_assets
         adb push out/${arch}-rel/skqp /data/local/tmp/
-        adb shell "cd /data/local/tmp; ./skqp gmkb report"
+        adb shell "cd /data/local/tmp; ./skqp skqp_assets report"
 
 2.  Get the error report if there are errors:
 
-        if adb shell test -d /data/local/tmp/report; then
-            adb pull /data/local/tmp/report /tmp/
-            tools/skqp/sysopen.py /tmp/report/report.html
-        fi
+        adb pull /data/local/tmp/report /tmp/
+        tools/skqp/sysopen.py /tmp/report/report.html
 
 Run as an APK
 -------------
@@ -74,13 +72,18 @@ Run as an APK
 
         platform_tools/android/bin/android_build_app -C out/${arch}-rel skqp
         adb install -r out/${arch}-rel/skqp.apk
+        adb logcat -c
         adb shell am instrument -w \
             org.skia.skqp/android.support.test.runner.AndroidJUnitRunner
 
-2.  Retrieve the report if there are any errors:
+2.  Find out where the report went (and look for Skia errors):
+
+        adb logcat -d org.skia.skqp skia "*:S"
+
+3.  Retrieve and view the report if there are any errors:
+
+        adb pull /storage/emulated/0/Android/data/org.skia.skqp/files/output /tmp/
+        tools/skqp/sysopen.py /tmp/output/skqp_report/report.html
 
-        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
deleted file mode 100755
index 5fc3bf4ebe..0000000000
--- a/tools/skqp/extract_report.py
+++ /dev/null
@@ -1,29 +0,0 @@
-#! /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 201331c4bf..89a885cbb8 100644
--- a/tools/skqp/gm_knowledge.cpp
+++ b/tools/skqp/gm_knowledge.cpp
@@ -72,10 +72,6 @@ static SkPixmap rgba8888_to_pixmap(const uint32_t* pixels, int width, int height
     return SkPixmap(info, pixels, width * sizeof(uint32_t));
 }
 
-static bool asset_exists(skqp::AssetManager* mgr, const char* path) {
-    return mgr && nullptr != mgr->open(path);
-}
-
 static bool copy(skqp::AssetManager* mgr, const char* path, const char* dst) {
     if (mgr) {
         if (auto stream = mgr->open(path)) {
@@ -114,13 +110,6 @@ 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 and
-// that the report_directory_path already exists on disk.
 float Check(const uint32_t* pixels,
             int width,
             int height,
@@ -129,19 +118,25 @@ float Check(const uint32_t* pixels,
             skqp::AssetManager* assetManager,
             const char* report_directory_path,
             Error* error_out) {
+    if (report_directory_path && report_directory_path[0]) {
+        SkASSERT_RELEASE(sk_isdir(report_directory_path));
+    }
     if (width <= 0 || height <= 0) {
         return set_error_code(error_out, Error::kBadInput);
     }
     size_t N = (unsigned)width * (unsigned)height;
-    SkString max_path = SkOSPath::Join(name, PATH_MAX_PNG);
-    SkString min_path = SkOSPath::Join(name, PATH_MIN_PNG);
+    constexpr char PATH_ROOT[] = "gmkb";
+    SkString img_path = SkOSPath::Join(PATH_ROOT, name);
+    SkString max_path = SkOSPath::Join(img_path.c_str(), PATH_MAX_PNG);
+    SkString min_path = SkOSPath::Join(img_path.c_str(), PATH_MIN_PNG);
     SkBitmap max_image = ReadPngRgba8888FromFile(assetManager, max_path.c_str());
-    if (max_image.isNull()) {
-        return set_error_code(error_out, Error::kBadData);
-    }
     SkBitmap min_image = ReadPngRgba8888FromFile(assetManager, min_path.c_str());
-    if (min_image.isNull()) {
-        return set_error_code(error_out, Error::kBadData);
+    if (max_image.isNull() || min_image.isNull()) {
+        // No data.
+        if (error_out) {
+            *error_out = Error::kNone;
+        }
+        return 0;
     }
     if (max_image.width()  != min_image.width() ||
         max_image.height() != min_image.height())
@@ -168,7 +163,6 @@ float Check(const uint32_t* pixels,
         gErrors.push_back(Run{SkString(backend), SkString(name), 0, 0});
     }
     if (report_directory_path && badness > 0 && report_directory_path[0] != '\0') {
-        sk_mkdir(report_directory_path);
         if (!backend) {
             backend = "skia";
         }
@@ -206,9 +200,11 @@ float Check(const uint32_t* pixels,
 }
 
 bool MakeReport(const char* report_directory_path) {
+    SkASSERT_RELEASE(sk_isdir(report_directory_path));
     std::lock_guard<std::mutex> lock(gMutex);
     {
         SkFILEWStream csvOut(SkOSPath::Join(report_directory_path, "out.csv").c_str());
+        SkASSERT_RELEASE(csvOut.isValid());
         if (!csvOut.isValid()) {
             return false;
         }
@@ -220,7 +216,6 @@ bool MakeReport(const char* report_directory_path) {
         }
     }
 
-    sk_mkdir(report_directory_path);
     SkFILEWStream out(SkOSPath::Join(report_directory_path, PATH_REPORT).c_str());
     if (!out.isValid()) {
         return false;
diff --git a/tools/skqp/gm_knowledge.h b/tools/skqp/gm_knowledge.h
index 25399c4c0d..6a19a87034 100644
--- a/tools/skqp/gm_knowledge.h
+++ b/tools/skqp/gm_knowledge.h
@@ -57,16 +57,6 @@ float Check(const uint32_t* pixels,
             Error* error_out);
 
 /**
-Check to see if the given test has expected results.
-
-@param name          The name of a rendering test.
-@param assetManager  GM KnowledgeBase data files
-
-@return true of expected results are known for the given test.
-*/
-bool IsGoodGM(const char* name, skqp::AssetManager* assetManager);
-
-/**
 Call this after running all checks.
 
 @param report_directory_path  locatation to write report to.
diff --git a/tools/skqp/gm_runner.cpp b/tools/skqp/gm_runner.cpp
index 3c3885ef5c..980612b7b6 100644
--- a/tools/skqp/gm_runner.cpp
+++ b/tools/skqp/gm_runner.cpp
@@ -22,6 +22,48 @@
 #include "gm_knowledge.h"
 #include "vk/VkTestContext.h"
 
+static SkTHashSet<SkString> gDoNotScoreInCompatibilityTestMode;
+static SkTHashSet<SkString> gDoNotExecuteInExperimentalMode;
+static SkTHashSet<SkString> gKnownGpuUnitTests;
+static SkTHashSet<SkString> gKnownGMs;
+static gm_runner::Mode gMode = gm_runner::Mode::kCompatibilityTestMode;
+
+static bool is_empty(const SkTHashSet<SkString>& set) {
+    return 0 == set.count();
+}
+static bool in_set(const char* s, const SkTHashSet<SkString>& set) {
+    return !is_empty(set) && nullptr != set.find(SkString(s));
+}
+
+static void readlist(skqp::AssetManager* mgr, const char* path, SkTHashSet<SkString>* dst) {
+    auto asset = mgr->open(path);
+    SkASSERT_RELEASE(asset);
+    if (!asset || asset->getLength() == 0) {
+        return;
+    }
+    std::vector<char> buffer(asset->getLength() + 1);
+    asset->read(buffer.data(), buffer.size());
+    buffer.back() = '\0';
+    const char* ptr = buffer.data();
+    const char* end = &buffer.back();
+    SkASSERT(ptr < end);
+    while (true) {
+        while (*ptr == '\n' && ptr < end) {
+            ++ptr;
+        }
+        if (ptr == end) {
+            return;
+        }
+        const char* find = strchr(ptr, '\n');
+        if (!find) {
+            find = end;
+        }
+        SkASSERT(find > ptr);
+        dst->add(SkString(ptr, find - ptr));
+        ptr = find;
+    }
+}
+
 namespace gm_runner {
 
 const char* GetErrorString(Error e) {
@@ -57,10 +99,15 @@ std::vector<UnitTest> GetUnitTests() {
     std::vector<UnitTest> tests;
     for (const skiatest::TestRegistry* r = skiatest::TestRegistry::Head(); r; r = r->next()) {
         const skiatest::Test& test = r->factory();
-        if (test.needsGpu) {
+        if ((is_empty(gKnownGpuUnitTests) || in_set(test.name, gKnownGpuUnitTests))
+            && test.needsGpu) {
             tests.push_back(&test);
         }
     }
+    struct {
+        bool operator()(UnitTest u, UnitTest v) const { return strcmp(u->name, v->name) < 0; }
+    } less;
+    std::sort(tests.begin(), tests.end(), less);
     return tests;
 }
 
@@ -118,6 +165,7 @@ std::vector<SkiaBackend> GetSupportedBackends() {
             }
         }
     }
+    SkASSERT_RELEASE(result.size() > 0);
     return result;
 }
 
@@ -167,14 +215,21 @@ std::tuple<float, Error> EvaluateGM(SkiaBackend backend,
                                     skqp::AssetManager* assetManager,
                                     const char* reportDirectoryPath) {
     std::vector<uint32_t> pixels;
+    SkASSERT(gmFact);
     std::unique_ptr<skiagm::GM> gm(gmFact(nullptr));
+    SkASSERT(gm);
+    const char* name = gm->getName();
     int width = 0, height = 0;
     if (!evaluate_gm(backend, gm.get(), &width, &height, &pixels)) {
         return std::make_tuple(FLT_MAX, Error::SkiaFailure);
     }
+    if (Mode::kCompatibilityTestMode == gMode && in_set(name, gDoNotScoreInCompatibilityTestMode)) {
+        return std::make_tuple(0, Error::None);
+    }
+
     gmkb::Error e;
     float value = gmkb::Check(pixels.data(), width, height,
-                              gm->getName(), GetBackendName(backend), assetManager,
+                              name, GetBackendName(backend), assetManager,
                               reportDirectoryPath, &e);
     Error error = gmkb::Error::kBadInput == e ? Error::BadSkiaOutput
                 : gmkb::Error::kBadData  == e ? Error::BadGMKBData
@@ -182,19 +237,29 @@ std::tuple<float, Error> EvaluateGM(SkiaBackend backend,
     return std::make_tuple(value, error);
 }
 
-void InitSkia() {
+void InitSkia(Mode mode, skqp::AssetManager* mgr) {
     SkGraphics::Init();
     gSkFontMgr_DefaultFactory = &DM::MakeFontMgr;
+
+    gMode = mode;
+    readlist(mgr, "skqp/DoNotScoreInCompatibilityTestMode.txt",
+             &gDoNotScoreInCompatibilityTestMode);
+    readlist(mgr, "skqp/DoNotExecuteInExperimentalMode.txt", &gDoNotExecuteInExperimentalMode);
+    readlist(mgr, "skqp/KnownGpuUnitTests.txt", &gKnownGpuUnitTests);
+    readlist(mgr, "skqp/KnownGMs.txt", &gKnownGMs);
 }
 
 std::vector<GMFactory> GetGMFactories(skqp::AssetManager* assetManager) {
     std::vector<GMFactory> result;
     for (const skiagm::GMRegistry* r = skiagm::GMRegistry::Head(); r; r = r->next()) {
         GMFactory f = r->factory();
-
-        if (gmkb::IsGoodGM(GetGMName(f).c_str(), assetManager)) {
-            result.push_back(r->factory());
-            SkASSERT(result.back());
+        SkASSERT(f);
+        auto name = GetGMName(f);
+        if ((is_empty(gKnownGMs) || in_set(name.c_str(), gKnownGMs)) &&
+            !(Mode::kExperimentalMode == gMode &&
+              in_set(name.c_str(), gDoNotExecuteInExperimentalMode)))
+        {
+            result.push_back(f);
         }
     }
     struct {
diff --git a/tools/skqp/gm_runner.h b/tools/skqp/gm_runner.h
index 690b3714e6..2707966c9c 100644
--- a/tools/skqp/gm_runner.h
+++ b/tools/skqp/gm_runner.h
@@ -37,10 +37,20 @@ enum class SkiaBackend {
     kVulkan,
 };
 
+enum class Mode {
+    /** This mode is set when used by Android CTS.  All known tests are executed.  */
+    kCompatibilityTestMode,
+    /** This mode is set when used in the test lab.  Some tests are skipped, if
+        they are known to cause crashes in older devices.  All GMs are evaluated
+        with stricter requirements. */
+    kExperimentalMode,
+
+};
+
 /**
 Initialize Skia
 */
-void InitSkia();
+void InitSkia(Mode, skqp::AssetManager*);
 
 std::vector<SkiaBackend> GetSupportedBackends();
 
diff --git a/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp b/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
index 7347f28ff9..7864fc43c4 100644
--- a/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
+++ b/tools/skqp/jni/org_skia_skqp_SkQPRunner.cpp
@@ -8,9 +8,10 @@
 #include <mutex>
 #include <vector>
 
-#include <jni.h>
 #include <android/asset_manager.h>
 #include <android/asset_manager_jni.h>
+#include <jni.h>
+#include <sys/stat.h>
 
 #include "gm_runner.h"
 #include "gm_knowledge.h"
@@ -19,7 +20,7 @@
 
 ////////////////////////////////////////////////////////////////////////////////
 extern "C" {
-JNIEXPORT void JNICALL Java_org_skia_skqp_SkQP_nInit(JNIEnv*, jobject, jobject, jstring);
+JNIEXPORT void JNICALL Java_org_skia_skqp_SkQP_nInit(JNIEnv*, jobject, jobject, jstring, jboolean);
 JNIEXPORT jfloat JNICALL Java_org_skia_skqp_SkQP_nExecuteGM(JNIEnv*, jobject, jint, jint);
 JNIEXPORT jobjectArray JNICALL Java_org_skia_skqp_SkQP_nExecuteUnitTest(JNIEnv*, jobject,
                                                                               jint);
@@ -74,13 +75,13 @@ struct AndroidAssetManager : public skqp::AssetManager {
                 return dup;
             }
         };
+        // SkDebugf("AndroidAssetManager::open(\"%s\");", path);
         AAsset* asset = AndroidAssetManager::OpenAsset(fMgr, path);
         return asset ? std::unique_ptr<SkStreamAsset>(new AAStrm(fMgr, std::string(path), asset))
                      : nullptr;
     }
     static AAsset* OpenAsset(AAssetManager* mgr, const char* path) {
-        std::string fullPath = std::string("gmkb/") + path;
-        return mgr ? AAssetManager_open(mgr, fullPath.c_str(), AASSET_MODE_STREAMING) : nullptr;
+        return mgr ? AAssetManager_open(mgr, path, AASSET_MODE_STREAMING) : nullptr;
     }
 };
 }
@@ -119,23 +120,32 @@ jobjectArray to_java_string_array(JNIEnv* env,
 }
 
 void Java_org_skia_skqp_SkQP_nInit(JNIEnv* env, jobject object, jobject assetManager,
-                                         jstring dataDir) {
+                                         jstring dataDir, jboolean experimentalMode) {
     jclass clazz = env->GetObjectClass(object);
     jassert(env, assetManager);
 
-    gm_runner::InitSkia();
-
     std::lock_guard<std::mutex> lock(gMutex);
     gAssetManager.fMgr = AAssetManager_fromJava(env, assetManager);
     jassert(env, gAssetManager.fMgr);
 
+    gm_runner::InitSkia(experimentalMode ? gm_runner::Mode::kExperimentalMode
+                                         : gm_runner::Mode::kCompatibilityTestMode,
+                        &gAssetManager);
+
     const char* dataDirString = env->GetStringUTFChars(dataDir, nullptr);
+    jassert(env, dataDirString && dataDirString[0]);
     gReportDirectory =  std::string(dataDirString) + "/skqp_report";
+    int mkdirRetval = mkdir(gReportDirectory.c_str(), 0777);
+    SkASSERT_RELEASE(0 == mkdirRetval);
+
     env->ReleaseStringUTFChars(dataDir, dataDirString);
 
     gBackends = gm_runner::GetSupportedBackends();
+    jassert(env, gBackends.size() > 0);
     gGMs = gm_runner::GetGMFactories(&gAssetManager);
+    jassert(env, gGMs.size() > 0);
     gUnitTests = gm_runner::GetUnitTests();
+    jassert(env, gUnitTests.size() > 0);
     gStringClass = env->FindClass("java/lang/String");
 
     constexpr char stringArrayType[] = "[Ljava/lang/String;";
diff --git a/tools/skqp/make_gmkb.go b/tools/skqp/make_gmkb.go
index f839375d83..97e44e449e 100644
--- a/tools/skqp/make_gmkb.go
+++ b/tools/skqp/make_gmkb.go
@@ -44,39 +44,6 @@ func in(v string, a []string) bool {
 	return false
 }
 
-// TODO(halcanary): clean up this blacklist.
-var blacklist = []string{
-	"circular-clips",
-	"colorcomposefilter_wacky",
-	"coloremoji_blendmodes",
-	"colormatrix",
-	"complexclip_bw",
-	"complexclip_bw_invert",
-	"complexclip_bw_layer",
-	"complexclip_bw_layer_invert",
-	"convex-lineonly-paths-stroke-and-fill",
-	"dftext",
-	"downsamplebitmap_image_high_mandrill_512.png",
-	"downsamplebitmap_image_medium_mandrill_512.png",
-	"filterbitmap_image_mandrill_16.png",
-	"filterbitmap_image_mandrill_64.png",
-	"filterbitmap_image_mandrill_64.png_g8",
-	"gradients_degenerate_2pt",
-	"gradients_degenerate_2pt_nodither",
-	"gradients_local_perspective",
-	"gradients_local_perspective_nodither",
-	"imagefilterstransformed",
-	"image_scale_aligned",
-	"lattice",
-	"linear_gradient",
-	"mipmap_srgb",
-	"mixedtextblobs",
-	"OverStroke",
-	"simple-offsetimagefilter",
-	"strokerect",
-	"textblobmixedsizes",
-	"textblobmixedsizes_df"}
-
 func processTest(testName string, imgUrls []string, output string) error {
 	if strings.ContainsRune(testName, '/') {
 		return nil
@@ -190,9 +157,6 @@ func main() {
 
 	var wg sync.WaitGroup
 	for _, record := range records {
-		if in(record.TestName, blacklist) {
-			continue
-		}
 		var goodUrls []string
 		for _, digest := range record.Digests {
 			if (in("vk", digest.ParamSet["config"]) ||
diff --git a/tools/skqp/skqp.cpp b/tools/skqp/skqp.cpp
index fbd4c1e006..b3824a817a 100644
--- a/tools/skqp/skqp.cpp
+++ b/tools/skqp/skqp.cpp
@@ -5,6 +5,8 @@
  * found in the LICENSE file.
  */
 
+#include <sys/stat.h>
+
 #include "gm_knowledge.h"
 #include "gm_runner.h"
 
@@ -92,7 +94,7 @@ static void reg_test(const char* test, const char* testCase,
 
 
 void register_skia_tests() {
-    gm_runner::InitSkia();
+    gm_runner::InitSkia(gm_runner::Mode::kCompatibilityTestMode, gAssetMgr.get());
 
     // Rendering Tests
     std::vector<gm_runner::SkiaBackend> backends = gm_runner::GetSupportedBackends();
@@ -135,6 +137,7 @@ int main(int argc, char** argv) {
     gAssetMgr.reset(new StdAssetManager(argv[1]));
     if (argc > 2) {
         gReportDirectoryPath = argv[2];
+        (void)mkdir(gReportDirectoryPath.c_str(), 0777);
     }
     register_skia_tests();
     int ret = RUN_ALL_TESTS();