From c44797fbb4d18e09b39fc322f897cfdd0fb6d552 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 7 Jan 2020 17:27:58 -0800 Subject: [PATCH 1/3] Fix Path.from to deep copy. Add regression test. --- .../lib/src/engine/surface/painting.dart | 18 ++++++++++++++++++ .../src/engine/surface/recording_canvas.dart | 3 ++- lib/web_ui/test/path_test.dart | 13 +++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/surface/painting.dart b/lib/web_ui/lib/src/engine/surface/painting.dart index 0322cfac58c4f..bdaf40dbf61ff 100644 --- a/lib/web_ui/lib/src/engine/surface/painting.dart +++ b/lib/web_ui/lib/src/engine/surface/painting.dart @@ -287,10 +287,28 @@ class SurfacePath implements ui.Path { /// This copy is fast and does not require additional memory unless either /// the `source` path or the path returned by this constructor are modified. SurfacePath.from(SurfacePath source) + : subpaths = _deepCopy(source.subpaths); + + SurfacePath._shallowCopy(SurfacePath source) : subpaths = List.from(source.subpaths); SurfacePath._clone(this.subpaths, this._fillType); + static List _deepCopy(List source) { + // The last sub path can potentially still be mutated by calling ops. + // Copy all sub paths except the last active one which needs a deep copy. + final List paths = []; + int len = source.length; + if (len != 0) { + --len; + for (int i = 0; i < len; i++) { + paths.add(source[i]); + } + paths.add(source[len].shift(const ui.Offset(0, 0))); + } + return paths; + } + /// Determines how the interior of this path is calculated. /// /// Defaults to the non-zero winding rule, [PathFillType.nonZero]. diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index f964323c49142..916d20ae9c88e 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -309,7 +309,8 @@ class RecordingCanvas { } _paintBounds.grow(pathBounds); // Clone path so it can be reused for subsequent draw calls. - final ui.Path clone = ui.Path.from(path); + final ui.Path clone = (experimentalUseSkia) ? + ui.Path.from(path) : SurfacePath._shallowCopy(path); clone.fillType = path.fillType; _commands.add(PaintDrawPath(clone, paint.paintData)); } diff --git a/lib/web_ui/test/path_test.dart b/lib/web_ui/test/path_test.dart index 7b4a062752538..c61a034b27924 100644 --- a/lib/web_ui/test/path_test.dart +++ b/lib/web_ui/test/path_test.dart @@ -238,4 +238,17 @@ void main() { distance: 0.1, from: const Rect.fromLTRB(220.0, 124.1, 382.9, 300.0))); }); + + test('Should deep copy path', () { + final SurfacePath path = SurfacePath(); + path.moveTo(25, 30); + path.lineTo(100, 200); + expect(path.getBounds(), const Rect.fromLTRB(25, 30, 100, 200)); + + final SurfacePath path2 = SurfacePath.from(path); + path2.lineTo(250, 300); + expect(path2.getBounds(), const Rect.fromLTRB(25, 30, 250, 300)); + // Expect original path to stay the same. + expect(path.getBounds(), const Rect.fromLTRB(25, 30, 100, 200)); + }); } From f6481852abaebd4b50b14a69973a1905e96a58b2 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 7 Jan 2020 17:28:17 -0800 Subject: [PATCH 2/3] update comment on test --- lib/web_ui/test/path_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/web_ui/test/path_test.dart b/lib/web_ui/test/path_test.dart index c61a034b27924..d5458bd94181b 100644 --- a/lib/web_ui/test/path_test.dart +++ b/lib/web_ui/test/path_test.dart @@ -239,6 +239,7 @@ void main() { from: const Rect.fromLTRB(220.0, 124.1, 382.9, 300.0))); }); + // Regression test for https://github.com/flutter/flutter/issues/46813. test('Should deep copy path', () { final SurfacePath path = SurfacePath(); path.moveTo(25, 30); From fde9f61359017d24a5b69373091e34e10ba3f760 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Jan 2020 11:14:10 -0800 Subject: [PATCH 3/3] Don't check for CanvasKit in recording_canvas since it uses picture recorder --- lib/web_ui/lib/src/engine/surface/recording_canvas.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 916d20ae9c88e..b6fb8f1a059f6 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -309,8 +309,7 @@ class RecordingCanvas { } _paintBounds.grow(pathBounds); // Clone path so it can be reused for subsequent draw calls. - final ui.Path clone = (experimentalUseSkia) ? - ui.Path.from(path) : SurfacePath._shallowCopy(path); + final ui.Path clone = SurfacePath._shallowCopy(path); clone.fillType = path.fillType; _commands.add(PaintDrawPath(clone, paint.paintData)); }