diff --git a/AUTHORS b/AUTHORS index c018e61980aff..31562cfd1e843 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,4 +24,5 @@ TheOneWithTheBraid Twin Sun, LLC Qixing Cao LinXunFeng -Amir Panahandeh \ No newline at end of file +Amir Panahandeh +Tim Maffett diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index b2a143669e131..e73815f4ab96b 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4737,24 +4737,9 @@ base class Vertices extends NativeFieldWrapperClass1 { List? colors, List? textureCoordinates, List? indices, - }) { - if (colors != null && colors.length != positions.length) { - throw ArgumentError('"positions" and "colors" lengths must match.'); - } - if (textureCoordinates != null && textureCoordinates.length != positions.length) { - throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); - } - if (indices != null) { - for (int index = 0; index < indices.length; index += 1) { - if (indices[index] >= positions.length) { - throw ArgumentError( - '"indices" values must be valid indices in the positions list ' - '(i.e. numbers in the range 0..${positions.length - 1}), ' - 'but indices[$index] is ${indices[index]}, which is too big.', - ); - } - } - } + }) : assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'), + assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'), + assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.') { final Float32List encodedPositions = _encodePointList(positions); final Float32List? encodedTextureCoordinates = (textureCoordinates != null) ? _encodePointList(textureCoordinates) @@ -4821,27 +4806,10 @@ base class Vertices extends NativeFieldWrapperClass1 { Int32List? colors, Float32List? textureCoordinates, Uint16List? indices, - }) { - if (positions.length % 2 != 0) { - throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); - } - if (colors != null && colors.length * 2 != positions.length) { - throw ArgumentError('"positions" and "colors" lengths must match.'); - } - if (textureCoordinates != null && textureCoordinates.length != positions.length) { - throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); - } - if (indices != null) { - for (int index = 0; index < indices.length; index += 1) { - if (indices[index] * 2 >= positions.length) { - throw ArgumentError( - '"indices" values must be valid indices in the positions list ' - '(i.e. numbers in the range 0..${positions.length ~/ 2 - 1}), ' - 'but indices[$index] is ${indices[index]}, which is too big.', - ); - } - } - } + }) : assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'), + assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'), + assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'), + assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.') { if (!_init(this, mode.index, positions, textureCoordinates, colors, indices)) { throw ArgumentError('Invalid configuration for vertices.'); } diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index 8c53334e37d09..3b6ea03ee3a32 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -17,20 +17,9 @@ class CkVertices implements ui.Vertices { List? colors, List? indices, }) { - if (textureCoordinates != null && - textureCoordinates.length != positions.length) { - throw ArgumentError( - '"positions" and "textureCoordinates" lengths must match.'); - } - if (colors != null && colors.length != positions.length) { - throw ArgumentError('"positions" and "colors" lengths must match.'); - } - if (indices != null && - indices.any((int i) => i < 0 || i >= positions.length)) { - throw ArgumentError( - '"indices" values must be valid indices in the positions list.'); - } - + assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'); + assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'); + assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.'); return CkVertices._( toSkVertexMode(mode), toFlatSkPoints(positions), @@ -47,20 +36,10 @@ class CkVertices implements ui.Vertices { Int32List? colors, Uint16List? indices, }) { - if (textureCoordinates != null && - textureCoordinates.length != positions.length) { - throw ArgumentError( - '"positions" and "textureCoordinates" lengths must match.'); - } - if (colors != null && colors.length * 2 != positions.length) { - throw ArgumentError('"positions" and "colors" lengths must match.'); - } - if (indices != null && - indices.any((int i) => i < 0 || i >= positions.length)) { - throw ArgumentError( - '"indices" values must be valid indices in the positions list.'); - } - + assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'); + assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'); + assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'); + assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.'); Uint32List? unsignedColors; if (colors != null) { unsignedColors = colors.buffer.asUint32List(colors.offsetInBytes, colors.length); diff --git a/lib/web_ui/lib/src/engine/html/render_vertices.dart b/lib/web_ui/lib/src/engine/html/render_vertices.dart index ca5d2acacf450..3f5241814f193 100644 --- a/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -26,7 +26,9 @@ class SurfaceVertices implements ui.Vertices { List positions, { List? colors, List? indices, - }) : colors = colors != null ? _int32ListFromColors(colors) : null, + }) : assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'), + assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.'), + colors = colors != null ? _int32ListFromColors(colors) : null, indices = indices != null ? Uint16List.fromList(indices) : null, positions = offsetListToFloat32List(positions) { initWebGl(); @@ -37,7 +39,9 @@ class SurfaceVertices implements ui.Vertices { this.positions, { this.colors, this.indices, - }) { + }) : assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'), + assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'), + assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.') { initWebGl(); } diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart index e4d5142c0e767..0cc76051636e9 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart @@ -18,6 +18,9 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert List? colors, List? indices, }) => withStackScope((StackScope scope) { + assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'); + assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'); + assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.'); final RawPointArray rawPositions = scope.convertPointArrayToNative(positions); final RawPointArray rawTextureCoordinates = textureCoordinates != null ? scope.convertPointArrayToNative(textureCoordinates) @@ -47,6 +50,10 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert Int32List? colors, Uint16List? indices, }) => withStackScope((StackScope scope) { + assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'); + assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'); + assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'); + assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.'); final RawPointArray rawPositions = scope.convertDoublesToNative(positions); final RawPointArray rawTextureCoordinates = textureCoordinates != null ? scope.convertDoublesToNative(textureCoordinates) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index e7373bb0aa865..be4b6bf0d4454 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -6,6 +6,7 @@ import 'dart:typed_data'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; +import 'package:ui/src/engine.dart' show renderer; import 'package:ui/ui.dart' as ui; import 'package:web_engine_tester/golden_tester.dart'; @@ -17,6 +18,12 @@ void main() { } void testMain() { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + group('Vertices', () { setUpUnitTests(setUpTestViewDimensions: false); @@ -58,6 +65,148 @@ void testMain() { await drawPictureUsingCurrentRenderer(recorder.endRecording()); await matchGoldenFile('ui_vertices_antialiased.png', region: region); }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 + + test('Vertices assert checks', () { + // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates + if (renderer.rendererTag != 'html') { + try { + final ui.Vertices invalidVertices = ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + textureCoordinates: const [ui.Offset.zero], + ); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); + } + } + try { + final ui.Vertices invalidVertices = ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + colors: const [ui.Color.fromRGBO(255, 0, 0, 1.0)], + ); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" and \"colors\" lengths must match.')); + } + try { + final ui.Vertices invalidVertices = ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + indices: Uint16List.fromList(const [0, 2, 5]), + ); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); + } + ui.Vertices( // This one does not throw. + ui.VertexMode.triangles, + const [ui.Offset.zero], + ).dispose(); + ui.Vertices( // This one should not throw. + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + indices: Uint16List.fromList(const [0, 2, 1, 2, 0, 1, 2, 0]), // Uint16List implements List so this is ok. + ).dispose(); + }); + + test('Vertices.raw assert checks', () { + try { + final ui.Vertices invalidVertices = ui.Vertices.raw( + ui.VertexMode.triangles, + Float32List.fromList(const [0.0]), + ); + if (assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" must have an even number of entries (each coordinate is an x,y pair).')); + } + // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates + if (renderer.rendererTag != 'html') { + try { + final ui.Vertices invalidVertices = ui.Vertices.raw( + ui.VertexMode.triangles, + Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), + textureCoordinates: Float32List.fromList(const [0.0, 0.0]), + ); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); + } + } + try { + final ui.Vertices invalidVertices = ui.Vertices.raw( + ui.VertexMode.triangles, + Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), + colors: Int32List.fromList(const [0xffff0000]), + ); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"colors\" length must be half the length of \"positions\".')); + } + try { + final ui.Vertices invalidVertices = ui.Vertices.raw( + ui.VertexMode.triangles, + Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), + indices: Uint16List.fromList(const [0, 2, 5]), + ); + if (assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } else { + // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object + // we just created to check if that causes exceptions/problems. + useAndDisposeOfInvalidVerticesObject(invalidVertices); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); + } + ui.Vertices.raw( // This one does not throw. + ui.VertexMode.triangles, + Float32List.fromList(const [0.0, 0.0]), + ).dispose(); + ui.Vertices.raw( // This one should not throw. + ui.VertexMode.triangles, + Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), + indices: Uint16List.fromList(const [0, 2, 1, 2, 0, 1, 2, 0]), + ).dispose(); + }); + } ui.Vertices _testVertices() { @@ -266,3 +415,23 @@ const List _circularVertexIndices = [ 34, 36 ]; + +// This function is used when asserts are NOT enabled and vertices() allows invalid +// vertices objects to be created. Here we attempt to use these invalid ui.Vertices +// objects to determine if they cause problems within drawVertices() +void useAndDisposeOfInvalidVerticesObject( ui.Vertices vertices ) { + 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); +} diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 680f639a8481f..edd4911940eb7 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -8,16 +8,48 @@ import 'dart:ui'; import 'package:litetest/litetest.dart'; void main() { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + test('Vertices checks', () { + try { + Vertices( + VertexMode.triangles, + const [Offset.zero, Offset.zero, Offset.zero], + textureCoordinates: const [Offset.zero], + ).dispose(); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } + } on AssertionError catch (e) { + expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); + } + try { + Vertices( + VertexMode.triangles, + const [Offset.zero, Offset.zero, Offset.zero], + colors: const [Color.fromRGBO(255, 0, 0, 1.0)], + ).dispose(); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } + } on AssertionError catch (e) { + expect('$e', contains('"positions" and "colors" lengths must match.')); + } try { Vertices( VertexMode.triangles, const [Offset.zero, Offset.zero, Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), - ); - throw 'Vertices did not throw the expected error.'; - } on ArgumentError catch (e) { - expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list (i.e. numbers in the range 0..2), but indices[2] is 5, which is too big.'); + ).dispose(); + if (assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } + } on AssertionError catch (e) { + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } Vertices( // This one does not throw. VertexMode.triangles, @@ -35,20 +67,24 @@ void main() { Vertices.raw( VertexMode.triangles, Float32List.fromList(const [0.0]), - ); - throw 'Vertices.raw did not throw the expected error.'; - } on ArgumentError catch (e) { - expect('$e', 'Invalid argument(s): "positions" must have an even number of entries (each coordinate is an x,y pair).'); + ).dispose(); + if (assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } + } on AssertionError catch (e) { + expect('$e', contains('"positions" must have an even number of entries (each coordinate is an x,y pair).')); } try { Vertices.raw( VertexMode.triangles, Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), - ); - throw 'Vertices.raw did not throw the expected error.'; - } on ArgumentError catch (e) { - expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list (i.e. numbers in the range 0..2), but indices[2] is 5, which is too big.'); + ).dispose(); + if (assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } + } on AssertionError catch (e) { + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } Vertices.raw( // This one does not throw. VertexMode.triangles,