Make ProcessorOptimizationValidationTest more forgiving
Bug: skia:
Change-Id: I3ff965644a0a59af1e2a2b1ea226a1417a234ced
Reviewed-on: https://skia-review.googlesource.com/c/158342
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp
index 4c1e806..6107b50 100644
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -359,6 +359,7 @@
// Encoded images are very verbose and this tests many potential images, so only export the
// first failure (subsequent failures have a reasonable chance of being related).
bool loggedFirstFailure = false;
+ bool loggedFirstWarning = false;
// Because processor factories configure themselves in random ways, this is not exhaustive.
for (int i = 0; i < FPFactory::Count(); ++i) {
int timesToInvokeFactory = 5;
@@ -389,7 +390,6 @@
rtc->readPixels(SkImageInfo::Make(kRenderSize, kRenderSize, kRGBA_8888_SkColorType,
kPremul_SkAlphaType),
readData.get(), 0, 0, 0);
- bool passing = true;
if (0) { // Useful to see what FPs are being tested.
SkString children;
for (int c = 0; c < clone->numChildProcessors(); ++c) {
@@ -401,8 +401,26 @@
}
SkDebugf("%s %s\n", clone->name(), children.c_str());
}
- for (int y = 0; y < kRenderSize && passing; ++y) {
- for (int x = 0; x < kRenderSize && passing; ++x) {
+
+ // This test has a history of being flaky on a number of devices. If an FP is logically
+ // violating the optimizations, it's reasonable to expect it to violate requirements on
+ // a large number of pixels in the image. Sporadic pixel violations are more indicative
+ // of device errors and represents a separate problem.
+#if defined(SK_SKQP_GLOBAL_ERROR_TOLERANCE)
+ static constexpr int kMaxAcceptableFailedPixels = 0; // Strict when running as SKQP
+#else
+ static constexpr int kMaxAcceptableFailedPixels = 2 * kRenderSize; // ~0.7% of the image
+#endif
+
+ int failedPixelCount = 0;
+ // Collect first optimization failure message, to be output later as a warning or an
+ // error depending on whether the rendering "passed" or failed.
+ SkString coverageMessage;
+ SkString opaqueMessage;
+ SkString constMessage;
+ for (int y = 0; y < kRenderSize; ++y) {
+ for (int x = 0; x < kRenderSize; ++x) {
+ bool passing = true;
GrColor input = input_texel_color(x, y);
GrColor output = readData.get()[y * kRenderSize + x];
if (clone->compatibleWithCoverageAsAlpha()) {
@@ -419,11 +437,13 @@
GrColorUnpackG(output) <= GrColorUnpackA(input) &&
GrColorUnpackB(output) <= GrColorUnpackA(input);
if (!legalColorModulation && !legalAlphaModulation) {
- ERRORF(reporter,
- "\"Modulating\" processor %s made color/alpha value larger. "
- "Input: 0x%08x, Output: 0x%08x, pixel (%d, %d).",
- clone->name(), input, output, x, y);
passing = false;
+
+ if (coverageMessage.isEmpty()) {
+ coverageMessage.printf("\"Modulating\" processor %s made color/"
+ "alpha value larger. Input: 0x%08x, Output: 0x%08x, pixel "
+ "(%d, %d).", clone->name(), input, output, x, y);
+ }
}
}
SkPMColor4f input4f = input_texel_color4f(x, y);
@@ -436,46 +456,93 @@
float aDiff = fabsf(output4f.fRGBA[3] - expected4f.fA);
static constexpr float kTol = 4 / 255.f;
if (rDiff > kTol || gDiff > kTol || bDiff > kTol || aDiff > kTol) {
- ERRORF(reporter,
- "Processor %s claimed output for const input doesn't match "
- "actual output. Error: %f, Tolerance: %f, input: (%f, %f, %f, "
- "%f), actual: (%f, %f, %f, %f), expected(%f, %f, %f, %f)",
- clone->name(),
- SkTMax(rDiff, SkTMax(gDiff, SkTMax(bDiff, aDiff))), kTol,
- input4f.fR, input4f.fG, input4f.fB, input4f.fA,
- output4f.fRGBA[0], output4f.fRGBA[1], output4f.fRGBA[2],
- output4f.fRGBA[3],
- expected4f.fR, expected4f.fG, expected4f.fB, expected4f.fA);
- passing = false;
+ if (constMessage.isEmpty()) {
+ passing = false;
+
+ constMessage.printf("Processor %s claimed output for const input "
+ "doesn't match actual output. Error: %f, Tolerance: %f, "
+ "input: (%f, %f, %f, %f), actual: (%f, %f, %f, %f), "
+ "expected(%f, %f, %f, %f)", clone->name(),
+ SkTMax(rDiff, SkTMax(gDiff, SkTMax(bDiff, aDiff))), kTol,
+ input4f.fR, input4f.fG, input4f.fB, input4f.fA,
+ output4f.fRGBA[0], output4f.fRGBA[1], output4f.fRGBA[2],
+ output4f.fRGBA[3], expected4f.fR, expected4f.fG,
+ expected4f.fB, expected4f.fA);
+ }
}
}
if (GrColorIsOpaque(input) && clone->preservesOpaqueInput() &&
!GrColorIsOpaque(output)) {
- ERRORF(reporter,
- "Processor %s claimed opaqueness is preserved but it is not. Input: "
- "0x%08x, Output: 0x%08x.",
- clone->name(), input, output);
passing = false;
- }
- if (!passing) {
- if (loggedFirstFailure) {
- // Do not export images
- ERRORF(reporter, "Seed: 0x%08x, Processor details: %s", seed,
- clone->dumpInfo().c_str());
- } else {
- SkString input;
- log_surface_proxy(context, inputTexture, &input);
- SkString output;
- log_surface_context(rtc, &output);
- ERRORF(reporter, "Seed: 0x%08x, Processor details: %s\n\n"
- "===========================================================\n\n"
- "Input image: %s\n\n"
- "===========================================================\n\n"
- "Output image: %s\n", seed, clone->dumpInfo().c_str(),
- input.c_str(), output.c_str());
- loggedFirstFailure = true;
+
+ if (opaqueMessage.isEmpty()) {
+ opaqueMessage.printf("Processor %s claimed opaqueness is preserved but "
+ "it is not. Input: 0x%08x, Output: 0x%08x.",
+ clone->name(), input, output);
}
}
+
+ if (!passing) {
+ // Regardless of how many optimizations the pixel violates, count it as a
+ // single bad pixel.
+ failedPixelCount++;
+ }
+ }
+ }
+
+ // Finished analyzing the entire image, see if the number of pixel failures meets the
+ // threshold for an FP violating the optimization requirements.
+ if (failedPixelCount > kMaxAcceptableFailedPixels) {
+ ERRORF(reporter, "Processor violated %d of %d pixels, seed: 0x%08x, processor: %s",
+ ", first failing pixel details are below:",
+ failedPixelCount, kRenderSize * kRenderSize, seed,
+ clone->dumpInfo().c_str());
+
+ // Print first failing pixel's details.
+ if (!coverageMessage.isEmpty()) {
+ ERRORF(reporter, coverageMessage.c_str());
+ }
+ if (!constMessage.isEmpty()) {
+ ERRORF(reporter, constMessage.c_str());
+ }
+ if (!opaqueMessage.isEmpty()) {
+ ERRORF(reporter, opaqueMessage.c_str());
+ }
+
+ if (!loggedFirstFailure) {
+ // Print with ERRORF to make sure the encoded image is output
+ SkString input;
+ log_surface_proxy(context, inputTexture, &input);
+ SkString output;
+ log_surface_context(rtc, &output);
+ ERRORF(reporter, "Input image: %s\n\n"
+ "===========================================================\n\n"
+ "Output image: %s\n", input.c_str(), output.c_str());
+ loggedFirstFailure = true;
+ }
+ } else {
+ // Don't trigger an error, but don't just hide the failures either.
+ INFOF(reporter, "Processor violated %d of %d pixels (below error threshold), seed: "
+ "0x%08x, processor: %s", failedPixelCount, kRenderSize * kRenderSize,
+ seed, clone->dumpInfo().c_str());
+ if (!coverageMessage.isEmpty()) {
+ INFOF(reporter, coverageMessage.c_str());
+ }
+ if (!constMessage.isEmpty()) {
+ INFOF(reporter, constMessage.c_str());
+ }
+ if (!opaqueMessage.isEmpty()) {
+ INFOF(reporter, opaqueMessage.c_str());
+ }
+ if (!loggedFirstWarning) {
+ SkString input;
+ log_surface_proxy(context, inputTexture, &input);
+ SkString output;
+ log_surface_context(rtc, &output);
+ INFOF(reporter, "Input image: %s\n\n"
+ "===========================================================\n\n"
+ "Output image: %s\n", input.c_str(), output.c_str());
+ loggedFirstWarning = true;
}
}
}