From c9f5242019af5ca5d0f0fb3478cf0691a28bdfa4 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Wed, 24 Jul 2024 10:48:48 -0700 Subject: [PATCH 1/3] [skwasm] Fix platform view occlusion logic. (#54061) The occlusion rectangle for platform views was going through this `inverseMapRect` code path, which actually was giving us the wrong results. The operations should just be doing the normal transformation on the rectangles to get the right result. It actually turns out we don't need the inverse mapping function, so I removed it, and I renamed the somewhat confusingly named `cullRect` function to `mapRect` which I think makes a bit more sense. This should resolve https://github.com/flutter/flutter/issues/152139 --- lib/web_ui/lib/src/engine/layers.dart | 72 +++++-------------- lib/web_ui/lib/src/engine/scene_builder.dart | 2 +- .../test/engine/scene_builder_test.dart | 32 ++++++++- .../test/engine/scene_builder_utils.dart | 26 +++++-- 4 files changed, 72 insertions(+), 60 deletions(-) diff --git a/lib/web_ui/lib/src/engine/layers.dart b/lib/web_ui/lib/src/engine/layers.dart index 7d6940be76be5..eb7925652db43 100644 --- a/lib/web_ui/lib/src/engine/layers.dart +++ b/lib/web_ui/lib/src/engine/layers.dart @@ -21,10 +21,7 @@ class BackdropFilterOperation implements LayerOperation { final ui.BlendMode mode; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect; - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect; @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -50,10 +47,7 @@ class ClipPathOperation implements LayerOperation { final ui.Clip clip; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect.intersect(path.getBounds()); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect.intersect(path.getBounds()); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -89,10 +83,7 @@ class ClipRectOperation implements LayerOperation { final ui.Clip clip; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect.intersect(rect); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect.intersect(rect); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -128,10 +119,7 @@ class ClipRRectOperation implements LayerOperation { final ui.Clip clip; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect.intersect(rrect.outerRect); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect.intersect(rrect.outerRect); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -166,10 +154,7 @@ class ColorFilterOperation implements LayerOperation { final ui.ColorFilter filter; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect; - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect; @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -191,14 +176,11 @@ class ImageFilterLayer class ImageFilterOperation implements LayerOperation { ImageFilterOperation(this.filter, this.offset); - final ui.ImageFilter filter; + final SceneImageFilter filter; final ui.Offset offset; @override - ui.Rect cullRect(ui.Rect contentRect) => (filter as SceneImageFilter).filterBounds(contentRect); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => filter.filterBounds(contentRect); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -206,8 +188,7 @@ class ImageFilterOperation implements LayerOperation { canvas.save(); canvas.translate(offset.dx, offset.dy); } - final ui.Rect adjustedContentRect = - (filter as SceneImageFilter).filterBounds(contentRect); + final ui.Rect adjustedContentRect = filter.filterBounds(contentRect); canvas.saveLayer(adjustedContentRect, ui.Paint()..imageFilter = filter); } @@ -241,10 +222,7 @@ class OffsetOperation implements LayerOperation { final double dy; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect.shift(ui.Offset(dx, dy)); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect.shift(ui.Offset(-dx, -dy)); + ui.Rect mapRect(ui.Rect contentRect) => contentRect.shift(ui.Offset(dx, dy)); @override void pre(SceneCanvas canvas, ui.Rect cullRect) { @@ -273,10 +251,7 @@ class OpacityOperation implements LayerOperation { final ui.Offset offset; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect.shift(offset); - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect.shift(offset); @override void pre(SceneCanvas canvas, ui.Rect cullRect) { @@ -314,16 +289,11 @@ class TransformOperation implements LayerOperation { final Float64List transform; - Matrix4 getMatrix() => Matrix4.fromFloat32List(toMatrix32(transform)); + Matrix4? _memoizedMatrix; + Matrix4 get matrix => _memoizedMatrix ?? (_memoizedMatrix = Matrix4.fromFloat32List(toMatrix32(transform))); @override - ui.Rect cullRect(ui.Rect contentRect) => getMatrix().transformRect(contentRect); - - @override - ui.Rect inverseMapRect(ui.Rect rect) { - final Matrix4 matrix = getMatrix()..invert(); - return matrix.transformRect(rect); - } + ui.Rect mapRect(ui.Rect contentRect) => matrix.transformRect(contentRect); @override void pre(SceneCanvas canvas, ui.Rect cullRect) { @@ -338,7 +308,7 @@ class TransformOperation implements LayerOperation { @override PlatformViewStyling createPlatformViewStyling() => PlatformViewStyling( - position: PlatformViewPosition.transform(getMatrix()), + position: PlatformViewPosition.transform(matrix), ); } @@ -353,10 +323,7 @@ class ShaderMaskOperation implements LayerOperation { final ui.BlendMode blendMode; @override - ui.Rect cullRect(ui.Rect contentRect) => contentRect; - - @override - ui.Rect inverseMapRect(ui.Rect rect) => rect; + ui.Rect mapRect(ui.Rect contentRect) => contentRect; @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -444,11 +411,8 @@ abstract class LayerOperation { // Given an input content rectangle, this returns a conservative estimate of // the covering rectangle of the content after it has been processed by the // layer operation. - ui.Rect cullRect(ui.Rect contentRect); + ui.Rect mapRect(ui.Rect contentRect); - // Takes a rectangle in the layer's coordinate space and maps it to the parent - // coordinate space. - ui.Rect inverseMapRect(ui.Rect rect); void pre(SceneCanvas canvas, ui.Rect contentRect); void post(SceneCanvas canvas, ui.Rect contentRect); @@ -636,7 +600,7 @@ class LayerBuilder { // Merge the existing draw commands into a single picture and add a slice // with that picture to the slice list. final ui.Rect drawnRect = picturesRect ?? ui.Rect.zero; - final ui.Rect rect = operation?.cullRect(drawnRect) ?? drawnRect; + final ui.Rect rect = operation?.mapRect(drawnRect) ?? drawnRect; final (ui.PictureRecorder recorder, SceneCanvas canvas) = _createRecorder(rect); operation?.pre(canvas, rect); @@ -660,7 +624,7 @@ class LayerBuilder { // slice. ui.Rect? occlusionRect = platformViewRect; if (occlusionRect != null && operation != null) { - occlusionRect = operation!.inverseMapRect(occlusionRect); + occlusionRect = operation!.mapRect(occlusionRect); } layer.slices.add(PlatformViewSlice(pendingPlatformViews, occlusionRect)); } diff --git a/lib/web_ui/lib/src/engine/scene_builder.dart b/lib/web_ui/lib/src/engine/scene_builder.dart index 4e0736c1e87da..c54cd65bfc96f 100644 --- a/lib/web_ui/lib/src/engine/scene_builder.dart +++ b/lib/web_ui/lib/src/engine/scene_builder.dart @@ -185,7 +185,7 @@ class EngineSceneBuilder implements ui.SceneBuilder { ui.ImageFilterEngineLayer? oldLayer }) => pushLayer( ImageFilterLayer(), - ImageFilterOperation(filter, offset), + ImageFilterOperation(filter as SceneImageFilter, offset), ); @override diff --git a/lib/web_ui/test/engine/scene_builder_test.dart b/lib/web_ui/test/engine/scene_builder_test.dart index 0a80cb2cad987..c4e45b4f2d2cd 100644 --- a/lib/web_ui/test/engine/scene_builder_test.dart +++ b/lib/web_ui/test/engine/scene_builder_test.dart @@ -158,6 +158,33 @@ void testMain() { )) ])); }); + + test('platform view sandwich (overlapping) with offset layers', () { + final EngineSceneBuilder sceneBuilder = EngineSceneBuilder(); + + const ui.Rect pictureRect1 = ui.Rect.fromLTRB(100, 100, 200, 200); + sceneBuilder.addPicture(ui.Offset.zero, StubPicture(pictureRect1)); + + sceneBuilder.pushOffset(150, 150); + const ui.Rect platformViewRect = ui.Rect.fromLTRB(0, 0, 100, 100); + sceneBuilder.addPlatformView( + 1, + offset: platformViewRect.topLeft, + width: platformViewRect.width, + height: platformViewRect.height + ); + sceneBuilder.pushOffset(50, 50); + sceneBuilder.addPicture(ui.Offset.zero, StubPicture(const ui.Rect.fromLTRB(0, 0, 100, 100))); + + final EngineScene scene = sceneBuilder.build() as EngineScene; + final List slices = scene.rootLayer.slices; + expect(slices.length, 3); + expect(slices[0], pictureSliceWithRect(pictureRect1)); + expect(slices[1], platformViewSliceWithViews([ + PlatformView(1, platformViewRect, const PlatformViewStyling(position: PlatformViewPosition.offset(ui.Offset(150, 150)))) + ])); + expect(slices[2], pictureSliceWithRect(const ui.Rect.fromLTRB(200, 200, 300, 300))); + }); }); } @@ -217,12 +244,15 @@ class PlatformViewSliceMatcher extends Matcher { final PlatformView expectedView = expectedPlatformViews[i]; final PlatformView actualView = item.views[i]; if (expectedView.viewId != actualView.viewId) { + print('viewID mismatch'); return false; } - if (expectedView.size != actualView.size) { + if (expectedView.bounds != actualView.bounds) { + print('bounds mismatch'); return false; } if (expectedView.styling != actualView.styling) { + print('styling mismatch'); return false; } } diff --git a/lib/web_ui/test/engine/scene_builder_utils.dart b/lib/web_ui/test/engine/scene_builder_utils.dart index 72541786f819f..033177c8d730b 100644 --- a/lib/web_ui/test/engine/scene_builder_utils.dart +++ b/lib/web_ui/test/engine/scene_builder_utils.dart @@ -60,9 +60,21 @@ class StubPictureRecorder implements ui.PictureRecorder { class StubSceneCanvas implements SceneCanvas { List pictures = []; + // We actually use offsets in some of the tests, so we need to track the + // translate calls as they are made. + List offsetStack = [ui.Offset.zero]; + + ui.Offset get currentOffset { + return offsetStack.last; + } + + set currentOffset(ui.Offset offset) { + offsetStack[offsetStack.length - 1] = offset; + } + @override void drawPicture(ui.Picture picture) { - pictures.add(picture as StubPicture); + pictures.add(StubPicture((picture as StubPicture).cullRect.shift(currentOffset))); } @override @@ -155,7 +167,9 @@ class StubSceneCanvas implements SceneCanvas { } @override - void restore() {} + void restore() { + offsetStack.removeLast(); + } @override void restoreToCount(int count) {} @@ -164,7 +178,9 @@ class StubSceneCanvas implements SceneCanvas { void rotate(double radians) {} @override - void save() {} + void save() { + offsetStack.add(currentOffset); + } @override void saveLayer(ui.Rect? bounds, ui.Paint paint) {} @@ -182,5 +198,7 @@ class StubSceneCanvas implements SceneCanvas { void transform(Float64List matrix4) {} @override - void translate(double dx, double dy) {} + void translate(double dx, double dy) { + currentOffset += ui.Offset(dx, dy); + } } From dec4a04554442cc9fea8b71c84796b7147afa9eb Mon Sep 17 00:00:00 2001 From: Rexios Date: Thu, 29 Aug 2024 13:00:07 -0400 Subject: [PATCH 2/3] Update scene_builder_test.dart --- lib/web_ui/test/engine/scene_builder_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/engine/scene_builder_test.dart b/lib/web_ui/test/engine/scene_builder_test.dart index c4e45b4f2d2cd..f3cb90455043a 100644 --- a/lib/web_ui/test/engine/scene_builder_test.dart +++ b/lib/web_ui/test/engine/scene_builder_test.dart @@ -247,8 +247,8 @@ class PlatformViewSliceMatcher extends Matcher { print('viewID mismatch'); return false; } - if (expectedView.bounds != actualView.bounds) { - print('bounds mismatch'); + if (expectedView.size != actualView.size) { + print('size mismatch'); return false; } if (expectedView.styling != actualView.styling) { From fa79ceecd737e6ff33803dba7d599b095a8c3edd Mon Sep 17 00:00:00 2001 From: Rexios Date: Thu, 29 Aug 2024 13:39:44 -0400 Subject: [PATCH 3/3] Fix test --- lib/web_ui/test/engine/scene_builder_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/engine/scene_builder_test.dart b/lib/web_ui/test/engine/scene_builder_test.dart index f3cb90455043a..60d65d98fa611 100644 --- a/lib/web_ui/test/engine/scene_builder_test.dart +++ b/lib/web_ui/test/engine/scene_builder_test.dart @@ -181,7 +181,7 @@ void testMain() { expect(slices.length, 3); expect(slices[0], pictureSliceWithRect(pictureRect1)); expect(slices[1], platformViewSliceWithViews([ - PlatformView(1, platformViewRect, const PlatformViewStyling(position: PlatformViewPosition.offset(ui.Offset(150, 150)))) + PlatformView(1, platformViewRect.size, const PlatformViewStyling(position: PlatformViewPosition.offset(ui.Offset(150, 150)))) ])); expect(slices[2], pictureSliceWithRect(const ui.Rect.fromLTRB(200, 200, 300, 300))); });