[skwasm] Combine offset and transform properly. (flutter#53967)

Our treatment of the interaction between offset and transform was incorrect. Modified our platform view unit tests to cover more cases of nested offsets and transforms.
a-siva · Jul 18, 2024 · b65c93e · b65c93e
1 parent 312b519
commit b65c93e
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 29 deletions.
diff --git a/lib/web_ui/lib/src/engine/layers.dart b/lib/web_ui/lib/src/engine/layers.dart
@@ -499,18 +499,19 @@ class PlatformViewPosition {
     }
 
     // Otherwise, at least one of the positions involves a matrix transform.
-    final Matrix4 newTransform;
-    if (outerOffset != null) {
-      newTransform = Matrix4.translationValues(outerOffset.dx, outerOffset.dy, 0);
+    final Matrix4 innerTransform;
+    final Matrix4 outerTransform;
+    if (innerOffset != null) {
+      innerTransform = Matrix4.translationValues(innerOffset.dx, innerOffset.dy, 0);
     } else {
-      newTransform = outer.transform!.clone();
+      innerTransform = inner.transform!;
     }
-    if (innerOffset != null) {
-      newTransform.translate(innerOffset.dx, innerOffset.dy);
+    if (outerOffset != null) {
+      outerTransform = Matrix4.translationValues(outerOffset.dx, outerOffset.dy, 0);
     } else {
-      newTransform.multiply(inner.transform!);
+      outerTransform = outer.transform!;
     }
-    return PlatformViewPosition.transform(newTransform);
+    return PlatformViewPosition.transform(outerTransform.multiplied(innerTransform));
   }
 
   @override

diff --git a/lib/web_ui/lib/src/engine/scene_view.dart b/lib/web_ui/lib/src/engine/scene_view.dart
@@ -308,26 +308,26 @@ final class PlatformViewContainer extends SliceContainer {
     assert(_bounds != null);
     if (_dirty) {
       final DomCSSStyleDeclaration style = container.style;
-      final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio;
-      final double logicalWidth = _bounds!.width / devicePixelRatio;
-      final double logicalHeight = _bounds!.height / devicePixelRatio;
-      style.width = '${logicalWidth}px';
-      style.height = '${logicalHeight}px';
       style.position = 'absolute';
+      style.width = '${_bounds!.width}px';
+      style.height = '${_bounds!.height}px';
 
-      final PlatformViewPosition position = PlatformViewPosition.combine(
-        _styling!.position,
-        PlatformViewPosition.offset(_bounds!.topLeft),
-      );
-
-      final ui.Offset offset = position.offset ?? ui.Offset.zero;
-      final double logicalLeft = offset.dx / devicePixelRatio;
-      final double logicalTop = offset.dy / devicePixelRatio;
-      style.left = '${logicalLeft}px';
-      style.top = '${logicalTop}px';
+      final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio;
+      final PlatformViewPosition position = _styling!.position;
 
-      final Matrix4? transform = position.transform;
-      style.transform = transform != null ? float64ListToCssTransform3d(transform.storage) : '';
+      final Matrix4 transform;
+      if (position.transform != null) {
+        transform = position.transform!.clone()..translate(_bounds!.left, _bounds!.top);
+      } else {
+        final ui.Offset offset = position.offset ?? ui.Offset.zero;
+        transform = Matrix4.translationValues(_bounds!.left + offset.dx, _bounds!.top + offset.dy, 0);
+      }
+      final double inverseScale = 1.0 / devicePixelRatio;
+      final Matrix4 scaleMatrix =
+        Matrix4.diagonal3Values(inverseScale, inverseScale, 1);
+      scaleMatrix.multiply(transform);
+      style.transform = float64ListToCssTransform(scaleMatrix.storage);
+      style.transformOrigin = '0 0 0';
       style.opacity = _styling!.opacity != 1.0 ? '${_styling!.opacity}' : '';
       // TODO(jacksongardner): Implement clip styling for platform views
 

diff --git a/lib/web_ui/test/engine/scene_view_test.dart b/lib/web_ui/test/engine/scene_view_test.dart
@@ -167,10 +167,13 @@ void testMain() {
         containerElement.tagName, equalsIgnoringCase('flt-platform-view-slot'));
 
     final DomCSSStyleDeclaration style = containerElement.style;
-    expect(style.left, '25px');
-    expect(style.top, '40px');
-    expect(style.width, '50px');
-    expect(style.height, '60px');
+    expect(style.left, '');
+    expect(style.top, '');
+    expect(style.width, '100px');
+    expect(style.height, '120px');
+
+    // The heavy lifting of offsetting and sizing is done by the transform
+    expect(style.transform, 'matrix(0.5, 0, 0, 0.5, 25, 40)');
 
     debugOverrideDevicePixelRatio(null);
   });

diff --git a/lib/web_ui/test/ui/platform_view_test.dart b/lib/web_ui/test/ui/platform_view_test.dart
@@ -132,6 +132,39 @@ Future<void> testMain() async {
     await matchGoldenFile('platformview_transformed.png', region: region);
   });
 
+  test('transformed and offset platformview', () async {
+    await _createPlatformView(1, platformViewType);
+
+    final ui.PictureRecorder recorder = ui.PictureRecorder();
+    final ui.Canvas canvas = ui.Canvas(recorder);
+    canvas.drawCircle(
+      const ui.Offset(50, 50),
+      50,
+      ui.Paint()
+        ..style = ui.PaintingStyle.fill
+        ..color = const ui.Color(0xFFFF0000)
+    );
+
+    final ui.SceneBuilder sb = ui.SceneBuilder();
+    sb.pushOffset(0, 0);
+    sb.addPicture(const ui.Offset(100, 100), recorder.endRecording());
+
+    // Nest offsets both before and after the transform to make sure that they
+    // are applied properly.
+    sb.pushOffset(50, 50);
+    sb.pushTransform(Matrix4.rotationZ(0.1).toFloat64());
+    sb.pushOffset(25, 25);
+    sb.addPlatformView(
+      1,
+      offset: const ui.Offset(50, 50),
+      width: 50,
+      height: 50,
+    );
+    await renderScene(sb.build());
+
+    await matchGoldenFile('platformview_transformed_offset.png', region: region);
+  });
+
   test('offset platformview', () async {
     await _createPlatformView(1, platformViewType);