From 7bb35d8a86037577ac435a11b2ce4c817738e479 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen <1961493+harryterkelsen@users.noreply.github.com> Date: Thu, 5 Sep 2024 15:23:37 -0700 Subject: [PATCH] [canvaskit] Fix incorrect calculation of ImageFilter paint bounds (flutter/engine#54980) Fixes calculation of `ImageFilter` bounds by taking into account the offset. Fixes https://github.com/flutter/flutter/issues/154303 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../lib/src/engine/canvaskit/layer.dart | 30 ++++++++++--------- .../web_ui/test/ui/scene_builder_test.dart | 25 ++++++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart index b4d419a9265..010fc012786 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart @@ -415,32 +415,34 @@ class ImageFilterEngineLayer extends ContainerLayer } else { convertible = _filter as CkManagedSkImageFilterConvertible; } - final ui.Rect childPaintBounds = + ui.Rect childPaintBounds = prerollChildren(prerollContext, childMatrix); - if (_filter is ui.ColorFilter) { - // If the filter is a ColorFilter, the extended paint bounds will be the - // entire screen, which is not what we want. - paintBounds = childPaintBounds; - } else { - convertible.withSkImageFilter((skFilter) { - paintBounds = rectFromSkIRect( - skFilter.getOutputBounds(toSkRect(childPaintBounds)), - ); - }); - } + childPaintBounds = childPaintBounds.translate(_offset.dx, _offset.dy); + if (_filter is ui.ColorFilter) { + // If the filter is a ColorFilter, the extended paint bounds will be the + // entire screen, which is not what we want. + paintBounds = childPaintBounds; + } else { + convertible.withSkImageFilter((skFilter) { + paintBounds = rectFromSkIRect( + skFilter.getOutputBounds(toSkRect(childPaintBounds)), + ); + }); + } prerollContext.mutatorsStack.pop(); } @override void paint(PaintContext paintContext) { assert(needsPainting); + final ui.Rect offsetPaintBounds = paintBounds.shift(-_offset); paintContext.internalNodesCanvas.save(); paintContext.internalNodesCanvas.translate(_offset.dx, _offset.dy); paintContext.internalNodesCanvas - .clipRect(paintBounds, ui.ClipOp.intersect, false); + .clipRect(offsetPaintBounds, ui.ClipOp.intersect, false); final CkPaint paint = CkPaint(); paint.imageFilter = _filter; - paintContext.internalNodesCanvas.saveLayer(paintBounds, paint); + paintContext.internalNodesCanvas.saveLayer(offsetPaintBounds, paint); paintChildren(paintContext); paintContext.internalNodesCanvas.restore(); paintContext.internalNodesCanvas.restore(); diff --git a/engine/src/flutter/lib/web_ui/test/ui/scene_builder_test.dart b/engine/src/flutter/lib/web_ui/test/ui/scene_builder_test.dart index fc77fd98c2d..aeaef07ad0c 100644 --- a/engine/src/flutter/lib/web_ui/test/ui/scene_builder_test.dart +++ b/engine/src/flutter/lib/web_ui/test/ui/scene_builder_test.dart @@ -262,6 +262,31 @@ Future testMain() async { await matchGoldenFile('scene_builder_image_filter.png', region: region); }); + // Regression test for https://github.com/flutter/flutter/issues/154303 + test('image filter layer with offset', () async { + final ui.SceneBuilder sceneBuilder = ui.SceneBuilder(); + + sceneBuilder.pushClipRect(const ui.Rect.fromLTWH(100, 100, 100, 100)); + sceneBuilder.pushImageFilter( + ui.ImageFilter.blur( + sigmaX: 5.0, + sigmaY: 5.0, + ), + offset: const ui.Offset(100, 100), + ); + + sceneBuilder.addPicture(ui.Offset.zero, drawPicture((ui.Canvas canvas) { + canvas.drawCircle(const ui.Offset(50, 50), 25, + ui.Paint()..color = const ui.Color(0xFF00FF00)); + })); + + await renderScene(sceneBuilder.build()); + await matchGoldenFile( + 'scene_builder_image_filter_with_offset.png', + region: region, + ); + }); + test('color filter layer', () async { final ui.SceneBuilder sceneBuilder = ui.SceneBuilder(); const ui.ColorFilter sepia = ui.ColorFilter.matrix([