From a56bcf18957c3e33770f20e02c778158620294b4 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 16 Oct 2020 18:17:36 -0700 Subject: [PATCH 1/7] Fix image gap due to svg element without position attribute --- .../lib/src/engine/html/color_filter.dart | 16 +-- lib/web_ui/lib/src/engine/util.dart | 90 +++++++++------ .../engine/color_filter_golden_test.dart | 106 ++++++++++++++---- 3 files changed, 150 insertions(+), 62 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/color_filter.dart b/lib/web_ui/lib/src/engine/html/color_filter.dart index 89c1960c27655..6ac54922eb04a 100644 --- a/lib/web_ui/lib/src/engine/html/color_filter.dart +++ b/lib/web_ui/lib/src/engine/html/color_filter.dart @@ -208,7 +208,9 @@ String? svgFilterFromBlendMode( case ui.BlendMode.dstOver: case ui.BlendMode.clear: case ui.BlendMode.srcOver: - assert(false, 'Invalid svg filter request for blend-mode ' + assert( + false, + 'Invalid svg filter request for blend-mode ' '$colorFilterBlendMode'); break; } @@ -232,7 +234,7 @@ int _filterIdCounter = 0; // A' = a1*R + a2*G + a3*B + a4*A + a5 String _srcInColorFilterToSvg(ui.Color? color) { _filterIdCounter += 1; - return '' + return '$kSvgResourceHeader' '' '' + return '$kSvgResourceHeader' '' '' @@ -261,7 +263,7 @@ String _srcOutColorFilterToSvg(ui.Color? color) { String _xorColorFilterToSvg(ui.Color? color) { _filterIdCounter += 1; - return '' + return '$kSvgResourceHeader' '' '' @@ -276,7 +278,7 @@ String _xorColorFilterToSvg(ui.Color? color) { String _compositeColorFilterToSvg( ui.Color? color, double k1, double k2, double k3, double k4) { _filterIdCounter += 1; - return '' + return '$kSvgResourceHeader' '' '' @@ -295,7 +297,7 @@ String _modulateColorFilterToSvg(ui.Color color) { final double r = color.red / 255.0; final double b = color.blue / 255.0; final double g = color.green / 255.0; - return '' + return '$kSvgResourceHeader' '' '' + return '$kSvgResourceHeader' '' '' diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 277f5484e2f07..9b092439f3822 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -111,8 +111,8 @@ TransformKind transformKindOf(Float32List matrix) { // If matrix contains scaling, rotation, z translation or // perspective transform, it is not considered simple. - final bool isSimple2dTransform = - m[15] == 1.0 && // start reading from the last element to eliminate range checks in subsequent reads. + final bool isSimple2dTransform = m[15] == + 1.0 && // start reading from the last element to eliminate range checks in subsequent reads. m[14] == 0.0 && // z translation is NOT simple // m[13] - y translation is simple // m[12] - x translation is simple @@ -126,8 +126,8 @@ TransformKind transformKindOf(Float32List matrix) { // m[4] - 2D rotation is simple m[3] == 0.0 && m[2] == 0.0; - // m[1] - 2D rotation is simple - // m[0] - scale x is simple + // m[1] - 2D rotation is simple + // m[0] - scale x is simple if (!isSimple2dTransform) { return TransformKind.complex; @@ -136,8 +136,7 @@ TransformKind transformKindOf(Float32List matrix) { // From this point on we're sure the transform is 2D, but we don't know if // it's identity or not. To check, we need to look at the remaining elements // that were not checked above. - final bool isIdentityTransform = - m[0] == 1.0 && + final bool isIdentityTransform = m[0] == 1.0 && m[1] == 0.0 && m[4] == 0.0 && m[5] == 1.0 && @@ -165,7 +164,7 @@ bool isIdentityFloat32ListTransform(Float32List matrix) { /// transform. Consider removing the CSS `transform` property from elements /// that apply identity transform. String float64ListToCssTransform2d(Float32List matrix) { - assert (transformKindOf(matrix) != TransformKind.complex); + assert(transformKindOf(matrix) != TransformKind.complex); return 'matrix(${matrix[0]},${matrix[1]},${matrix[4]},${matrix[5]},${matrix[12]},${matrix[13]})'; } @@ -279,10 +278,22 @@ void transformLTRB(Matrix4 transform, Float32List ltrb) { _tempPointMatrix.multiplyTranspose(transform); - ltrb[0] = math.min(math.min(math.min(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), _tempPointData[3]); - ltrb[1] = math.min(math.min(math.min(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), _tempPointData[7]); - ltrb[2] = math.max(math.max(math.max(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), _tempPointData[3]); - ltrb[3] = math.max(math.max(math.max(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), _tempPointData[7]); + ltrb[0] = math.min( + math.min( + math.min(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), + _tempPointData[3]); + ltrb[1] = math.min( + math.min( + math.min(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), + _tempPointData[7]); + ltrb[2] = math.max( + math.max( + math.max(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), + _tempPointData[3]); + ltrb[3] = math.max( + math.max( + math.max(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), + _tempPointData[7]); } /// Returns true if [rect] contains every point that is also contained by the @@ -300,6 +311,13 @@ bool rectContainsOther(ui.Rect rect, ui.Rect other) { /// Counter used for generating clip path id inside an svg tag. int _clipIdCounter = 0; +/// Used for clipping and filter svg resources. +/// +/// Position needs to be absolute since these svgs are sandwiched between +/// canvas elements and can cause layout shifts otherwise. +const String kSvgResourceHeader = ''; + /// Converts Path to svg element that contains a clip-path definition. /// /// Calling this method updates [_clipIdCounter]. The HTML id of the generated @@ -311,8 +329,7 @@ String _pathToSvgClipPath(ui.Path path, double scaleY = 1.0}) { _clipIdCounter += 1; final StringBuffer sb = StringBuffer(); - sb.write(''); + sb.write(kSvgResourceHeader); sb.write(''); final String clipId = 'svgClip$_clipIdCounter'; @@ -420,10 +437,11 @@ const Set _genericFontFamilies = { /// /// For iOS, default to -apple-system, where it should be available, otherwise /// default to Arial. BlinkMacSystemFont is used for Chrome on iOS. -final String _fallbackFontFamily = _isMacOrIOS ? - '-apple-system, BlinkMacSystemFont' : 'Arial'; +final String _fallbackFontFamily = + _isMacOrIOS ? '-apple-system, BlinkMacSystemFont' : 'Arial'; -bool get _isMacOrIOS => operatingSystem == OperatingSystem.iOs || +bool get _isMacOrIOS => + operatingSystem == OperatingSystem.iOs || operatingSystem == OperatingSystem.macOs; /// Create a font-family string appropriate for CSS. @@ -439,8 +457,10 @@ String? canonicalizeFontFamily(String? fontFamily) { // on sans-serif. // Map to San Francisco Text/Display fonts, use -apple-system, // BlinkMacSystemFont. - if (fontFamily == '.SF Pro Text' || fontFamily == '.SF Pro Display' || - fontFamily == '.SF UI Text' || fontFamily == '.SF UI Display') { + if (fontFamily == '.SF Pro Text' || + fontFamily == '.SF Pro Display' || + fontFamily == '.SF UI Text' || + fontFamily == '.SF UI Display') { return _fallbackFontFamily; } } @@ -474,7 +494,8 @@ void applyWebkitClipFix(html.Element? containerElement) { } } -final ByteData? _fontChangeMessage = JSONMessageCodec().encodeMessage({'type': 'fontsChange'}); +final ByteData? _fontChangeMessage = + JSONMessageCodec().encodeMessage({'type': 'fontsChange'}); // Font load callbacks will typically arrive in sequence, we want to prevent // sendFontChangeMessage of causing multiple synchronous rebuilds. @@ -482,19 +503,18 @@ final ByteData? _fontChangeMessage = JSONMessageCodec().encodeMessage( sendFontChangeMessage() async { - if (window._onPlatformMessage != null) - if (!_fontChangeScheduled) { - _fontChangeScheduled = true; - // Batch updates into next animationframe. - html.window.requestAnimationFrame((num _) { - _fontChangeScheduled = false; - window.invokeOnPlatformMessage( - 'flutter/system', - _fontChangeMessage, - (_) {}, - ); - }); - } + if (window._onPlatformMessage != null) if (!_fontChangeScheduled) { + _fontChangeScheduled = true; + // Batch updates into next animationframe. + html.window.requestAnimationFrame((num _) { + _fontChangeScheduled = false; + window.invokeOnPlatformMessage( + 'flutter/system', + _fontChangeMessage, + (_) {}, + ); + }); + } } // Stores matrix in a form that allows zero allocation transforms. @@ -529,14 +549,16 @@ bool isUnsoundNull(dynamic object) { } bool _offsetIsValid(ui.Offset offset) { - assert(offset != null, 'Offset argument was null.'); // ignore: unnecessary_null_comparison + assert(offset != null, + 'Offset argument was null.'); // ignore: unnecessary_null_comparison assert(!offset.dx.isNaN && !offset.dy.isNaN, 'Offset argument contained a NaN value.'); return true; } bool _matrix4IsValid(Float32List matrix4) { - assert(matrix4 != null, 'Matrix4 argument was null.'); // ignore: unnecessary_null_comparison + assert(matrix4 != null, + 'Matrix4 argument was null.'); // ignore: unnecessary_null_comparison assert(matrix4.length == 16, 'Matrix4 must have 16 entries.'); return true; } diff --git a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart index 97ca81b29426f..db751d1c78589 100644 --- a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart @@ -2,8 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.6 +// @dart = 2.10 import 'dart:html' as html; +import 'dart:js_util' as js_util; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; @@ -35,55 +36,118 @@ void testMain() async { final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); final Picture backgroundPicture = _drawBackground(); builder.addPicture(Offset.zero, backgroundPicture); - builder.pushColorFilter(EngineColorFilter.mode(Color(0xF0000080), - BlendMode.color)); + builder.pushColorFilter( + EngineColorFilter.mode(Color(0xF0000080), BlendMode.color)); final Picture circles1 = _drawTestPictureWithCircles(30, 30); builder.addPicture(Offset.zero, circles1); builder.pop(); - html.document.body.append(builder - .build() - .webOnlyRootElement); + html.document.body!.append(builder.build().webOnlyRootElement!); await matchGoldenFile('color_filter_blendMode_color.png', region: region); }); + + /// Regression test for https://github.com/flutter/flutter/issues/59451. + /// + /// Picture with overlay blend inside a physical shape. Should show image + /// at 0,0. In the filed issue it was leaving a gap on top. + test('Should render image with color filter without gap', () async { + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + final Path path = Path(); + path.addRRect(RRect.fromRectAndRadius( + Rect.fromLTRB(0, 0, 400, 400), Radius.circular(2))); + PhysicalShapeEngineLayer oldLayer = builder.pushPhysicalShape( + path: path, color: Color(0xFFFFFFFF), elevation: 0); + final Picture circles1 = _drawTestPictureWithImage( + ColorFilter.mode(Color(0x3C4043), BlendMode.overlay)); + builder.addPicture(Offset(10, 0), circles1); + builder.pop(); + builder.build(); + + final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder(); + builder2.pushPhysicalShape( + path: path, color: Color(0xFFFFFFFF), elevation: 0, oldLayer: oldLayer); + builder2.addPicture(Offset(10, 0), circles1); + builder2.pop(); + + html.document.body!.append(builder2.build().webOnlyRootElement!); + + await matchGoldenFile('color_filter_blendMode_overlay.png', + region: region, write: true); + }); } Picture _drawTestPictureWithCircles(double offsetX, double offsetY) { - final EnginePictureRecorder recorder = PictureRecorder(); + final EnginePictureRecorder recorder = + PictureRecorder() as EnginePictureRecorder; final RecordingCanvas canvas = - recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); - canvas.drawCircle( - Offset(offsetX + 10, offsetY + 10), 10, Paint()..style = PaintingStyle.fill); + recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); + canvas.drawCircle(Offset(offsetX + 10, offsetY + 10), 10, + (Paint()..style = PaintingStyle.fill) as SurfacePaint); canvas.drawCircle( Offset(offsetX + 60, offsetY + 10), 10, - Paint() + (Paint() ..style = PaintingStyle.fill - ..color = const Color.fromRGBO(255, 0, 0, 1)); + ..color = const Color.fromRGBO(255, 0, 0, 1)) as SurfacePaint); canvas.drawCircle( Offset(offsetX + 10, offsetY + 60), 10, - Paint() + (Paint() ..style = PaintingStyle.fill - ..color = const Color.fromRGBO(0, 255, 0, 1)); + ..color = const Color.fromRGBO(0, 255, 0, 1)) as SurfacePaint); canvas.drawCircle( Offset(offsetX + 60, offsetY + 60), 10, - Paint() + (Paint() ..style = PaintingStyle.fill - ..color = const Color.fromRGBO(0, 0, 255, 1)); + ..color = const Color.fromRGBO(0, 0, 255, 1)) as SurfacePaint); + return recorder.endRecording(); +} + +Picture _drawTestPictureWithImage(ColorFilter filter) { + final EnginePictureRecorder recorder = + PictureRecorder() as EnginePictureRecorder; + final RecordingCanvas canvas = + recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); + Image testImage = createTestImage(); + canvas.drawImageRect( + testImage, + Rect.fromLTWH(0, 0, 200, 150), + Rect.fromLTWH(0, 0, 300, 300), + (Paint() + ..style = PaintingStyle.fill + ..colorFilter = filter + ..color = const Color.fromRGBO(0, 0, 255, 1)) as SurfacePaint); return recorder.endRecording(); } Picture _drawBackground() { - final EnginePictureRecorder recorder = PictureRecorder(); + final EnginePictureRecorder recorder = + PictureRecorder() as EnginePictureRecorder; final RecordingCanvas canvas = - recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); + recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); canvas.drawRect( Rect.fromLTWH(8, 8, 400.0 - 16, 400.0 - 16), - Paint() + (Paint() ..style = PaintingStyle.fill - ..color = Color(0xFFE0FFE0) - ); + ..color = Color(0xFFE0FFE0)) as SurfacePaint); return recorder.endRecording(); } + +HtmlImage createTestImage({int width = 200, int height = 150}) { + html.CanvasElement canvas = + new html.CanvasElement(width: width, height: height); + html.CanvasRenderingContext2D ctx = canvas.context2D; + ctx.fillStyle = '#E04040'; + ctx.fillRect(0, 0, width / 3, height); + ctx.fill(); + ctx.fillStyle = '#40E080'; + ctx.fillRect(width / 3, 0, width / 3, height); + ctx.fill(); + ctx.fillStyle = '#2040E0'; + ctx.fillRect(2 * width / 3, 0, width / 3, height); + ctx.fill(); + html.ImageElement imageElement = html.ImageElement(); + imageElement.src = js_util.callMethod(canvas, 'toDataURL', []); + return HtmlImage(imageElement, width, height); +} From d8a5139a6a944ef70dc49c4ee70cc71a961a6de6 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Sat, 17 Oct 2020 00:14:26 -0700 Subject: [PATCH 2/7] remove screenshot write --- .../test/golden_tests/engine/color_filter_golden_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart index db751d1c78589..333d42c302539 100644 --- a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart @@ -72,7 +72,7 @@ void testMain() async { html.document.body!.append(builder2.build().webOnlyRootElement!); await matchGoldenFile('color_filter_blendMode_overlay.png', - region: region, write: true); + region: region); }); } From 00752be832e9e76cf2656ce3b542950722ca384f Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 19 Oct 2020 09:30:59 -0700 Subject: [PATCH 3/7] update golden lock --- lib/web_ui/dev/goldens_lock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 1f1b59c91e98d..f558ffe268551 100644 --- a/lib/web_ui/dev/goldens_lock.yaml +++ b/lib/web_ui/dev/goldens_lock.yaml @@ -1,2 +1,2 @@ repository: https://github.com/flutter/goldens.git -revision: 1556280d6f1d70fac9ddff9b38639757e105b4b0 +revision: 67f22ef933be27ba2be8b27df1b71b2c69eb86e5 From ad7f334e82bc273c7593f8a50c547103d6b36e8e Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 19 Oct 2020 11:18:31 -0700 Subject: [PATCH 4/7] address reviewer comments on style --- lib/web_ui/lib/src/engine/util.dart | 2 +- .../test/golden_tests/engine/color_filter_golden_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 9b092439f3822..8f47d30405add 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -503,7 +503,7 @@ final ByteData? _fontChangeMessage = bool _fontChangeScheduled = false; FutureOr sendFontChangeMessage() async { - if (window._onPlatformMessage != null) if (!_fontChangeScheduled) { + if (window._onPlatformMessage != null && !_fontChangeScheduled) { _fontChangeScheduled = true; // Batch updates into next animationframe. html.window.requestAnimationFrame((num _) { diff --git a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart index 333d42c302539..ae30bc4173bda 100644 --- a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart @@ -109,7 +109,7 @@ Picture _drawTestPictureWithImage(ColorFilter filter) { PictureRecorder() as EnginePictureRecorder; final RecordingCanvas canvas = recorder.beginRecording(const Rect.fromLTRB(0, 0, 400, 400)); - Image testImage = createTestImage(); + final Image testImage = createTestImage(); canvas.drawImageRect( testImage, Rect.fromLTWH(0, 0, 200, 150), From 8ee91f55ae3cf6f15ce8facc727b04847cf06835 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 19 Oct 2020 11:49:40 -0700 Subject: [PATCH 5/7] remove unnecessary null comparisons --- lib/web_ui/lib/src/engine/util.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 8f47d30405add..2f94a7c196bba 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -549,16 +549,12 @@ bool isUnsoundNull(dynamic object) { } bool _offsetIsValid(ui.Offset offset) { - assert(offset != null, - 'Offset argument was null.'); // ignore: unnecessary_null_comparison assert(!offset.dx.isNaN && !offset.dy.isNaN, 'Offset argument contained a NaN value.'); return true; } bool _matrix4IsValid(Float32List matrix4) { - assert(matrix4 != null, - 'Matrix4 argument was null.'); // ignore: unnecessary_null_comparison assert(matrix4.length == 16, 'Matrix4 must have 16 entries.'); return true; } From d6c695317d748655dcc5d65f2b6626aed33219fd Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 23 Oct 2020 12:43:12 -0700 Subject: [PATCH 6/7] merge and update test diff --- .../test/golden_tests/engine/color_filter_golden_test.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart index ae30bc4173bda..f7d24350a20c9 100644 --- a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart @@ -43,7 +43,10 @@ void testMain() async { builder.pop(); html.document.body!.append(builder.build().webOnlyRootElement!); - await matchGoldenFile('color_filter_blendMode_color.png', region: region); + // TODO: update golden for this test after canvas sandwich detection is + // added to RecordingCanvas. + await matchGoldenFile('color_filter_blendMode_color.png', region: region, + maxDiffRatePercent: 12.0); }); /// Regression test for https://github.com/flutter/flutter/issues/59451. From e28bb0decf12996065354e5a4d8f22b5ecbb67c2 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 23 Oct 2020 16:07:14 -0700 Subject: [PATCH 7/7] update golden diff --- .../test/golden_tests/engine/color_filter_golden_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart index f7d24350a20c9..c39306528429a 100644 --- a/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/color_filter_golden_test.dart @@ -75,7 +75,8 @@ void testMain() async { html.document.body!.append(builder2.build().webOnlyRootElement!); await matchGoldenFile('color_filter_blendMode_overlay.png', - region: region); + region: region, + maxDiffRatePercent: 12.0); }); }