From 06d198b7ec4908c7c70be1fa1616d5341d9b0d85 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Tue, 23 Jul 2024 16:00:21 -0700 Subject: [PATCH 1/3] [skwasm] Fix platform view occlusion logic. --- lib/web_ui/lib/src/engine/layers.dart | 62 ++++--------------- .../test/engine/scene_builder_test.dart | 32 ++++++++++ .../test/engine/scene_builder_utils.dart | 26 ++++++-- 3 files changed, 67 insertions(+), 53 deletions(-) diff --git a/lib/web_ui/lib/src/engine/layers.dart b/lib/web_ui/lib/src/engine/layers.dart index a0568c120f42e..400aa41ba052e 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) { @@ -195,10 +180,7 @@ class ImageFilterOperation implements LayerOperation { 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 as SceneImageFilter).filterBounds(contentRect); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -241,10 +223,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 +252,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) { @@ -317,13 +293,7 @@ class TransformOperation implements LayerOperation { Matrix4 getMatrix() => 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) => getMatrix().transformRect(contentRect); @override void pre(SceneCanvas canvas, ui.Rect cullRect) { @@ -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); @@ -625,7 +589,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); @@ -649,7 +613,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/test/engine/scene_builder_test.dart b/lib/web_ui/test/engine/scene_builder_test.dart index 69088f897e440..756e6d194c6de 100644 --- a/lib/web_ui/test/engine/scene_builder_test.dart +++ b/lib/web_ui/test/engine/scene_builder_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:io'; + import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; @@ -150,6 +152,33 @@ void testMain() { PlatformView(1, platformViewRect, const PlatformViewStyling()) ])); }); + + 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))); + }); }); } @@ -209,12 +238,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.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 773d159a0ff6b8b51b9e04544aa834ea2c1b4fb8 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Tue, 23 Jul 2024 16:25:44 -0700 Subject: [PATCH 2/3] Remove unused import. --- lib/web_ui/test/engine/scene_builder_test.dart | 2 -- 1 file changed, 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 756e6d194c6de..d8826e8da6033 100644 --- a/lib/web_ui/test/engine/scene_builder_test.dart +++ b/lib/web_ui/test/engine/scene_builder_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:io'; - import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; From 7973a8203b1a6baa59cbd16de72cc3811e03ce85 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Wed, 24 Jul 2024 09:54:18 -0700 Subject: [PATCH 3/3] Implemented Yegor's suggestions. --- lib/web_ui/lib/src/engine/layers.dart | 14 +++++++------- lib/web_ui/lib/src/engine/scene_builder.dart | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/lib/src/engine/layers.dart b/lib/web_ui/lib/src/engine/layers.dart index 400aa41ba052e..1cbe6956f43f3 100644 --- a/lib/web_ui/lib/src/engine/layers.dart +++ b/lib/web_ui/lib/src/engine/layers.dart @@ -176,11 +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 mapRect(ui.Rect contentRect) => (filter as SceneImageFilter).filterBounds(contentRect); + ui.Rect mapRect(ui.Rect contentRect) => filter.filterBounds(contentRect); @override void pre(SceneCanvas canvas, ui.Rect contentRect) { @@ -188,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); } @@ -290,10 +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 mapRect(ui.Rect contentRect) => getMatrix().transformRect(contentRect); + ui.Rect mapRect(ui.Rect contentRect) => matrix.transformRect(contentRect); @override void pre(SceneCanvas canvas, ui.Rect cullRect) { @@ -308,7 +308,7 @@ class TransformOperation implements LayerOperation { @override PlatformViewStyling createPlatformViewStyling() => PlatformViewStyling( - position: PlatformViewPosition.transform(getMatrix()), + position: PlatformViewPosition.transform(matrix), ); } 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