From 5d25998b9e7898f09f5c176a51e9c1d865135812 Mon Sep 17 00:00:00 2001
From: ferhatb <ferhat@gmail.com>
Date: Thu, 15 Oct 2020 21:15:16 -0700
Subject: [PATCH 1/5] Implement ClipOp.difference

---
 lib/web_ui/lib/src/engine/bitmap_canvas.dart  | 16 +++++--
 lib/web_ui/lib/src/engine/canvas_pool.dart    | 15 +++++--
 lib/web_ui/lib/src/engine/dom_canvas.dart     |  2 +-
 lib/web_ui/lib/src/engine/engine_canvas.dart  |  4 +-
 .../lib/src/engine/html/recording_canvas.dart |  4 +-
 .../engine/canvas_golden_test.dart            |  4 +-
 .../engine/clip_op_golden_test.dart           | 44 +++++++++++++++++++
 .../test/golden_tests/engine/screenshot.dart  | 43 ++++++++++++++++++
 lib/web_ui/test/mock_engine_canvas.dart       |  2 +-
 9 files changed, 120 insertions(+), 14 deletions(-)
 create mode 100644 lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
 create mode 100644 lib/web_ui/test/golden_tests/engine/screenshot.dart

diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart
index 0f054559b18a0..8a61405a5822f 100644
--- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart
+++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart
@@ -271,8 +271,18 @@ class BitmapCanvas extends EngineCanvas {
   }
 
   @override
-  void clipRect(ui.Rect rect) {
-    _canvasPool.clipRect(rect);
+  void clipRect(ui.Rect rect, ui.ClipOp op) {
+    if (op == ui.ClipOp.difference) {
+      // Create 2 rectangles inside each other that represents
+      // clip area difference using even-odd fill rule.
+      SurfacePath path = new SurfacePath();
+      path.fillType = ui.PathFillType.evenOdd;
+      path.addRect(ui.Rect.fromLTWH(0, 0, _bounds.width, _bounds.height));
+      path.addRect(rect);
+      _canvasPool.clipPath(path);
+    } else {
+      _canvasPool.clipRect(rect);
+    }
   }
 
   @override
@@ -466,7 +476,7 @@ class BitmapCanvas extends EngineCanvas {
     } else {
       if (requiresClipping) {
         save();
-        clipRect(dst);
+        clipRect(dst, ui.ClipOp.intersect);
       }
       double targetLeft = dst.left;
       double targetTop = dst.top;
diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart
index d56351124eed6..76894efc00aa1 100644
--- a/lib/web_ui/lib/src/engine/canvas_pool.dart
+++ b/lib/web_ui/lib/src/engine/canvas_pool.dart
@@ -210,8 +210,13 @@ class _CanvasPool extends _SaveStackTracking {
         } else if (clipEntry.rrect != null) {
           _clipRRect(ctx, clipEntry.rrect!);
         } else if (clipEntry.path != null) {
-          _runPath(ctx, clipEntry.path as SurfacePath);
-          ctx.clip();
+          SurfacePath path = clipEntry.path as SurfacePath;
+          _runPath(ctx, path);
+          if (path.fillType == ui.PathFillType.evenOdd) {
+            ctx.clip('evenodd');
+          } else {
+            ctx.clip();
+          }
         }
       }
     }
@@ -443,7 +448,11 @@ class _CanvasPool extends _SaveStackTracking {
     if (_canvas != null) {
       html.CanvasRenderingContext2D ctx = context;
       _runPath(ctx, path as SurfacePath);
-      ctx.clip();
+      if (path.fillType == ui.PathFillType.nonZero) {
+        ctx.clip();
+      } else {
+        ctx.clip('evenodd');
+      }
     }
   }
 
diff --git a/lib/web_ui/lib/src/engine/dom_canvas.dart b/lib/web_ui/lib/src/engine/dom_canvas.dart
index 0eabf6d8a4aac..11668eb2472fc 100644
--- a/lib/web_ui/lib/src/engine/dom_canvas.dart
+++ b/lib/web_ui/lib/src/engine/dom_canvas.dart
@@ -28,7 +28,7 @@ class DomCanvas extends EngineCanvas with SaveElementStackTracking {
   }
 
   @override
