[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 https://github.com/flutter/flutter/issues/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
This commit is contained in:
parent
fca7da1eb0
commit
385878e7c1
@ -236,6 +236,9 @@ class CkCanvas {
|
||||
}
|
||||
|
||||
void drawVertices(CkVertices vertices, ui.BlendMode blendMode, CkPaint paint) {
|
||||
if (vertices.hasNoPoints) {
|
||||
return;
|
||||
}
|
||||
final skPaint = paint.toSkPaint();
|
||||
skCanvas.drawVertices(vertices.skiaObject, toSkBlendMode(blendMode), skPaint);
|
||||
skPaint.delete();
|
||||
|
@ -68,14 +68,22 @@ class CkVertices implements ui.Vertices {
|
||||
}
|
||||
|
||||
CkVertices._(this._mode, this._positions, this._textureCoordinates, this._colors, this._indices) {
|
||||
final SkVertices skVertices = canvasKit.MakeVertices(
|
||||
_mode,
|
||||
_positions,
|
||||
_textureCoordinates,
|
||||
_colors,
|
||||
_indices,
|
||||
);
|
||||
_ref = UniqueRef<SkVertices>(this, skVertices, 'Vertices');
|
||||
// If [_positions] is empty, then [canvasKit.MakeVertices] will return
|
||||
// `null`, which breaks our JS interop. So, if we see that [_positions] is
|
||||
// empty, we do not create a [SkVertices] object and just treat this as
|
||||
// an empty vertices. Drawing an empty Vertices object is a no-op.
|
||||
if (_positions.isNotEmpty) {
|
||||
final SkVertices skVertices = canvasKit.MakeVertices(
|
||||
_mode,
|
||||
_positions,
|
||||
_textureCoordinates,
|
||||
_colors,
|
||||
_indices,
|
||||
);
|
||||
_ref = UniqueRef<SkVertices>(this, skVertices, 'Vertices');
|
||||
} else {
|
||||
_ref = null;
|
||||
}
|
||||
}
|
||||
|
||||
final SkVertexMode _mode;
|
||||
@ -83,15 +91,20 @@ class CkVertices implements ui.Vertices {
|
||||
final Float32List? _textureCoordinates;
|
||||
final Uint32List? _colors;
|
||||
final Uint16List? _indices;
|
||||
late final UniqueRef<SkVertices> _ref;
|
||||
late final UniqueRef<SkVertices>? _ref;
|
||||
|
||||
SkVertices get skiaObject => _ref.nativeObject;
|
||||
SkVertices get skiaObject => _ref!.nativeObject;
|
||||
|
||||
bool get hasNoPoints => _ref == null;
|
||||
|
||||
bool _isDisposed = false;
|
||||
|
||||
@override
|
||||
void dispose() {
|
||||
_ref.dispose();
|
||||
_ref?.dispose();
|
||||
_isDisposed = true;
|
||||
}
|
||||
|
||||
@override
|
||||
bool get debugDisposed => _ref.isDisposed;
|
||||
bool get debugDisposed => _isDisposed;
|
||||
}
|
||||
|
@ -938,6 +938,7 @@ class BitmapCanvas extends EngineCanvas {
|
||||
void drawParagraph(CanvasParagraph paragraph, ui.Offset offset) {
|
||||
assert(paragraph.isLaidOut);
|
||||
|
||||
// dart format off
|
||||
// Normally, text is composited as a plain HTML <p> tag. However, if a
|
||||
// bitmap canvas was used for a preceding drawing command, then it's more
|
||||
// efficient to continue compositing into the existing canvas, if possible.
|
||||
@ -948,13 +949,13 @@ class BitmapCanvas extends EngineCanvas {
|
||||
// in the first place.
|
||||
paragraph.canDrawOnCanvas &&
|
||||
// Cannot composite if there's no bitmap canvas to composite into.
|
||||
// Creating a new bitmap canvas just to draw text doesn't make sense.
|
||||
_canvasPool
|
||||
.hasCanvas &&
|
||||
// Creating a new bitmap canvas just to draw text doesn't make sense.
|
||||
_canvasPool.hasCanvas &&
|
||||
!_childOverdraw &&
|
||||
// Bitmap canvas introduces correctness issues in the presence of SVG
|
||||
// filters, so prefer plain HTML in this case.
|
||||
!_renderStrategy.isInsideSvgFilterTree;
|
||||
// dart format on
|
||||
|
||||
if (canCompositeIntoBitmapCanvas) {
|
||||
paragraph.paint(this, offset);
|
||||
@ -1010,6 +1011,10 @@ class BitmapCanvas extends EngineCanvas {
|
||||
paint.shader == null || paint.shader is EngineImageShader,
|
||||
'Linear/Radial/SweepGradient not supported yet',
|
||||
);
|
||||
if (vertices.hasNoPoints) {
|
||||
// Drawing empty vertices is a no-op.
|
||||
return;
|
||||
}
|
||||
final Int32List? colors = vertices.colors;
|
||||
final ui.VertexMode mode = vertices.mode;
|
||||
final DomCanvasRenderingContext2D ctx = _canvasPool.context;
|
||||
|
@ -620,7 +620,9 @@ class RecordingCanvas {
|
||||
renderStrategy.hasArbitraryPaint = true;
|
||||
_didDraw = true;
|
||||
final PaintDrawVertices command = PaintDrawVertices(vertices, blendMode, paint.paintData);
|
||||
_growPaintBoundsByPoints(vertices.positions, 0, paint, command);
|
||||
if (!vertices.hasNoPoints) {
|
||||
_growPaintBoundsByPoints(vertices.positions, 0, paint, command);
|
||||
}
|
||||
_commands.add(command);
|
||||
}
|
||||
|
||||
|
@ -50,6 +50,8 @@ class SurfaceVertices implements ui.Vertices {
|
||||
return list;
|
||||
}
|
||||
|
||||
bool get hasNoPoints => positions.isEmpty;
|
||||
|
||||
bool _disposed = false;
|
||||
|
||||
@override
|
||||
@ -130,6 +132,10 @@ class _WebGlRenderer implements GlRenderer {
|
||||
) {
|
||||
// Compute bounds of vertices.
|
||||
final Float32List positions = vertices.positions;
|
||||
if (positions.isEmpty) {
|
||||
// Drawing empty vertices is a no-op.
|
||||
return;
|
||||
}
|
||||
final ui.Rect bounds = _computeVerticesBounds(positions, transform);
|
||||
final double minValueX = bounds.left;
|
||||
final double minValueY = bounds.top;
|
||||
|
@ -30,6 +30,17 @@ void testMain() {
|
||||
vertices.dispose();
|
||||
expect(vertices.debugDisposed, isTrue);
|
||||
});
|
||||
|
||||
test('can be empty', () {
|
||||
final ui.Vertices vertices = ui.Vertices(ui.VertexMode.triangles, const <ui.Offset>[]);
|
||||
expect(vertices.debugDisposed, isFalse);
|
||||
|
||||
final ui.PictureRecorder recorder = ui.PictureRecorder();
|
||||
final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100));
|
||||
canvas.drawVertices(vertices, ui.BlendMode.srcOver, ui.Paint());
|
||||
vertices.dispose();
|
||||
expect(vertices.debugDisposed, isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
test('Vertices are not anti-aliased by default', () async {
|
||||
|
Loading…
x
Reference in New Issue
Block a user