[web] Update SurfacePath to use PathRef #19557
Conversation
| static const int kInitialPointsCapacity = 8; | ||
| static const int kInitialVerbsCapacity = 8; | ||
|
|
||
| /// Returns a const pointer to the first point. |
| /// | ||
| /// Tracking whether a path is an oval is considered an | ||
| /// optimization for performance and so some paths that are in | ||
| // fact ovals can report false. |
There was a problem hiding this comment.
typo: should be a /// comment
|
|
||
| static Uint8List _fVerbsFromSource(PathRef source) { | ||
| Uint8List verbs = Uint8List(source._fVerbsCapacity); | ||
| js_util.callMethod(verbs, 'set', <dynamic>[source._fVerbs]); |
There was a problem hiding this comment.
You can use verbs.setAll(0, source._fVerbs) and dart2js will call set under the hood but without having to go through all of the overhead of JS-interop: https://github.com/dart-lang/sdk/blob/master/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart#L692
| _conicWeightsLength = source._conicWeightsLength; | ||
| if (source._conicWeights != null) { | ||
| _conicWeights = Float32List(_conicWeightsCapacity); | ||
| js_util |
There was a problem hiding this comment.
You can also use setAll here
| } | ||
| _fVerbsCapacity = source._fVerbsCapacity; | ||
| _fVerbsLength = source._fVerbsLength; | ||
| _fVerbs = Uint8List(_fVerbsCapacity); |
There was a problem hiding this comment.
I think these 2 lines are redundant because we already initialized _fVerbs = _fVerbsFromSource(source) in the initializers
| resetToSize(verbCount, pointCount, weightCount, additionalReserveVerbs, | ||
| additionalReservePoints); | ||
|
|
||
| js_util.callMethod(_fVerbs, 'set', [ref._fVerbs]); |
There was a problem hiding this comment.
ditto above re: setAll
| fSegmentMask = 0; | ||
| startEdit(); | ||
|
|
||
| _resizePoints(pointCount); |
There was a problem hiding this comment.
should we use reservePoints here and reserveVerbs below?
There was a problem hiding this comment.
Done. Thanks for finding this.
| if (numVerbs != 0) { | ||
| int curLength = countVerbs(); | ||
| _resizePoints(curLength + numVerbs); | ||
| for (int i = 0; i < numVerbs; i++) { |
There was a problem hiding this comment.
You could probably also do setAll here too
| } | ||
| final PathRefIterator iter = PathRefIterator(path.pathRef); | ||
| int verb = 0; | ||
| final Float32List outPts = Float32List(10); |
There was a problem hiding this comment.
nit: can be length 8
There was a problem hiding this comment.
Done. Added a const to PageRefIterator.
| @@ -509,69 +509,47 @@ class _CanvasPool extends _SaveStackTracking { | |||
| } | |||
| } | |||
|
|
|||
| // Float buffer used for path iteration. | |||
| static Float32List _runBuffer = Float32List(10); | |||
There was a problem hiding this comment.
nit: can be length 8
|
bench_path_recording.html.recordPathConstruction.average 4083.45 (4.12%) 4918.23 (2.12%) 0.83x |
Description
Moves SurfacePath to use skia based PathRef with points,verbs and conicWeights.
Implement convexity/winding.
Related Issues
Related: flutter/flutter#44572, flutter/flutter#57682
Tests
I added the following tests:
path_winding_test.dart.
Updated path_test and path_metrics_test.dart.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read [handling breaking changes].