From 70a0afd240c33cf465dd1dab44311fb04612951a Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 12 Apr 2023 14:46:46 -0700 Subject: [PATCH] [web:canvaskit] move path API to UniqueRef --- .../src/engine/canvaskit/native_memory.dart | 2 +- lib/web_ui/lib/src/engine/canvaskit/path.dart | 56 +++++------ .../src/engine/canvaskit/path_metrics.dart | 96 ++++--------------- lib/web_ui/test/canvaskit/path_test.dart | 37 +------ 4 files changed, 46 insertions(+), 145 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart b/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart index a4f0648a913a5..a8423620a1d08 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart @@ -54,7 +54,7 @@ class UniqueRef { /// The returned reference must not be stored. I should only be borrowed /// temporarily. Storing this reference may result in dangling pointer errors. T get nativeObject { - assert(!isDisposed, 'Native object was disposed.'); + assert(!isDisposed, 'The native object of $_debugOwnerLabel was disposed.'); return _nativeObject!; } diff --git a/lib/web_ui/lib/src/engine/canvaskit/path.dart b/lib/web_ui/lib/src/engine/canvaskit/path.dart index fe60fc2837194..d7bf7d088d4bf 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/path.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/path.dart @@ -9,25 +9,38 @@ import 'package:ui/ui.dart' as ui; import '../vector_math.dart'; import 'canvaskit_api.dart'; +import 'native_memory.dart'; import 'path_metrics.dart'; -import 'skia_object_cache.dart'; /// An implementation of [ui.Path] which is backed by an `SkPath`. /// /// The `SkPath` is required for `CkCanvas` methods which take a path. -class CkPath extends ManagedSkiaObject implements ui.Path { - CkPath() : _fillType = ui.PathFillType.nonZero; +class CkPath implements ui.Path { + factory CkPath() { + final SkPath skPath = SkPath(); + skPath.setFillType(toSkFillType(ui.PathFillType.nonZero)); + return CkPath._(skPath, ui.PathFillType.nonZero); + } + + factory CkPath.from(CkPath other) { + final SkPath skPath = other.skiaObject.copy(); + skPath.setFillType(toSkFillType(other._fillType)); + return CkPath._(skPath, other._fillType); + } - CkPath.from(CkPath other) - : _fillType = other.fillType, - super(other.skiaObject.copy()) { - skiaObject.setFillType(toSkFillType(_fillType)); + factory CkPath.fromSkPath(SkPath skPath, ui.PathFillType fillType) { + skPath.setFillType(toSkFillType(fillType)); + return CkPath._(skPath, fillType); } - CkPath.fromSkPath(SkPath super.skPath, this._fillType) { - skiaObject.setFillType(toSkFillType(_fillType)); + CkPath._(SkPath nativeObject, this._fillType) { + _ref = UniqueRef(this, nativeObject, 'Path'); } + late final UniqueRef _ref; + + SkPath get skiaObject => _ref.nativeObject; + ui.PathFillType _fillType; @override @@ -309,29 +322,4 @@ class CkPath extends ManagedSkiaObject implements ui.Path { bool get isEmpty { return skiaObject.isEmpty(); } - - @override - bool get isResurrectionExpensive => true; - - @override - SkPath createDefault() { - final SkPath path = SkPath(); - path.setFillType(toSkFillType(_fillType)); - return path; - } - - late List _cachedCommands; - - @override - void delete() { - _cachedCommands = skiaObject.toCmds(); - rawSkiaObject?.delete(); - } - - @override - SkPath resurrect() { - final SkPath path = canvasKit.Path.MakeFromCmds(_cachedCommands); - path.setFillType(toSkFillType(_fillType)); - return path; - } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart b/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart index f47d08d400c29..344fabad329ff 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart @@ -8,11 +8,10 @@ import 'dart:typed_data'; import 'package:ui/ui.dart' as ui; import 'canvaskit_api.dart'; +import 'native_memory.dart'; import 'path.dart'; -import 'skia_object_cache.dart'; -class CkPathMetrics extends IterableBase - implements ui.PathMetrics { +class CkPathMetrics extends IterableBase implements ui.PathMetrics { CkPathMetrics(this._path, this._forceClosed); final CkPath _path; @@ -23,18 +22,21 @@ class CkPathMetrics extends IterableBase late final Iterator iterator = _path.isEmpty ? const CkPathMetricIteratorEmpty._() : CkContourMeasureIter(this); - - /// A fresh [CkContourMeasureIter] which is only used for resurrecting a - /// [CkContourMeasure]. We can't use [iterator] here because [iterator] is - /// memoized. - CkContourMeasureIter _iteratorForResurrection() => CkContourMeasureIter(this); } -class CkContourMeasureIter extends ManagedSkiaObject - implements Iterator { - CkContourMeasureIter(this._metrics); +class CkContourMeasureIter implements Iterator { + CkContourMeasureIter(this._metrics) { + _ref = UniqueRef(this, SkContourMeasureIter( + _metrics._path.skiaObject, + _metrics._forceClosed, + 1.0, + ), 'Iterator'); + } final CkPathMetrics _metrics; + late final UniqueRef _ref; + + SkContourMeasureIter get skiaObject => _ref.nativeObject; /// A monotonically increasing counter used to generate [ui.PathMetric.contourIndex]. /// @@ -68,45 +70,22 @@ class CkContourMeasureIter extends ManagedSkiaObject _contourIndexCounter += 1; return true; } - - @override - SkContourMeasureIter createDefault() { - return SkContourMeasureIter( - _metrics._path.skiaObject, - _metrics._forceClosed, - 1.0, - ); - } - - @override - SkContourMeasureIter resurrect() { - final SkContourMeasureIter iterator = createDefault(); - - // When resurrecting we must advance the iterator to the last known - // position. - for (int i = 0; i < _contourIndexCounter; i++) { - iterator.next(); - } - - return iterator; - } - - @override - void delete() { - rawSkiaObject?.delete(); - } } -class CkContourMeasure extends ManagedSkiaObject - implements ui.PathMetric { - CkContourMeasure(this._metrics, SkContourMeasure jsObject, this.contourIndex) - : super(jsObject); +class CkContourMeasure implements ui.PathMetric { + CkContourMeasure(this._metrics, SkContourMeasure skiaObject, this.contourIndex) { + _ref = UniqueRef(this, skiaObject, 'PathMetric'); + } /// The path metrics used to create this measure. /// /// This is used to resurrect the object if it is deleted prematurely. final CkPathMetrics _metrics; + late final UniqueRef _ref; + + SkContourMeasure get skiaObject => _ref.nativeObject; + @override final int contourIndex; @@ -134,39 +113,6 @@ class CkContourMeasure extends ManagedSkiaObject double get length { return skiaObject.length(); } - - @override - SkContourMeasure createDefault() { - // This method must never be called. The default instance comes from the - // iterator's [SkContourMeasureIter.next] method initialized by the - // constructor. - throw StateError('Unreachable code'); - } - - @override - SkContourMeasure resurrect() { - final CkContourMeasureIter iterator = _metrics._iteratorForResurrection(); - final SkContourMeasureIter skIterator = iterator.skiaObject; - - // When resurrecting we must advance the iterator to the last known - // position. - for (int i = 0; i < contourIndex; i++) { - skIterator.next(); - } - - final SkContourMeasure? result = skIterator.next(); - - if (result == null) { - throw StateError('Failed to resurrect SkContourMeasure'); - } - - return result; - } - - @override - void delete() { - rawSkiaObject?.delete(); - } } class CkPathMetricIteratorEmpty implements Iterator { diff --git a/lib/web_ui/test/canvaskit/path_test.dart b/lib/web_ui/test/canvaskit/path_test.dart index bad26e4a9e2d8..e3f77f9f2cd83 100644 --- a/lib/web_ui/test/canvaskit/path_test.dart +++ b/lib/web_ui/test/canvaskit/path_test.dart @@ -94,17 +94,7 @@ void testMain() { expect(path.getBounds(), testRect); }); - test('CkPath resurrection', () { - const ui.Rect rect = ui.Rect.fromLTRB(0, 0, 10, 10); - final CkPath path = CkPath(); - path.addRect(rect); - path.delete(); - - final SkPath resurrectedCopy = path.resurrect(); - expect(fromSkRect(resurrectedCopy.getBounds()), rect); - }); - - test('Resurrect CkContourMeasure in the middle of iteration', () { + test('CkContourMeasure iteration', () { browserSupportsFinalizationRegistry = false; final ui.Path path = ui.Path(); expect(path, isA()); @@ -119,18 +109,9 @@ void testMain() { expect(iterator.current.contourIndex, 0); expect(iterator.moveNext(), isTrue); expect(iterator.current.contourIndex, 1); - - // Delete iterator in the middle of iteration - iterator.delete(); - iterator.rawSkiaObject = null; - - // Check that the iterator can continue from the last position. - expect(iterator.moveNext(), isTrue); - expect(iterator.current.contourIndex, 2); - expect(iterator.moveNext(), isFalse); }); - test('Resurrect CkContourMeasure', () { + test('CkContourMeasure index', () { browserSupportsFinalizationRegistry = false; final ui.Path path = ui.Path(); expect(path, isA()); @@ -150,20 +131,6 @@ void testMain() { final CkContourMeasure measure1 = iterator.current as CkContourMeasure; expect(measure1.contourIndex, 1); expect(measure1.extractPath(0, 15).getBounds(), const ui.Rect.fromLTRB(20, 20, 30, 25)); - - // Delete iterator and the measure in the middle of iteration - iterator.delete(); - iterator.rawSkiaObject = null; - measure0.delete(); - measure0.rawSkiaObject = null; - measure1.delete(); - measure1.rawSkiaObject = null; - - // Check that the measure is still value after resurrection. - expect(measure0.contourIndex, 0); - expect(measure0.extractPath(0, 15).getBounds(), const ui.Rect.fromLTRB(0, 0, 10, 5)); - expect(measure1.contourIndex, 1); - expect(measure1.extractPath(0, 15).getBounds(), const ui.Rect.fromLTRB(20, 20, 30, 25)); }); test('Path.from', () {