[web] Fix handling of ClipOp.none (#25080)

lib/web_ui/dev/goldens_lock.yaml

 repository: https://github.com/flutter/goldens.git
-revision: 4b4c256d6124a135b70c1a9a7ff10cf2827df31c
+revision: b86dc52ac1c8725ea17c50d9a7704687b5252833

lib/web_ui/lib/src/engine/html/clip.dart

@@ -76,7 +76,11 @@ class PersistedClipRect extends PersistedContainerSurface
   @override
   void recomputeTransformAndClip() {
     _transform = parent!._transform;
-    _localClipBounds = rect;
+    if (clipBehavior != ui.Clip.none) {
+      _localClipBounds = rect;
+    } else {
+      _localClipBounds = null;
+    }
     _localTransformInverse = null;
     _projectedClip = null;
   }
@@ -107,6 +111,7 @@ class PersistedClipRect extends PersistedContainerSurface
   void update(PersistedClipRect oldSurface) {
     super.update(oldSurface);
     if (rect != oldSurface.rect || clipBehavior != oldSurface.clipBehavior) {
+      _localClipBounds = null;
       apply();
     }
   }
@@ -129,7 +134,11 @@ class PersistedClipRRect extends PersistedContainerSurface
   @override
   void recomputeTransformAndClip() {
     _transform = parent!._transform;
-    _localClipBounds = rrect.outerRect;
+    if (clipBehavior != ui.Clip.none) {
+      _localClipBounds = rrect.outerRect;
+    } else {
+      _localClipBounds = null;
+    }
     _localTransformInverse = null;
     _projectedClip = null;
   }
@@ -165,6 +174,7 @@ class PersistedClipRRect extends PersistedContainerSurface
   void update(PersistedClipRRect oldSurface) {
     super.update(oldSurface);
     if (rrect != oldSurface.rrect || clipBehavior != oldSurface.clipBehavior) {
+      _localClipBounds = null;
       apply();
     }
   }
@@ -196,16 +206,20 @@ class PersistedPhysicalShape extends PersistedContainerSurface
   void recomputeTransformAndClip() {
     _transform = parent!._transform;
 
-    final ui.RRect? roundRect = path.toRoundedRect();
-    if (roundRect != null) {
-      _localClipBounds = roundRect.outerRect;
-    } else {
-      final ui.Rect? rect = path.toRect();
-      if (rect != null) {
-        _localClipBounds = rect;
+    if (clipBehavior != ui.Clip.none) {
+      final ui.RRect? roundRect = path.toRoundedRect();
+      if (roundRect != null) {
+        _localClipBounds = roundRect.outerRect;
       } else {
-        _localClipBounds = null;
+        final ui.Rect? rect = path.toRect();
+        if (rect != null) {
+          _localClipBounds = rect;
+        } else {
+          _localClipBounds = null;
+        }
       }
+    } else {
+      _localClipBounds = null;
     }
     _localTransformInverse = null;
     _projectedClip = null;
@@ -323,6 +337,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface
             offsetY: 0.0,
             scaleX: 1.0 / pathBounds.right,
             scaleY: 1.0 / pathBounds.bottom);
+
     /// If apply is called multiple times (without update), remove prior
     /// svg clip and render elements.
     _clipElement?.remove();
@@ -363,20 +378,23 @@ class PersistedPhysicalShape extends PersistedContainerSurface
 
     final ui.Rect pathBounds2 = path.getBounds();
     _svgElement = _pathToSvgElement(
-        path, SurfacePaintData()
+        path,
+        SurfacePaintData()
           ..style = ui.PaintingStyle.fill
-          ..color = color, '${pathBounds2.right}', '${pathBounds2.bottom}');
+          ..color = color,
+        '${pathBounds2.right}',
+        '${pathBounds2.bottom}');
+
     /// Render element behind the clipped content.
     rootElement!.insertBefore(_svgElement!, childContainer);
 
     final SurfaceShadowData shadow = computeShadow(pathBounds, elevation)!;
     final ui.Color boxShadowColor = toShadowColor(shadowColor);
     _svgElement!.style
-        ..filter =
-        'drop-shadow(${shadow.offset.dx}px ${shadow.offset.dy}px '
-        '${shadow.blurWidth}px '
-        'rgba(${boxShadowColor.red}, ${boxShadowColor.green}, '
-        '${boxShadowColor.blue}, ${boxShadowColor.alpha / 255}))'
+      ..filter = 'drop-shadow(${shadow.offset.dx}px ${shadow.offset.dy}px '
+          '${shadow.blurWidth}px '
+          'rgba(${boxShadowColor.red}, ${boxShadowColor.green}, '
+          '${boxShadowColor.blue}, ${boxShadowColor.alpha / 255}))'
       ..transform = 'translate(-${pathBounds2.left}px, -${pathBounds2.top}px)';
 
     rootElement!.style.backgroundColor = '';
@@ -385,8 +403,14 @@ class PersistedPhysicalShape extends PersistedContainerSurface
   @override
   void update(PersistedPhysicalShape oldSurface) {
     super.update(oldSurface);
-    if (oldSurface.path != path || oldSurface.elevation != elevation ||
-        oldSurface.shadowColor != shadowColor || oldSurface.color != color) {
+    bool pathChanged = oldSurface.path != path;
+    if (pathChanged) {
+      _localClipBounds = null;
+    }
+    if (pathChanged ||
+        oldSurface.elevation != elevation ||
+        oldSurface.shadowColor != shadowColor ||
+        oldSurface.color != color) {
       oldSurface._clipElement?.remove();
       oldSurface._clipElement = null;
       oldSurface._svgElement?.remove();
@@ -433,7 +457,11 @@ class PersistedClipPath extends PersistedContainerSurface
   @override
   void recomputeTransformAndClip() {
     super.recomputeTransformAndClip();
-    _localClipBounds ??= clipPath.getBounds();
+    if (clipBehavior != ui.Clip.none) {
+      _localClipBounds ??= clipPath.getBounds();
+    } else {
+      _localClipBounds = null;
+    }
   }
 
   @override

lib/web_ui/lib/src/engine/html/scene_builder.dart

@@ -114,7 +114,6 @@ class SurfaceSceneBuilder implements ui.SceneBuilder {
     ui.ClipRectEngineLayer? oldLayer,
   }) {
     assert(clipBehavior != null); // ignore: unnecessary_null_comparison
-    assert(clipBehavior != ui.Clip.none);
     return _pushSurface<PersistedClipRect>(
         PersistedClipRect(oldLayer as PersistedClipRect?, rect, clipBehavior));
   }
@@ -146,7 +145,6 @@ class SurfaceSceneBuilder implements ui.SceneBuilder {
     ui.ClipPathEngineLayer? oldLayer,
   }) {
     assert(clipBehavior != null); // ignore: unnecessary_null_comparison
-    assert(clipBehavior != ui.Clip.none);
     return _pushSurface<PersistedClipPath>(
         PersistedClipPath(oldLayer as PersistedClipPath?, path, clipBehavior));
   }

lib/web_ui/test/golden_tests/engine/compositing_golden_test.dart

@@ -69,6 +69,52 @@ void testMain() async {
         region: region);
   });
 
+  test('pushClipRect with offset and transform ClipOp none should not clip',
+      () async {
+    final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
+
+    builder.pushOffset(0, 80);
+    builder.pushTransform(
+      Matrix4.diagonal3Values(1, -1, 1).toFloat64(),
+    );
+    builder.pushClipRect(Rect.fromLTRB(10, 10, 60, 60),
+        clipBehavior: Clip.none);
+    _drawTestPicture(builder);
+    builder.pop();
+    builder.pop();
+    builder.pop();
+
+    html.document.body.append(builder.build().webOnlyRootElement);
+
+    await matchGoldenFile('compositing_clip_rect_clipop_none.png',
+        region: region);
+  });
+
+  test('pushClipRRect with offset and transform ClipOp none should not clip',
+      () async {
+    final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
+
+    builder.pushOffset(0, 80);
+    builder.pushTransform(
+      Matrix4.diagonal3Values(1, -1, 1).toFloat64(),
+    );
+    builder.pushClipRRect(
+        RRect.fromRectAndRadius(
+          const Rect.fromLTRB(10, 10, 60, 60),
+          const Radius.circular(1),
+        ),
+        clipBehavior: Clip.none);
+    _drawTestPicture(builder);
+    builder.pop();
+    builder.pop();
+    builder.pop();
+
+    html.document.body.append(builder.build().webOnlyRootElement);
+
+    await matchGoldenFile('compositing_clip_rrect_clipop_none.png',
+        region: region);
+  });
+
   test('pushClipRRect', () async {
     final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
     builder.pushClipRRect(
@@ -111,6 +157,35 @@ void testMain() async {
         region: region);
   });
 
+  test('pushPhysicalShape clipOp.none', () async {
+    final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
+    builder.pushPhysicalShape(
+      path: Path()..addRect(const Rect.fromLTRB(10, 10, 60, 60)),
+      clipBehavior: Clip.hardEdge,
+      color: const Color.fromRGBO(0, 0, 0, 0.3),
+      elevation: 0,
+    );
+    _drawTestPicture(builder);
+    builder.pop();
+
+    builder.pushOffset(70, 0);
+    builder.pushPhysicalShape(
+      path: Path()
+        ..addRRect(RRect.fromLTRBR(10, 10, 60, 60, const Radius.circular(5))),
+      clipBehavior: Clip.none,
+      color: const Color.fromRGBO(0, 0, 0, 0.3),
+      elevation: 0,
+    );
+    _drawTestPicture(builder);
+    builder.pop();
+    builder.pop();
+
+    html.document.body.append(builder.build().webOnlyRootElement);
+
+    await matchGoldenFile('compositing_shifted_physical_shape_clipnone.png',
+        region: region);
+  });
+
   test('pushPhysicalShape with path and elevation', () async {
     Path cutCornersButton = Path()
       ..moveTo(15, 10)
@@ -154,8 +229,9 @@ void testMain() async {
 
     builder.pushOffset(210, 0);
     builder.pushPhysicalShape(
-      path: Path()..addRRect(RRect.fromRectAndRadius(
-          Rect.fromLTRB(10, 10, 60, 60), Radius.circular(10.0))),
+      path: Path()
+        ..addRRect(RRect.fromRectAndRadius(
+            Rect.fromLTRB(10, 10, 60, 60), Radius.circular(10.0))),
       clipBehavior: Clip.hardEdge,
       color: const Color(0xFFA0FFFF),
       elevation: 4,
@@ -192,8 +268,7 @@ void testMain() async {
 
     html.Element viewElement = builder.build().webOnlyRootElement;
     html.document.body.append(viewElement);
-    await matchGoldenFile('compositing_physical_update_1.png',
-        region: region);
+    await matchGoldenFile('compositing_physical_update_1.png', region: region);
     viewElement.remove();
 
     /// Update color to green.
@@ -210,8 +285,7 @@ void testMain() async {
 
     html.Element viewElement2 = builder2.build().webOnlyRootElement;
     html.document.body.append(viewElement2);
-    await matchGoldenFile('compositing_physical_update_2.png',
-        region: region);
+    await matchGoldenFile('compositing_physical_update_2.png', region: region);
     viewElement2.remove();
 
     /// Update elevation.
@@ -246,8 +320,7 @@ void testMain() async {
 
     html.Element viewElement4 = builder4.build().webOnlyRootElement;
     html.document.body.append(viewElement4);
-    await matchGoldenFile('compositing_physical_update_4.png',
-        region: region);
+    await matchGoldenFile('compositing_physical_update_4.png', region: region);
     viewElement4.remove();
 
     /// Update shape back to arbitrary path.
@@ -265,8 +338,8 @@ void testMain() async {
     html.Element viewElement5 = builder5.build().webOnlyRootElement;
     html.document.body.append(viewElement5);
     await matchGoldenFile('compositing_physical_update_3.png',
-         region: region, maxDiffRatePercent:
-         browserEngine == BrowserEngine.webkit ? 0.6 : 0.4);
+        region: region,
+        maxDiffRatePercent: browserEngine == BrowserEngine.webkit ? 0.6 : 0.4);
     viewElement5.remove();
 
     /// Update shadow color.
@@ -284,8 +357,7 @@ void testMain() async {
 
     html.Element viewElement6 = builder6.build().webOnlyRootElement;
     html.document.body.append(viewElement6);
-    await matchGoldenFile('compositing_physical_update_5.png',
-        region: region);
+    await matchGoldenFile('compositing_physical_update_5.png', region: region);
     viewElement6.remove();
   });
 
@@ -592,8 +664,10 @@ void _testCullRectComputation() {
     final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
 
     builder.pushTransform(Matrix4.diagonal3Values(
-        EnginePlatformDispatcher.browserDevicePixelRatio,
-        EnginePlatformDispatcher.browserDevicePixelRatio, 1.0).toFloat64());
+            EnginePlatformDispatcher.browserDevicePixelRatio,
+            EnginePlatformDispatcher.browserDevicePixelRatio,
+            1.0)
+        .toFloat64());
 
     // TODO(yjbanov): see the TODO below.
     // final double screenWidth = html.window.innerWidth.toDouble();
@@ -610,9 +684,8 @@ void _testCullRectComputation() {
       const Rect.fromLTRB(-200, -200, 200, 200),
     );
 
-    builder.pushTransform(
-      Matrix4.rotationY(45.0 * math.pi / 180.0).toFloat64()
-    );
+    builder
+        .pushTransform(Matrix4.rotationY(45.0 * math.pi / 180.0).toFloat64());
 
     builder.pushClipRect(
       const Rect.fromLTRB(-140, -140, 140, 140),
@@ -726,7 +799,9 @@ void _testCullRectComputation() {
     () async {
       // To reproduce blurriness we need real clipping.
       final DomParagraph paragraph =
-          (DomParagraphBuilder(ParagraphStyle(fontFamily: 'Roboto'))..addText('Am I blurry?')).build();
+          (DomParagraphBuilder(ParagraphStyle(fontFamily: 'Roboto'))
+                ..addText('Am I blurry?'))
+              .build();
       paragraph.layout(const ParagraphConstraints(width: 1000));
 
       final Rect canvasSize = Rect.fromLTRB(
@@ -774,7 +849,10 @@ void _testCullRectComputation() {
 
       final html.Element sceneElement = builder.build().webOnlyRootElement;
       expect(
-        sceneElement.querySelectorAll('p').map<String>((e) => e.innerText).toList(),
+        sceneElement
+            .querySelectorAll('p')
+            .map<String>((e) => e.innerText)
+            .toList(),
         <String>['Am I blurry?', 'Am I blurry?'],
         reason: 'Expected to render text using HTML',
       );