-  void clipRect(ui.Rect rect) {
+  void clipRect(ui.Rect rect, ui.ClipOp op) {
     throw UnimplementedError();
   }
 
diff --git a/lib/web_ui/lib/src/engine/engine_canvas.dart b/lib/web_ui/lib/src/engine/engine_canvas.dart
index 5b0f7d31d8167..c197515900a67 100644
--- a/lib/web_ui/lib/src/engine/engine_canvas.dart
+++ b/lib/web_ui/lib/src/engine/engine_canvas.dart
@@ -33,7 +33,7 @@ abstract class EngineCanvas {
 
   void transform(Float32List matrix4);
 
-  void clipRect(ui.Rect rect);
+  void clipRect(ui.Rect rect, ui.ClipOp clipOp);
 
   void clipRRect(ui.RRect rrect);
 
@@ -222,7 +222,7 @@ mixin SaveStackTracking on EngineCanvas {
   ///
   /// Classes that override this method must call `super.clipRect()`.
   @override
-  void clipRect(ui.Rect rect) {
+  void clipRect(ui.Rect rect, ui.ClipOp op) {
     _clipStack ??= <_SaveClipEntry>[];
     _clipStack!.add(_SaveClipEntry.rect(rect, _currentTransform.clone()));
   }
diff --git a/lib/web_ui/lib/src/engine/html/recording_canvas.dart b/lib/web_ui/lib/src/engine/html/recording_canvas.dart
index cb6a1ed626fed..9f7e6b93bca92 100644
--- a/lib/web_ui/lib/src/engine/html/recording_canvas.dart
+++ b/lib/web_ui/lib/src/engine/html/recording_canvas.dart
@@ -275,7 +275,7 @@ class RecordingCanvas {
 
   void clipRect(ui.Rect rect, ui.ClipOp clipOp) {
     assert(!_recordingEnded);
-    final PaintClipRect command = PaintClipRect(rect, clipOp);
+    final DrawCommand command = PaintClipRect(rect, clipOp);
     switch (clipOp) {
       case ui.ClipOp.intersect:
         _paintBounds.clipRect(rect, command);
@@ -810,7 +810,7 @@ class PaintClipRect extends DrawCommand {
 
   @override
   void apply(EngineCanvas canvas) {
-    canvas.clipRect(rect);
+    canvas.clipRect(rect, clipOp);
   }
 
   @override
diff --git a/lib/web_ui/test/golden_tests/engine/canvas_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_golden_test.dart
index a072d025974fd..dde35315d38bc 100644
--- a/lib/web_ui/test/golden_tests/engine/canvas_golden_test.dart
+++ b/lib/web_ui/test/golden_tests/engine/canvas_golden_test.dart
@@ -157,9 +157,9 @@ void testMain() async {
 
     canvas = BitmapCanvas(canvasSize);
     canvas.debugChildOverdraw = true;
-    canvas.clipRect(outerClip);
+    canvas.clipRect(outerClip, ClipOp.intersect);
     canvas.drawParagraph(paragraph, const Offset(8.5, 8.5));
-    canvas.clipRect(innerClip);
+    canvas.clipRect(innerClip, ClipOp.intersect);
     canvas.drawParagraph(paragraph, Offset(8.5, 8.5 + innerClip.top));
 
     expect(
diff --git a/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
new file mode 100644
index 0000000000000..459ca034a2e48
--- /dev/null
+++ b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
@@ -0,0 +1,44 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.6
+import 'package:test/bootstrap/browser.dart';
+import 'package:test/test.dart';
+import 'package:ui/ui.dart';
+import 'package:ui/src/engine.dart';
+import 'screenshot.dart';
+
+void main() {
+  internalBootstrapBrowserTest(() => testMain);
+}
+
+void testMain() async {
+  setUp(() async {
+    debugEmulateFlutterTesterEnvironment = true;
+  });
+
+  // setUpStableTestFonts();
+
+  /// Regression test for https://github.com/flutter/flutter/issues/64734.
+  test('Clips using difference', () async {
+    final Rect region = const Rect.fromLTRB(0, 0, 400, 300);
+    final RecordingCanvas canvas = RecordingCanvas(region);
+    final Rect titleRect = Rect.fromLTWH(20, 0, 50, 20);
+    final Paint paint = Paint()
+      ..style = PaintingStyle.stroke
+      ..color = const Color(0xff000000)
+      ..strokeWidth = 1;
+    canvas.save();
+    try {
+      final Rect borderRect = Rect.fromLTRB(0, 10, region.width, region.height);
+      canvas.clipRect(titleRect, ClipOp.difference);
+      canvas.drawRect(borderRect, paint);
+    } finally {
+      canvas.restore();
+    }
+    canvas..drawRect(titleRect, paint);
+    await canvasScreenshot(canvas, 'clip_op_difference',
+        region: region.inflate(20));
+  });
+}
diff --git a/lib/web_ui/test/golden_tests/engine/screenshot.dart b/lib/web_ui/test/golden_tests/engine/screenshot.dart
new file mode 100644
index 0000000000000..f120f790f1840
--- /dev/null
+++ b/lib/web_ui/test/golden_tests/engine/screenshot.dart
@@ -0,0 +1,43 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.6
+import 'dart:html' as html;
+import 'package:ui/ui.dart' as ui;
+import 'package:ui/src/engine.dart';
+import 'package:web_engine_tester/golden_tester.dart';
+import 'package:test/test.dart';
+
+// Commit a recording canvas to a bitmap, and compare with the expected
+Future<void> canvasScreenshot(RecordingCanvas rc, String fileName,
+    {ui.Rect region = const ui.Rect.fromLTWH(0, 0, 600, 800),
+      double maxDiffRatePercent = 0.0, bool write: false}) async {
+  final EngineCanvas engineCanvas = BitmapCanvas(region);
+
+  rc.endRecording();
+  rc.apply(engineCanvas, region);
+
+  // Wrap in <flt-scene> so that our CSS selectors kick in.
+  final html.Element sceneElement = html.Element.tag('flt-scene');
+  try {
+    sceneElement.append(engineCanvas.rootElement);
+    html.document.body.append(sceneElement);
+    await matchGoldenFile('$fileName.png',
+        region: region, maxDiffRatePercent: maxDiffRatePercent, write: write);
+  } finally {
+    // The page is reused across tests, so remove the element after taking the
+    // Scuba screenshot.
+    sceneElement.remove();
+  }
+}
+
+/// Configures the test to use bundled Roboto and Ahem fonts to avoid golden
+/// screenshot differences due to differences in the preinstalled system fonts.
+void setUpStableTestFonts() {
+  setUp(() async {
+    await ui.webOnlyInitializePlatform();
+    ui.webOnlyFontCollection.debugRegisterTestFonts();
+    await ui.webOnlyFontCollection.ensureFontsLoaded();
+  });
+}
diff --git a/lib/web_ui/test/mock_engine_canvas.dart b/lib/web_ui/test/mock_engine_canvas.dart
index a88dccc0b12ca..91b59ceb8e320 100644
--- a/lib/web_ui/test/mock_engine_canvas.dart
+++ b/lib/web_ui/test/mock_engine_canvas.dart
@@ -99,7 +99,7 @@ class MockEngineCanvas implements EngineCanvas {
   }
 
   @override
-  void clipRect(Rect rect) {
+  void clipRect(Rect rect, ClipOp op) {
     _called('clipRect', arguments: rect);
   }
 

From 11ebcfbf96b99c0bf8cbd3b8daf3f9d55fdaea9f Mon Sep 17 00:00:00 2001
From: ferhatb <ferhat@gmail.com>
Date: Thu, 15 Oct 2020 21:59:30 -0700
Subject: [PATCH 2/5] update test

---
 lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
index 459ca034a2e48..b2807ff995250 100644
--- a/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
+++ b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
@@ -39,6 +39,6 @@ void testMain() async {
     }
     canvas..drawRect(titleRect, paint);
     await canvasScreenshot(canvas, 'clip_op_difference',
-        region: region.inflate(20));
+        region: const Rect.fromLTRB(0, 0, 420, 360));
   });
 }

From 2cb6d24db9f9cb4e192b45430c430631605946bf Mon Sep 17 00:00:00 2001
From: ferhatb <ferhat@gmail.com>
Date: Fri, 16 Oct 2020 15:04:14 -0700
Subject: [PATCH 3/5] addressed reviewer comments

---
 lib/web_ui/lib/src/engine/canvas_pool.dart | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart
index 76894efc00aa1..62914af760086 100644
--- a/lib/web_ui/lib/src/engine/canvas_pool.dart
+++ b/lib/web_ui/lib/src/engine/canvas_pool.dart
@@ -212,10 +212,10 @@ class _CanvasPool extends _SaveStackTracking {
         } else if (clipEntry.path != null) {
           SurfacePath path = clipEntry.path as SurfacePath;
           _runPath(ctx, path);
-          if (path.fillType == ui.PathFillType.evenOdd) {
-            ctx.clip('evenodd');
-          } else {
+          if (path.fillType == ui.PathFillType.nonZero) {
             ctx.clip();
+          } else {
+            ctx.clip('evenodd');
           }
         }
       }

From e7230a81af20b254c89019986f439f8a89f9c92a Mon Sep 17 00:00:00 2001
From: ferhatb <ferhat@gmail.com>
Date: Fri, 16 Oct 2020 15:50:58 -0700
Subject: [PATCH 4/5] Address reviewer comments

---
 lib/web_ui/lib/src/engine/bitmap_canvas.dart                 | 2 +-
 lib/web_ui/lib/src/engine/canvas_pool.dart                   | 2 +-
 lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart | 2 --
 lib/web_ui/test/golden_tests/engine/screenshot.dart          | 2 +-
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart
index 8a61405a5822f..b52588717183b 100644
--- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart
+++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart
@@ -275,7 +275,7 @@ class BitmapCanvas extends EngineCanvas {
     if (op == ui.ClipOp.difference) {
       // Create 2 rectangles inside each other that represents
       // clip area difference using even-odd fill rule.
-      SurfacePath path = new SurfacePath();
+      final SurfacePath path = new SurfacePath();
       path.fillType = ui.PathFillType.evenOdd;
       path.addRect(ui.Rect.fromLTWH(0, 0, _bounds.width, _bounds.height));
       path.addRect(rect);
diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart
index 62914af760086..9a297398605c5 100644
--- a/lib/web_ui/lib/src/engine/canvas_pool.dart
+++ b/lib/web_ui/lib/src/engine/canvas_pool.dart
@@ -210,7 +210,7 @@ class _CanvasPool extends _SaveStackTracking {
         } else if (clipEntry.rrect != null) {
           _clipRRect(ctx, clipEntry.rrect!);
         } else if (clipEntry.path != null) {
-          SurfacePath path = clipEntry.path as SurfacePath;
+          final SurfacePath path = clipEntry.path as SurfacePath;
           _runPath(ctx, path);
           if (path.fillType == ui.PathFillType.nonZero) {
             ctx.clip();
diff --git a/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
index b2807ff995250..8e14c37caa308 100644
--- a/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
+++ b/lib/web_ui/test/golden_tests/engine/clip_op_golden_test.dart
@@ -18,8 +18,6 @@ void testMain() async {
     debugEmulateFlutterTesterEnvironment = true;
   });
 
-  // setUpStableTestFonts();
-
   /// Regression test for https://github.com/flutter/flutter/issues/64734.
   test('Clips using difference', () async {
     final Rect region = const Rect.fromLTRB(0, 0, 400, 300);
diff --git a/lib/web_ui/test/golden_tests/engine/screenshot.dart b/lib/web_ui/test/golden_tests/engine/screenshot.dart
index f120f790f1840..81cae453c2a56 100644
--- a/lib/web_ui/test/golden_tests/engine/screenshot.dart
+++ b/lib/web_ui/test/golden_tests/engine/screenshot.dart
@@ -9,7 +9,7 @@ import 'package:ui/src/engine.dart';
 import 'package:web_engine_tester/golden_tester.dart';
 import 'package:test/test.dart';
 
-// Commit a recording canvas to a bitmap, and compare with the expected
+/// Commit a recording canvas to a bitmap, and compare with the expected.
 Future<void> canvasScreenshot(RecordingCanvas rc, String fileName,
     {ui.Rect region = const ui.Rect.fromLTWH(0, 0, 600, 800),
       double maxDiffRatePercent = 0.0, bool write: false}) async {

From 1ca9b7ae5ed636e90a5b1cb243f75aa9d92c5286 Mon Sep 17 00:00:00 2001
From: ferhatb <ferhat@gmail.com>
Date: Fri, 16 Oct 2020 15:55:03 -0700
Subject: [PATCH 5/5] update golden lock

---
 lib/web_ui/dev/goldens_lock.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml
index 66a641aed621b..1f1b59c91e98d 100644
--- a/lib/web_ui/dev/goldens_lock.yaml
+++ b/lib/web_ui/dev/goldens_lock.yaml
@@ -1,2 +1,2 @@
 repository: https://github.com/flutter/goldens.git
-revision: 672510dc52daa5b059081f6990582bccdb4ea48f
+revision: 1556280d6f1d70fac9ddff9b38639757e105b4b0