Skip to content

Commit 385878e

Browse files
[web] Gracefully handle empty ui.Vertices (#162461)
It is valid to create a `ui.Vertices` object with empty positions. This fixes Flutter Web so we don't crash when we see an empty `ui.Vertices` object. Fixes flutter/flutter#160355 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent fca7da1 commit 385878e

File tree

6 files changed

+56
-16
lines changed

6 files changed

+56
-16
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvas.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ class CkCanvas {
236236
}
237237

238238
void drawVertices(CkVertices vertices, ui.BlendMode blendMode, CkPaint paint) {
239+
if (vertices.hasNoPoints) {
240+
return;
241+
}
239242
final skPaint = paint.toSkPaint();
240243
skCanvas.drawVertices(vertices.skiaObject, toSkBlendMode(blendMode), skPaint);
241244
skPaint.delete();

engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/vertices.dart

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,30 +68,43 @@ class CkVertices implements ui.Vertices {
6868
}
6969

7070
CkVertices._(this._mode, this._positions, this._textureCoordinates, this._colors, this._indices) {
71-
final SkVertices skVertices = canvasKit.MakeVertices(
72-
_mode,
73-
_positions,
74-
_textureCoordinates,
75-
_colors,
76-
_indices,
77-
);
78-
_ref = UniqueRef<SkVertices>(this, skVertices, 'Vertices');
71+
// If [_positions] is empty, then [canvasKit.MakeVertices] will return
72+
// `null`, which breaks our JS interop. So, if we see that [_positions] is
73+
// empty, we do not create a [SkVertices] object and just treat this as
74+
// an empty vertices. Drawing an empty Vertices object is a no-op.
75+
if (_positions.isNotEmpty) {
76+
final SkVertices skVertices = canvasKit.MakeVertices(
77+
_mode,
78+
_positions,
79+
_textureCoordinates,
80+
_colors,
81+
_indices,
82+
);
83+
_ref = UniqueRef<SkVertices>(this, skVertices, 'Vertices');
84+
} else {
85+
_ref = null;
86+
}
7987
}
8088

8189
final SkVertexMode _mode;
8290
final Float32List _positions;
8391
final Float32List? _textureCoordinates;
8492
final Uint32List? _colors;
8593
final Uint16List? _indices;
86-
late final UniqueRef<SkVertices> _ref;
94+
late final UniqueRef<SkVertices>? _ref;
95+
96+
SkVertices get skiaObject => _ref!.nativeObject;
97+
98+
bool get hasNoPoints => _ref == null;
8799

88-
SkVertices get skiaObject => _ref.nativeObject;
100+
bool _isDisposed = false;
89101

90102
@override
91103
void dispose() {
92-
_ref.dispose();
104+
_ref?.dispose();
105+
_isDisposed = true;
93106
}
94107

95108
@override
96-
bool get debugDisposed => _ref.isDisposed;
109+
bool get debugDisposed => _isDisposed;
97110
}

engine/src/flutter/lib/web_ui/lib/src/engine/html/bitmap_canvas.dart

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,7 @@ class BitmapCanvas extends EngineCanvas {
938938
void drawParagraph(CanvasParagraph paragraph, ui.Offset offset) {
939939
assert(paragraph.isLaidOut);
940940

941+
// dart format off
941942
// Normally, text is composited as a plain HTML <p> tag. However, if a
942943
// bitmap canvas was used for a preceding drawing command, then it's more
943944
// efficient to continue compositing into the existing canvas, if possible.
@@ -948,13 +949,13 @@ class BitmapCanvas extends EngineCanvas {
948949
// in the first place.
949950
paragraph.canDrawOnCanvas &&
950951
// Cannot composite if there's no bitmap canvas to composite into.
951-
// Creating a new bitmap canvas just to draw text doesn't make sense.
952-
_canvasPool
953-
.hasCanvas &&
952+
// Creating a new bitmap canvas just to draw text doesn't make sense.
953+
_canvasPool.hasCanvas &&
954954
!_childOverdraw &&
955955
// Bitmap canvas introduces correctness issues in the presence of SVG
956956
// filters, so prefer plain HTML in this case.
957957
!_renderStrategy.isInsideSvgFilterTree;
958+
// dart format on
958959

959960
if (canCompositeIntoBitmapCanvas) {
960961
paragraph.paint(this, offset);
@@ -1010,6 +1011,10 @@ class BitmapCanvas extends EngineCanvas {
10101011
paint.shader == null || paint.shader is EngineImageShader,
10111012
'Linear/Radial/SweepGradient not supported yet',
10121013
);
1014+
if (vertices.hasNoPoints) {
1015+
// Drawing empty vertices is a no-op.
1016+
return;
1017+
}
10131018
final Int32List? colors = vertices.colors;
10141019
final ui.VertexMode mode = vertices.mode;
10151020
final DomCanvasRenderingContext2D ctx = _canvasPool.context;

engine/src/flutter/lib/web_ui/lib/src/engine/html/recording_canvas.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,9 @@ class RecordingCanvas {
620620
renderStrategy.hasArbitraryPaint = true;
621621
_didDraw = true;
622622
final PaintDrawVertices command = PaintDrawVertices(vertices, blendMode, paint.paintData);
623-
_growPaintBoundsByPoints(vertices.positions, 0, paint, command);
623+
if (!vertices.hasNoPoints) {
624+
_growPaintBoundsByPoints(vertices.positions, 0, paint, command);
625+
}
624626
_commands.add(command);
625627
}
626628

engine/src/flutter/lib/web_ui/lib/src/engine/html/render_vertices.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ class SurfaceVertices implements ui.Vertices {
5050
return list;
5151
}
5252

53+
bool get hasNoPoints => positions.isEmpty;
54+
5355
bool _disposed = false;
5456

5557
@override
@@ -130,6 +132,10 @@ class _WebGlRenderer implements GlRenderer {
130132
) {
131133
// Compute bounds of vertices.
132134
final Float32List positions = vertices.positions;
135+
if (positions.isEmpty) {
136+
// Drawing empty vertices is a no-op.
137+
return;
138+
}
133139
final ui.Rect bounds = _computeVerticesBounds(positions, transform);
134140
final double minValueX = bounds.left;
135141
final double minValueY = bounds.top;

engine/src/flutter/lib/web_ui/test/ui/vertices_test.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ void testMain() {
3030
vertices.dispose();
3131
expect(vertices.debugDisposed, isTrue);
3232
});
33+
34+
test('can be empty', () {
35+
final ui.Vertices vertices = ui.Vertices(ui.VertexMode.triangles, const <ui.Offset>[]);
36+
expect(vertices.debugDisposed, isFalse);
37+
38+
final ui.PictureRecorder recorder = ui.PictureRecorder();
39+
final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100));
40+
canvas.drawVertices(vertices, ui.BlendMode.srcOver, ui.Paint());
41+
vertices.dispose();
42+
expect(vertices.debugDisposed, isTrue);
43+
});
3344
});
3445

3546
test('Vertices are not anti-aliased by default', () async {

0 commit comments

Comments
 (0)