From 03258048e9194cd6179d92fb720aea116bdfff60 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 19 Nov 2021 13:24:41 -0500 Subject: [PATCH 1/6] [web] Use fuzzy matching in Gold --- web_sdk/web_test_utils/lib/image_compare.dart | 12 +++-- web_sdk/web_test_utils/lib/skia_client.dart | 47 ++++++++++++++++++- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/web_sdk/web_test_utils/lib/image_compare.dart b/web_sdk/web_test_utils/lib/image_compare.dart index 7952930c95658..0520e6f14c9b4 100644 --- a/web_sdk/web_test_utils/lib/image_compare.dart +++ b/web_sdk/web_test_utils/lib/image_compare.dart @@ -172,11 +172,13 @@ Future _uploadToSkiaGold( final File goldenFile = File(p.join(environment.webUiSkiaGoldDirectory.path, filename)); await goldenFile.writeAsBytes(encodePng(screenshot), flush: true); + final int screenshotSize = screenshot.width * screenshot.height; + if (_isPreSubmit) { - return _uploadInPreSubmit(skiaClient, filename, goldenFile); + return _uploadInPreSubmit(skiaClient, filename, goldenFile, screenshotSize); } if (_isPostSubmit) { - return _uploadInPostSubmit(skiaClient, filename, goldenFile); + return _uploadInPostSubmit(skiaClient, filename, goldenFile, screenshotSize); } } @@ -184,16 +186,18 @@ Future _uploadInPreSubmit( SkiaGoldClient skiaClient, String filename, File goldenFile, + int screenshotSize, ) { assert(_isPreSubmit); - return skiaClient.tryjobAdd(filename, goldenFile); + return skiaClient.tryjobAdd(filename, goldenFile, screenshotSize); } Future _uploadInPostSubmit( SkiaGoldClient skiaClient, String filename, File goldenFile, + int screenshotSize, ) { assert(_isPostSubmit); - return skiaClient.imgtestAdd(filename, goldenFile); + return skiaClient.imgtestAdd(filename, goldenFile, screenshotSize); } diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index fae028364bcd9..611a2883368d1 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -16,6 +16,13 @@ const String _kGoldctlKey = 'GOLDCTL'; const String _skiaGoldHost = 'https://flutter-engine-gold.skia.org'; const String _instance = 'flutter-engine'; +/// The percentage of accepted pixels to be wrong. +/// +/// This should be a double between 0.0 and 1.0. A value of 0.0 means we don't +/// accept any pixel to be different. A value of 1.0 means we accept 100% of +/// pixels to be different. +const double kMaxDifferentPixelsRate = 0.1; + /// A client for uploading image tests and making baseline requests to the /// Flutter Gold Dashboard. class SkiaGoldClient { @@ -162,7 +169,7 @@ class SkiaGoldClient { /// /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated. - Future imgtestAdd(String testName, File goldenFile) async { + Future imgtestAdd(String testName, File goldenFile, int screenshotSize) async { await _imgtestInit(); final List imgtestCommand = [ @@ -171,6 +178,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, + ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -250,7 +258,7 @@ class SkiaGoldClient { /// /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated. - Future tryjobAdd(String testName, File goldenFile) async { + Future tryjobAdd(String testName, File goldenFile, int screenshotSize) async { await _tryjobInit(); final List imgtestCommand = [ @@ -259,6 +267,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, + ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -279,6 +288,40 @@ class SkiaGoldClient { } } + List _getMatchingArguments(String testName, int screenshotSize) { + // The algorithm to be used when matching images. The available options are: + // - "fuzzy": Allows for customizing the thresholds of pixel differences. + // - "sobel": Same as "fuzzy" but performs edge detection before performing + // a fuzzy match. + const String algorithm = 'fuzzy'; + + // The number of pixels in this image that are allowed to differ from the + // baseline. It's okay for this to be a slightly high number like 10% of the + // image size because those wrong pixels are constrained by + // `pixelDeltaThreshold` below. + final String maxDifferentPixels = '${screenshotSize * kMaxDifferentPixelsRate}'; + + // The maximum acceptable difference in RGB channels of each pixel. + // + // ``` + // abs(r(image) - r(golden)) + abs(g(image) - g(golden)) + abs(b(image) - b(golden)) <= pixelDeltaThreshold + // ``` + final String pixelDeltaThreshold; + if (testName.startsWith('canvaskit_')) { + pixelDeltaThreshold = '21'; + } else if (browserName == 'ios-safari') { + pixelDeltaThreshold = '15'; + } else { + pixelDeltaThreshold = '3'; + } + + return [ + '--add-test-optional-key', 'image_matching_algorithm:$algorithm', + '--add-test-optional-key', 'fuzzy_max_different_pixels:$maxDifferentPixels', + '--add-test-optional-key', 'fuzzy_pixel_delta_threshold:$pixelDeltaThreshold', + ]; + } + /// Returns the latest positive digest for the given test known to Skia Gold /// at head. Future getExpectationForTest(String testName) async { From 5cfdf5f299d8839ab0c231a3482492548f8e3a78 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 19 Nov 2021 15:13:32 -0500 Subject: [PATCH 2/6] fix double --- web_sdk/web_test_utils/lib/skia_client.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index 611a2883368d1..88d21382ceeb1 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -299,7 +299,7 @@ class SkiaGoldClient { // baseline. It's okay for this to be a slightly high number like 10% of the // image size because those wrong pixels are constrained by // `pixelDeltaThreshold` below. - final String maxDifferentPixels = '${screenshotSize * kMaxDifferentPixelsRate}'; + final int maxDifferentPixels = (screenshotSize * kMaxDifferentPixelsRate).toInt(); // The maximum acceptable difference in RGB channels of each pixel. // From c72e36026e6c4434f6eefa132f98d49f03f4fcc0 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 22 Nov 2021 11:37:34 -0500 Subject: [PATCH 3/6] re-order trace keys --- web_sdk/web_test_utils/lib/skia_client.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index 88d21382ceeb1..16d502480a69f 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -399,9 +399,9 @@ class SkiaGoldClient { /// browser the image was rendered on. Map _getKeys() { return { + 'Browser': browserName, 'CI': 'luci', 'Platform': Platform.operatingSystem, - 'Browser': browserName, }; } From 1722dc9291153115f71ffc923909337586dc1985 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 23 Nov 2021 15:30:41 -0500 Subject: [PATCH 4/6] don't use fuzzy algorithm --- web_sdk/web_test_utils/lib/skia_client.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index 16d502480a69f..6e3a62c443fc8 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -178,7 +178,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - ..._getMatchingArguments(testName, screenshotSize), + // ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -267,7 +267,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - ..._getMatchingArguments(testName, screenshotSize), + // ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); From 3b22fc37d13f72069c22cebfdc5f9398f134d40c Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 24 Nov 2021 09:35:29 -0500 Subject: [PATCH 5/6] use fuzzy algorithm --- web_sdk/web_test_utils/lib/skia_client.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index 6e3a62c443fc8..16d502480a69f 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -178,7 +178,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - // ..._getMatchingArguments(testName, screenshotSize), + ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -267,7 +267,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - // ..._getMatchingArguments(testName, screenshotSize), + ..._getMatchingArguments(testName, screenshotSize), ]; final ProcessResult result = await process.run(imgtestCommand); From 1d4b3346bc46b160b3d0411f1f4390ae05515ade Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 29 Nov 2021 13:51:39 -0500 Subject: [PATCH 6/6] use the canvaskit boolean instead of test name --- lib/web_ui/dev/test_platform.dart | 16 +++++++----- .../web_engine_tester/lib/golden_tester.dart | 6 ++--- web_sdk/web_test_utils/lib/image_compare.dart | 14 ++++++---- web_sdk/web_test_utils/lib/skia_client.dart | 26 ++++++++++++++----- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/lib/web_ui/dev/test_platform.dart b/lib/web_ui/dev/test_platform.dart index d9fd05ccd6efd..cc2dd887e1af3 100644 --- a/lib/web_ui/dev/test_platform.dart +++ b/lib/web_ui/dev/test_platform.dart @@ -339,17 +339,20 @@ class BrowserPlatform extends PlatformPlugin { requestData['region'] as Map; final PixelComparison pixelComparison = PixelComparison.values.firstWhere( (PixelComparison value) => value.toString() == requestData['pixelComparison']); + final bool isCanvaskitTest = requestData['isCanvaskitTest'] as bool; final String result = await _diffScreenshot( - filename, write, maxDiffRate, region, pixelComparison); + filename, write, maxDiffRate, region, pixelComparison, isCanvaskitTest); return shelf.Response.ok(json.encode(result)); } Future _diffScreenshot( - String filename, - bool write, - double maxDiffRateFailure, - Map region, - PixelComparison pixelComparison) async { + String filename, + bool write, + double maxDiffRateFailure, + Map region, + PixelComparison pixelComparison, + bool isCanvaskitTest, + ) async { if (doUpdateScreenshotGoldens) { write = true; } @@ -387,6 +390,7 @@ class BrowserPlatform extends PlatformPlugin { pixelComparison, maxDiffRateFailure, skiaClient, + isCanvaskitTest: isCanvaskitTest, goldensDirectory: goldensDirectory, filenameSuffix: _screenshotManager!.filenameSuffix, write: write, diff --git a/web_sdk/web_engine_tester/lib/golden_tester.dart b/web_sdk/web_engine_tester/lib/golden_tester.dart index fcd22fedf8136..db7b0dc3188a2 100644 --- a/web_sdk/web_engine_tester/lib/golden_tester.dart +++ b/web_sdk/web_engine_tester/lib/golden_tester.dart @@ -6,10 +6,9 @@ import 'dart:async'; import 'dart:convert'; import 'dart:html' as html; -import 'package:ui/src/engine.dart'; -import 'package:ui/ui.dart'; - import 'package:test/test.dart'; +import 'package:ui/src/engine.dart' show operatingSystem, OperatingSystem, useCanvasKit; +import 'package:ui/ui.dart'; Future _callScreenshotServer(dynamic requestData) async { final html.HttpRequest request = await html.HttpRequest.request( @@ -63,6 +62,7 @@ Future matchGoldenFile(String filename, 'height': region.height }, 'pixelComparison': pixelComparison.toString(), + 'isCanvaskitTest': useCanvasKit, }; // Chrome on macOS renders slightly differently from Linux, so allow it an diff --git a/web_sdk/web_test_utils/lib/image_compare.dart b/web_sdk/web_test_utils/lib/image_compare.dart index 0520e6f14c9b4..4a3bef917f034 100644 --- a/web_sdk/web_test_utils/lib/image_compare.dart +++ b/web_sdk/web_test_utils/lib/image_compare.dart @@ -32,6 +32,7 @@ Future compareImage( PixelComparison pixelComparison, double maxDiffRateFailure, SkiaGoldClient? skiaClient, { + required bool isCanvaskitTest, // TODO(mdebbar): Remove these args with goldens repo. String goldensDirectory = '', String filenameSuffix = '', @@ -43,7 +44,7 @@ Future compareImage( // comparison. // TODO(mdebbar): Use Skia Gold for comparison, not only for uploading. - await _uploadToSkiaGold(skiaClient, screenshot, filename); + await _uploadToSkiaGold(skiaClient, screenshot, filename, isCanvaskitTest); } filename = filename.replaceAll('.png', '$filenameSuffix.png'); @@ -163,6 +164,7 @@ Future _uploadToSkiaGold( SkiaGoldClient skiaClient, Image screenshot, String filename, + bool isCanvaskitTest, ) async { // Can't upload to Gold Skia unless running in LUCI. assert(_isLuci); @@ -175,10 +177,10 @@ Future _uploadToSkiaGold( final int screenshotSize = screenshot.width * screenshot.height; if (_isPreSubmit) { - return _uploadInPreSubmit(skiaClient, filename, goldenFile, screenshotSize); + return _uploadInPreSubmit(skiaClient, filename, goldenFile, screenshotSize, isCanvaskitTest); } if (_isPostSubmit) { - return _uploadInPostSubmit(skiaClient, filename, goldenFile, screenshotSize); + return _uploadInPostSubmit(skiaClient, filename, goldenFile, screenshotSize, isCanvaskitTest); } } @@ -187,9 +189,10 @@ Future _uploadInPreSubmit( String filename, File goldenFile, int screenshotSize, + bool isCanvaskitTest, ) { assert(_isPreSubmit); - return skiaClient.tryjobAdd(filename, goldenFile, screenshotSize); + return skiaClient.tryjobAdd(filename, goldenFile, screenshotSize, isCanvaskitTest); } Future _uploadInPostSubmit( @@ -197,7 +200,8 @@ Future _uploadInPostSubmit( String filename, File goldenFile, int screenshotSize, + bool isCanvaskitTest, ) { assert(_isPostSubmit); - return skiaClient.imgtestAdd(filename, goldenFile, screenshotSize); + return skiaClient.imgtestAdd(filename, goldenFile, screenshotSize, isCanvaskitTest); } diff --git a/web_sdk/web_test_utils/lib/skia_client.dart b/web_sdk/web_test_utils/lib/skia_client.dart index 16d502480a69f..13aeb7c16acda 100644 --- a/web_sdk/web_test_utils/lib/skia_client.dart +++ b/web_sdk/web_test_utils/lib/skia_client.dart @@ -169,7 +169,12 @@ class SkiaGoldClient { /// /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated. - Future imgtestAdd(String testName, File goldenFile, int screenshotSize) async { + Future imgtestAdd( + String testName, + File goldenFile, + int screenshotSize, + bool isCanvaskitTest, + ) async { await _imgtestInit(); final List imgtestCommand = [ @@ -178,7 +183,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - ..._getMatchingArguments(testName, screenshotSize), + ..._getMatchingArguments(testName, screenshotSize, isCanvaskitTest), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -258,7 +263,12 @@ class SkiaGoldClient { /// /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated. - Future tryjobAdd(String testName, File goldenFile, int screenshotSize) async { + Future tryjobAdd( + String testName, + File goldenFile, + int screenshotSize, + bool isCanvaskitTest, + ) async { await _tryjobInit(); final List imgtestCommand = [ @@ -267,7 +277,7 @@ class SkiaGoldClient { '--work-dir', _tempPath, '--test-name', cleanTestName(testName), '--png-file', goldenFile.path, - ..._getMatchingArguments(testName, screenshotSize), + ..._getMatchingArguments(testName, screenshotSize, isCanvaskitTest), ]; final ProcessResult result = await process.run(imgtestCommand); @@ -288,7 +298,11 @@ class SkiaGoldClient { } } - List _getMatchingArguments(String testName, int screenshotSize) { + List _getMatchingArguments( + String testName, + int screenshotSize, + bool isCanvaskitTest, + ) { // The algorithm to be used when matching images. The available options are: // - "fuzzy": Allows for customizing the thresholds of pixel differences. // - "sobel": Same as "fuzzy" but performs edge detection before performing @@ -307,7 +321,7 @@ class SkiaGoldClient { // abs(r(image) - r(golden)) + abs(g(image) - g(golden)) + abs(b(image) - b(golden)) <= pixelDeltaThreshold // ``` final String pixelDeltaThreshold; - if (testName.startsWith('canvaskit_')) { + if (isCanvaskitTest) { pixelDeltaThreshold = '21'; } else if (browserName == 'ios-safari') { pixelDeltaThreshold = '15';