From 98c1aeaa3b1b4d6721f35cade23a8032f16ea96c Mon Sep 17 00:00:00 2001 From: Ferhat Date: Fri, 17 Jan 2020 10:41:02 -0800 Subject: [PATCH] Clipping path fix for browsers that don't use correct units when applying clip-path css using svg (#15712) --- lib/web_ui/dev/goldens_lock.yaml | 2 +- lib/web_ui/lib/src/engine/bitmap_canvas.dart | 6 +- lib/web_ui/lib/src/engine/surface/clip.dart | 55 ++++++--- lib/web_ui/lib/src/engine/util.dart | 20 ++-- .../engine/canvas_clip_path_test.dart | 105 ++++++++++++++++++ 5 files changed, 155 insertions(+), 33 deletions(-) create mode 100644 lib/web_ui/test/golden_tests/engine/canvas_clip_path_test.dart diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 547d0f7ad..e82bd5456 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: b20fee88d8917af269e8073910c31aa373f8a188 +revision: 30ef2668489dab3191a5df251330aedb9b0c239a diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index df6381474..4af753999 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -722,13 +722,9 @@ List _clipContent(List<_SaveClipEntry> clipStack, setElementTransform(curElement, newClipTransform.storage); } else if (entry.path != null) { curElement.style.transform = matrix4ToCssTransform(newClipTransform); - final String svgClipPath = _pathToSvgClipPath(entry.path); + String svgClipPath = createSvgClipDef(curElement, entry.path); final html.Element clipElement = html.Element.html(svgClipPath, treeSanitizer: _NullTreeSanitizer()); - domRenderer.setElementStyle( - curElement, 'clip-path', 'url(#svgClip$_clipIdCounter)'); - domRenderer.setElementStyle( - curElement, '-webkit-clip-path', 'url(#svgClip$_clipIdCounter)'); clipDefs.add(clipElement); } // Reverse the transform of the clipping element so children can use diff --git a/lib/web_ui/lib/src/engine/surface/clip.dart b/lib/web_ui/lib/src/engine/surface/clip.dart index 8cee72685..10369c43e 100644 --- a/lib/web_ui/lib/src/engine/surface/clip.dart +++ b/lib/web_ui/lib/src/engine/surface/clip.dart @@ -88,8 +88,8 @@ class PersistedClipRect extends PersistedContainerSurface // the shift in the coordinate system introduced by the translation of the // rootElement. Clipping in Flutter has no effect on the coordinate system. childContainer.style - ..left = '${-rect.left}px' - ..top = '${-rect.top}px'; + ..left = '${-rect.left}px' + ..top = '${-rect.top}px'; } @override @@ -141,8 +141,8 @@ class PersistedClipRRect extends PersistedContainerSurface // the shift in the coordinate system introduced by the translation of the // rootElement. Clipping in Flutter has no effect on the coordinate system. childContainer.style - ..left = '${-rrect.left}px' - ..top = '${-rrect.top}px'; + ..left = '${-rrect.left}px' + ..top = '${-rrect.top}px'; } @override @@ -278,9 +278,12 @@ class PersistedPhysicalShape extends PersistedContainerSurface } } - final ui.Rect bounds = path.getBounds(); - final String svgClipPath = - _pathToSvgClipPath(path, offsetX: -bounds.left, offsetY: -bounds.top); + final ui.Rect pathBounds = path.getBounds(); + final String svgClipPath = _pathToSvgClipPath(path, + offsetX: -pathBounds.left, + offsetY: -pathBounds.top, + scaleX: 1.0 / pathBounds.width, + scaleY: 1.0 / pathBounds.height); assert(_clipElement == null); _clipElement = html.Element.html(svgClipPath, treeSanitizer: _NullTreeSanitizer()); @@ -292,14 +295,14 @@ class PersistedPhysicalShape extends PersistedContainerSurface final html.CssStyleDeclaration rootElementStyle = rootElement.style; rootElementStyle ..overflow = '' - ..left = '${bounds.left}px' - ..top = '${bounds.top}px' - ..width = '${bounds.width}px' - ..height = '${bounds.height}px' + ..left = '${pathBounds.left}px' + ..top = '${pathBounds.top}px' + ..width = '${pathBounds.width}px' + ..height = '${pathBounds.height}px' ..borderRadius = ''; childContainer.style - ..left = '-${bounds.left}px' - ..top = '-${bounds.top}px'; + ..left = '-${pathBounds.left}px' + ..top = '-${pathBounds.top}px'; } @override @@ -364,15 +367,11 @@ class PersistedClipPath extends PersistedContainerSurface } return; } - final String svgClipPath = _pathToSvgClipPath(clipPath); _clipElement?.remove(); + final String svgClipPath = createSvgClipDef(childContainer, clipPath); _clipElement = html.Element.html(svgClipPath, treeSanitizer: _NullTreeSanitizer()); domRenderer.append(childContainer, _clipElement); - domRenderer.setElementStyle( - childContainer, 'clip-path', 'url(#svgClip$_clipIdCounter)'); - domRenderer.setElementStyle( - childContainer, '-webkit-clip-path', 'url(#svgClip$_clipIdCounter)'); } @override @@ -395,3 +394,23 @@ class PersistedClipPath extends PersistedContainerSurface super.discard(); } } + +/// Creates an svg clipPath and applies it to [element]. +String createSvgClipDef(html.HtmlElement element, ui.Path clipPath) { + final ui.Rect pathBounds = clipPath.getBounds(); + final String svgClipPath = _pathToSvgClipPath(clipPath, + scaleX: 1.0 / pathBounds.right, scaleY: 1.0 / pathBounds.bottom); + domRenderer.setElementStyle( + element, 'clip-path', 'url(#svgClip$_clipIdCounter)'); + domRenderer.setElementStyle( + element, '-webkit-clip-path', 'url(#svgClip$_clipIdCounter)'); + // We need to set width and height for the clipElement to cover the + // bounds of the path since browsers such as Safari and Edge + // seem to incorrectly intersect the element bounding rect with + // the clip path. Chrome and Firefox don't perform intersect instead they + // use the path itself as source of truth. + element.style + ..width = '${pathBounds.right}px' + ..height = '${pathBounds.bottom}px'; + return svgClipPath; +} diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 2eb5a6293..ce2da7439 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -74,7 +74,8 @@ void setElementTransform(html.Element element, Float64List matrix4) { // On low device-pixel ratio screens using CSS "transform" causes text blurriness // at least on Blink browsers. We therefore prefer using CSS "left" and "top" instead. - final bool isHighDevicePixelRatioScreen = EngineWindow.browserDevicePixelRatio > 1.0; + final bool isHighDevicePixelRatioScreen = + EngineWindow.browserDevicePixelRatio > 1.0; if (transformKind == TransformKind.complex || isHighDevicePixelRatioScreen) { final String cssTransform = float64ListToCssTransform3d(matrix4); @@ -122,8 +123,7 @@ TransformKind transformKindOf(Float64List matrix) { // If matrix contains scaling, rotation, z translation or // perspective transform, it is not considered simple. - final bool isSimpleTransform = - m[0] == 1.0 && + final bool isSimpleTransform = m[0] == 1.0 && m[1] == 0.0 && m[2] == 0.0 && m[3] == 0.0 && @@ -137,7 +137,7 @@ TransformKind transformKindOf(Float64List matrix) { m[11] == 0.0 && // m[12] - x translation is simple // m[13] - y translation is simple - m[14] == 0.0 && // z translation is NOT simple + m[14] == 0.0 && // z translation is NOT simple m[15] == 1.0; if (!isSimpleTransform) { @@ -312,18 +312,20 @@ int _clipIdCounter = 0; /// Calling this method updates [_clipIdCounter]. The HTML id of the generated /// clip is set to "svgClip${_clipIdCounter}", e.g. "svgClip123". String _pathToSvgClipPath(ui.Path path, - {double offsetX = 0, double offsetY = 0}) { + {double offsetX = 0, + double offsetY = 0, + double scaleX = 1.0, + double scaleY = 1.0}) { _clipIdCounter += 1; - final ui.Rect bounds = path.getBounds(); final StringBuffer sb = StringBuffer(); - sb.write(''); sb.write(''); final String clipId = 'svgClip$_clipIdCounter'; - sb.write(''); + sb.write(''); - sb.write(' _checkScreenshot(engine.RecordingCanvas rc, String fileName, + {Rect region = const Rect.fromLTWH(0, 0, 500, 500)}) async { + final engine.EngineCanvas engineCanvas = engine.BitmapCanvas(screenRect); + + rc.apply(engineCanvas); + + // Wrap in so that our CSS selectors kick in. + final html.Element sceneElement = html.Element.tag('flt-scene'); + try { + sceneElement.append(engineCanvas.rootElement); + html.document.body.append(sceneElement); + await matchGoldenFile('$fileName.png', region: region, maxDiffRate: 0.2); + } finally { + // The page is reused across tests, so remove the element after taking the + // Scuba screenshot. + sceneElement.remove(); + } + } + + setUp(() async { + debugEmulateFlutterTesterEnvironment = true; + await webOnlyInitializePlatform(); + webOnlyFontCollection.debugRegisterTestFonts(); + await webOnlyFontCollection.ensureFontsLoaded(); + }); + + // Regression test for https://github.com/flutter/flutter/issues/48683 + // Should clip image with oval. + test('Clips image with oval clip path', () async { + final engine.RecordingCanvas rc = + engine.RecordingCanvas(const Rect.fromLTRB(0, 0, 400, 300)); + rc.save(); + Image testImage = createTestImage(); + double testWidth = testImage.width.toDouble(); + double testHeight = testImage.height.toDouble(); + final Path path = Path(); + path.addOval(Rect.fromLTWH(100, 30, testWidth, testHeight)); + rc.clipPath(path); + rc.drawImageRect(testImage, Rect.fromLTRB(0, 0, testWidth, testHeight), + Rect.fromLTWH(100, 30, testWidth, testHeight), Paint()); + rc.restore(); + await _checkScreenshot(rc, 'image_clipped_by_oval'); + }); + + // Regression test for https://github.com/flutter/flutter/issues/48683 + test('Clips triangle with oval clip path', () async { + final engine.RecordingCanvas rc = + engine.RecordingCanvas(const Rect.fromLTRB(0, 0, 400, 300)); + rc.save(); + double testWidth = 200; + double testHeight = 150; + final Path path = Path(); + path.addOval(Rect.fromLTWH(100, 30, testWidth, testHeight)); + rc.clipPath(path); + final Path paintPath = new Path(); + paintPath.moveTo(testWidth / 2, 0); + paintPath.lineTo(testWidth, testHeight); + paintPath.lineTo(0, testHeight); + paintPath.close(); + rc.drawPath( + paintPath, + Paint() + ..color = Color(0xFF00FF00) + ..style = PaintingStyle.fill); + rc.restore(); + await _checkScreenshot(rc, 'triangle_clipped_by_oval'); + }); +} + +engine.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 engine.HtmlImage(imageElement, width, height); +} -- GitLab