From 6a6d8ccb46a70a83fa6c377b736a7aec51a4ad2d Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Mon, 3 Apr 2023 15:50:12 -0700 Subject: [PATCH] [Impeller] Adds the ability to specify a golden threshold (#40824) [Impeller] Adds the ability to specify a golden threshold --- impeller/aiks/aiks_unittests.cc | 3 --- impeller/golden_tests/golden_digest.cc | 21 ++++++++++++++++--- impeller/golden_tests/golden_digest.h | 2 ++ .../golden_playground_test_mac.cc | 15 +++++++------ impeller/golden_tests/golden_tests.cc | 1 + .../bin/golden_tests_harvester.dart | 2 +- .../lib/golden_tests_harvester.dart | 5 ++++- .../lib/skia_gold_client.dart | 16 +++++++++----- 8 files changed, 44 insertions(+), 21 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 94022971632d3..f8db238978e00 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1243,9 +1243,6 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) { } TEST_P(AiksTest, TextRotated) { -#ifdef IMPELLER_GOLDEN_TESTS - GTEST_SKIP() << "Test has small differences on different mac hosts"; -#endif Canvas canvas; canvas.Transform(Matrix(0.5, -0.3, 0, -0.002, // 0, 1, 0, 0, // diff --git a/impeller/golden_tests/golden_digest.cc b/impeller/golden_tests/golden_digest.cc index 9181bfa6022ce..9dde1bdd9b14e 100644 --- a/impeller/golden_tests/golden_digest.cc +++ b/impeller/golden_tests/golden_digest.cc @@ -6,6 +6,9 @@ #include +static const double kMaxDiffPixelsPercent = 0.01; +static const int32_t kMaxColorDelta = 8; + namespace impeller { namespace testing { @@ -24,7 +27,8 @@ void GoldenDigest::AddImage(const std::string& test_name, const std::string& filename, int32_t width, int32_t height) { - entries_.push_back({test_name, filename, width, height}); + entries_.push_back({test_name, filename, width, height, kMaxDiffPixelsPercent, + kMaxColorDelta}); } bool GoldenDigest::Write(WorkingDirectory* working_directory) { @@ -46,8 +50,19 @@ bool GoldenDigest::Write(WorkingDirectory* working_directory) { << "\"testName\" : \"" << entry.test_name << "\", " << "\"filename\" : \"" << entry.filename << "\", " << "\"width\" : " << entry.width << ", " - << "\"height\" : " << entry.height << " " - << "}"; + << "\"height\" : " << entry.height << ", "; + + if (entry.max_diff_pixels_percent == + static_cast(entry.max_diff_pixels_percent)) { + fout << "\"maxDiffPixelsPercent\" : " << entry.max_diff_pixels_percent + << ".0, "; + } else { + fout << "\"maxDiffPixelsPercent\" : " << entry.max_diff_pixels_percent + << ", "; + } + + fout << "\"maxColorDelta\":" << entry.max_color_delta << " "; + fout << "}"; } fout << std::endl << "]" << std::endl; diff --git a/impeller/golden_tests/golden_digest.h b/impeller/golden_tests/golden_digest.h index 011607ee599ae..a018ac1b9c7d8 100644 --- a/impeller/golden_tests/golden_digest.h +++ b/impeller/golden_tests/golden_digest.h @@ -36,6 +36,8 @@ class GoldenDigest { std::string filename; int32_t width; int32_t height; + double max_diff_pixels_percent; + int32_t max_color_delta; }; static GoldenDigest* instance_; diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index a19c4c2122ff7..5ff897c5c9d5a 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -42,10 +42,9 @@ bool SaveScreenshot(std::unique_ptr screenshot) { } // namespace struct GoldenPlaygroundTest::GoldenPlaygroundTestImpl { - GoldenPlaygroundTestImpl() - : screenshoter_(new testing::MetalScreenshoter()) {} - std::unique_ptr screenshoter_; - ISize window_size_ = ISize{1024, 768}; + GoldenPlaygroundTestImpl() : screenshoter(new testing::MetalScreenshoter()) {} + std::unique_ptr screenshoter; + ISize window_size = ISize{1024, 768}; }; GoldenPlaygroundTest::GoldenPlaygroundTest() @@ -93,7 +92,7 @@ PlaygroundBackend GoldenPlaygroundTest::GetBackend() const { bool GoldenPlaygroundTest::OpenPlaygroundHere(const Picture& picture) { auto screenshot = - pimpl_->screenshoter_->MakeScreenshot(picture, pimpl_->window_size_); + pimpl_->screenshoter->MakeScreenshot(picture, pimpl_->window_size); return SaveScreenshot(std::move(screenshot)); } @@ -116,11 +115,11 @@ std::shared_ptr GoldenPlaygroundTest::CreateTextureForFixture( } std::shared_ptr GoldenPlaygroundTest::GetContext() const { - return pimpl_->screenshoter_->GetContext().GetContext(); + return pimpl_->screenshoter->GetContext().GetContext(); } Point GoldenPlaygroundTest::GetContentScale() const { - return pimpl_->screenshoter_->GetPlayground().GetContentScale(); + return pimpl_->screenshoter->GetPlayground().GetContentScale(); } Scalar GoldenPlaygroundTest::GetSecondsElapsed() const { @@ -128,7 +127,7 @@ Scalar GoldenPlaygroundTest::GetSecondsElapsed() const { } ISize GoldenPlaygroundTest::GetWindowSize() const { - return pimpl_->window_size_; + return pimpl_->window_size; } } // namespace impeller diff --git a/impeller/golden_tests/golden_tests.cc b/impeller/golden_tests/golden_tests.cc index d3a33deb59830..a54353fa0114a 100644 --- a/impeller/golden_tests/golden_tests.cc +++ b/impeller/golden_tests/golden_tests.cc @@ -43,6 +43,7 @@ bool SaveScreenshot(std::unique_ptr screenshot) { return screenshot->WriteToPNG( WorkingDirectory::Instance()->GetFilenamePath(filename)); } + } // namespace class GoldenTests : public ::testing::Test { diff --git a/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart index 7aa1f84d128be..8b5d2a2b55ba1 100644 --- a/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart +++ b/impeller/golden_tests_harvester/bin/golden_tests_harvester.dart @@ -23,7 +23,7 @@ class FakeSkiaGoldClient implements SkiaGoldClient { {double differentPixelsRate = 0.01, int pixelColorDelta = 0, required int screenshotSize}) async { - Logger.instance.log('addImg $testName ${goldenFile.path} $screenshotSize'); + Logger.instance.log('addImg testName:$testName goldenFile:${goldenFile.path} screenshotSize:$screenshotSize differentPixelsRate:$differentPixelsRate pixelColorDelta:$pixelColorDelta'); } @override diff --git a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart index 85fb43b60cafe..144bd1562dbb7 100644 --- a/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart +++ b/impeller/golden_tests_harvester/lib/golden_tests_harvester.dart @@ -32,9 +32,12 @@ Future harvest( final String filename = (map['filename'] as String?)!; final int width = (map['width'] as int?)!; final int height = (map['height'] as int?)!; + final double maxDiffPixelsPercent = (map['maxDiffPixelsPercent'] as double?)!; + final int maxColorDelta = (map['maxColorDelta'] as int?)!; final File goldenImage = File(p.join(workDirectory.path, filename)); final Future future = skiaGoldClient - .addImg(filename, goldenImage, screenshotSize: width * height) + .addImg(filename, goldenImage, + screenshotSize: width * height, differentPixelsRate: maxDiffPixelsPercent, pixelColorDelta: maxColorDelta) .catchError((dynamic err) { Logger.instance.log('skia gold comparison failed: $err'); throw Exception('Failed comparison: $filename'); diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 793560e9021e3..b7d23abdb37c9 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -178,15 +178,21 @@ class SkiaGoldClient { /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated. /// - /// [pixelColorDelta] defines maximum acceptable difference in RGB channels of each pixel, - /// such that: + /// [pixelColorDelta] defines maximum acceptable difference in RGB channels of + /// each pixel, such that: /// /// ``` - /// abs(r(image) - r(golden)) + abs(g(image) - g(golden)) + abs(b(image) - b(golden)) <= pixelDeltaThreshold + /// bool isSame(Color image, Color golden, int pixelDeltaThreshold) { + /// return abs(image.r - golden.r) + /// + abs(image.g - golden.g) + /// + abs(image.b - golden.b) <= pixelDeltaThreshold; + /// } /// ``` /// - /// [differentPixelsRate] is the fraction of accepted pixels to be wrong in the range [0.0, 1.0]. - /// Defaults to 0.01. A value of 0.01 means that 1% of the pixels are allowed to change. + /// [differentPixelsRate] is the fraction of pixels that can differ, as + /// determined by the [pixelColorDelta] parameter. It's in the range [0.0, + /// 1.0] and defaults to 0.01. A value of 0.01 means that 1% of the pixels are + /// allowed to be different. Future addImg( String testName, File goldenFile, {