From 5aa29135779f59869d925c2d4220cde339e72ca1 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 4 May 2023 22:32:04 -0700 Subject: [PATCH 01/34] avoid unnecessary operations during indices check --- lib/ui/painting.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 8cbacfc328565..69adfe51604dd 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4714,8 +4714,10 @@ class Vertices extends NativeFieldWrapperClass1 { throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); } if (indices != null) { + final int halfPositions = positions.length>>1; // len/2 - we already checked that positions.length is even for (int index = 0; index < indices.length; index += 1) { - if (indices[index] * 2 >= positions.length) { + // we need to check that `indices[index] * 2 >= positions.length`, so do it without multiplying + if (indices[index] >= halfPositions) { throw ArgumentError( '"indices" values must be valid indices in the positions list ' '(i.e. numbers in the range 0..${positions.length ~/ 2 - 1}), ' From 57781a2ea56cc529c3b289f00c16071a0c020fa1 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 4 May 2023 23:17:02 -0700 Subject: [PATCH 02/34] fix indices check in raw() to correctly check >1; if (indices != null && - indices.any((int i) => i < 0 || i >= positions.length)) { + indices.any((int i) => i < 0 || i >= halfPositions)) { throw ArgumentError( '"indices" values must be valid indices in the positions list.'); } From e80a3dc73d810770e85d97ba538f9f8408e976e1 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 4 May 2023 23:20:55 -0700 Subject: [PATCH 03/34] formatting --- lib/ui/painting.dart | 2 +- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 69adfe51604dd..a58a1989d212c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4714,7 +4714,7 @@ class Vertices extends NativeFieldWrapperClass1 { throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); } if (indices != null) { - final int halfPositions = positions.length>>1; // len/2 - we already checked that positions.length is even + final int halfPositions = positions.length >> 1; // len/2 - we already checked that positions.length is even for (int index = 0; index < indices.length; index += 1) { // we need to check that `indices[index] * 2 >= positions.length`, so do it without multiplying if (indices[index] >= halfPositions) { diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index 20661185fcbd3..7711b3b277816 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -57,7 +57,7 @@ class CkVertices implements ui.Vertices { } // see ui.Vertices.raw() - all indices must be <(positions.length/2) because they refer to // pairs in the positions array. - final halfPositions = positions.length>>1; + final int halfPositions = positions.length >> 1; if (indices != null && indices.any((int i) => i < 0 || i >= halfPositions)) { throw ArgumentError( From 118a38a08ce1032eaac85f0ae42d28e3fe1f4d22 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 4 May 2023 23:57:33 -0700 Subject: [PATCH 04/34] add tests for ckVertices doing proper parameter checks --- lib/web_ui/test/canvaskit/vertices_test.dart | 54 ++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/lib/web_ui/test/canvaskit/vertices_test.dart b/lib/web_ui/test/canvaskit/vertices_test.dart index a070522c5fc91..ce13d441b0e3d 100644 --- a/lib/web_ui/test/canvaskit/vertices_test.dart +++ b/lib/web_ui/test/canvaskit/vertices_test.dart @@ -66,6 +66,60 @@ void testMain() { .draw(builder.build().layerTree); await matchGoldenFile('canvaskit_vertices_antialiased.png', region: region); }, skip: isSafari); + + test('Vertices checks', () { + try { + ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.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.'); + } + 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 checks', () { + try { + ui.Vertices.raw( + ui.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).'); + } + try { + 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]), + ); + throw 'Vertices.raw did not throw the expected error.'; + } on ArgumentError catch (e) { + expect('$e', '"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(); + }); + } CkVertices _testVertices() { From d692f0f17705800fec5e778fc170bf6b847388fb Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 5 May 2023 00:03:54 -0700 Subject: [PATCH 05/34] add positions.length is even check to CkVertices.raw() --- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index 7711b3b277816..bc893c651ac3e 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -47,6 +47,9 @@ class CkVertices implements ui.Vertices { Int32List? colors, 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 (textureCoordinates != null && textureCoordinates.length != positions.length) { throw ArgumentError( From 046a7294734ff64da49eec30a76d439b9521ebb0 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 5 May 2023 08:45:56 -0700 Subject: [PATCH 06/34] made throws in vertices_test.dart throw ArgumentError to make linux analyze happy --- lib/web_ui/test/canvaskit/vertices_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/test/canvaskit/vertices_test.dart b/lib/web_ui/test/canvaskit/vertices_test.dart index ce13d441b0e3d..8b4c5910979f2 100644 --- a/lib/web_ui/test/canvaskit/vertices_test.dart +++ b/lib/web_ui/test/canvaskit/vertices_test.dart @@ -74,7 +74,7 @@ void testMain() { const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - throw 'Vertices did not throw the expected error.'; + throw ArgumentError('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.'); } @@ -95,7 +95,7 @@ void testMain() { ui.VertexMode.triangles, Float32List.fromList(const [0.0]), ); - throw 'Vertices.raw did not throw the expected error.'; + throw ArgumentError('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).'); } @@ -105,7 +105,7 @@ void testMain() { 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.'; + throw ArgumentError('Vertices.raw did not throw the expected error.'); } on ArgumentError catch (e) { expect('$e', '"indices" values must be valid indices in the positions list.'); } From 930d9ea5e488ce2750ac8d3412af95d430f69ea7 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 5 May 2023 10:40:50 -0700 Subject: [PATCH 07/34] fix tests expected error text --- lib/web_ui/test/canvaskit/vertices_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/vertices_test.dart b/lib/web_ui/test/canvaskit/vertices_test.dart index 8b4c5910979f2..f94842427c435 100644 --- a/lib/web_ui/test/canvaskit/vertices_test.dart +++ b/lib/web_ui/test/canvaskit/vertices_test.dart @@ -107,7 +107,7 @@ void testMain() { ); throw ArgumentError('Vertices.raw did not throw the expected error.'); } on ArgumentError catch (e) { - expect('$e', '"indices" values must be valid indices in the positions list.'); + expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list.'); } ui.Vertices.raw( // This one does not throw. ui.VertexMode.triangles, From fe4db2b499ab284df33860d60c3768e46a6ad5e9 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Tue, 16 May 2023 15:10:12 -0700 Subject: [PATCH 08/34] changed %2!=0 check to use isOdd --- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index bc893c651ac3e..d80a88ad24622 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -47,7 +47,7 @@ class CkVertices implements ui.Vertices { Int32List? colors, Uint16List? indices, }) { - if (positions.length % 2 != 0) { + if (positions.length.isOdd) { throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); } if (textureCoordinates != null && From e15299a5cd70be291c9441c5533fe044d161e6e1 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 18:48:48 -0700 Subject: [PATCH 09/34] change vertices_test.dart to only check CkVertices for argument checks, SkWasm doesnt check args --- lib/web_ui/test/ui/vertices_test.dart | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index e90e57f0cc05b..6a39196cc55fa 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'; import 'package:ui/ui.dart' as ui; import 'package:web_engine_tester/golden_tester.dart'; @@ -59,9 +60,9 @@ void testMain() { await matchGoldenFile('ui_vertices_antialiased.png', region: region); }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 - test('Vertices checks', () { + test('Vertices checks (canvas kit only)', () { try { - ui.Vertices( + CkVertices( ui.VertexMode.triangles, const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), @@ -70,20 +71,20 @@ void testMain() { } on ArgumentError catch (e) { expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list.'); } - ui.Vertices( // This one does not throw. + CkVertices( // This one does not throw. ui.VertexMode.triangles, const [ui.Offset.zero], ).dispose(); - ui.Vertices( // This one should not throw. + CkVertices( // 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 checks', () { + test('Vertices.raw checks (canvas kit only)', () { try { - ui.Vertices.raw( + CkVertices.raw( ui.VertexMode.triangles, Float32List.fromList(const [0.0]), ); @@ -92,7 +93,7 @@ void testMain() { expect('$e', 'Invalid argument(s): "positions" must have an even number of entries (each coordinate is an x,y pair).'); } try { - ui.Vertices.raw( + CkVertices.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]), @@ -101,11 +102,11 @@ void testMain() { } on ArgumentError catch (e) { expect('$e', 'Invalid argument(s): "indices" values must be valid indices in the positions list.'); } - ui.Vertices.raw( // This one does not throw. + CkVertices.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. + CkVertices.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]), From 702ff2299ff1713465c81e20643df5425c57638f Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 19:59:10 -0700 Subject: [PATCH 10/34] change web_ui vertices checks to asserts and add to skwasm_impl --- lib/ui/painting.dart | 2 +- .../lib/src/engine/canvaskit/vertices.dart | 30 +++------ .../engine/skwasm/skwasm_impl/vertices.dart | 7 ++ lib/web_ui/test/ui/vertices_test.dart | 67 +++++++++++++------ testing/dart/painting_test.dart | 20 ++++++ 5 files changed, 85 insertions(+), 41 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index d125efe775804..77a15074b2e1f 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4833,7 +4833,7 @@ base class Vertices extends NativeFieldWrapperClass1 { 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.'); + throw ArgumentError('"colors" length must be half the length of "positions".'); } if (textureCoordinates != null && textureCoordinates.length != positions.length) { throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index d80a88ad24622..f0f5486f677d6 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -17,6 +17,10 @@ class CkVertices implements ui.Vertices { List? colors, List? indices, }) { + 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.any((int i) => i<0 && i>=positions.length),'"indices" values must be valid indices in the positions list.'); + /*BEFORE ASSERTS if (textureCoordinates != null && textureCoordinates.length != positions.length) { throw ArgumentError( @@ -30,7 +34,7 @@ class CkVertices implements ui.Vertices { throw ArgumentError( '"indices" values must be valid indices in the positions list.'); } - + */ return CkVertices._( toSkVertexMode(mode), toFlatSkPoints(positions), @@ -47,26 +51,10 @@ class CkVertices implements ui.Vertices { Int32List? colors, Uint16List? indices, }) { - if (positions.length.isOdd) { - throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); - } - 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.'); - } - // see ui.Vertices.raw() - all indices must be <(positions.length/2) because they refer to - // pairs in the positions array. - final int halfPositions = positions.length >> 1; - if (indices != null && - indices.any((int i) => i < 0 || i >= halfPositions)) { - 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.any((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/skwasm/skwasm_impl/vertices.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart index e4d5142c0e767..73c288c438a74 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.any((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.any((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 6a39196cc55fa..90704c70fa4e8 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -6,7 +6,6 @@ import 'dart:typed_data'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; -import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; import 'package:web_engine_tester/golden_tester.dart'; @@ -60,53 +59,83 @@ void testMain() { await matchGoldenFile('ui_vertices_antialiased.png', region: region); }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 - test('Vertices checks (canvas kit only)', () { + test('Vertices assert checks', () { try { - CkVertices( + ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + textureCoordinates: const [ui.Offset.zero], + ); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); + } + try { + 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)], + ); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"colors" length must be half the length of "positions".')); + } + try { + ui.Vertices( ui.VertexMode.triangles, const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - throw ArgumentError('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.'); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } - CkVertices( // This one does not throw. + ui.Vertices( // This one does not throw. ui.VertexMode.triangles, const [ui.Offset.zero], ).dispose(); - CkVertices( // This one should not throw. + 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 checks (canvas kit only)', () { + test('Vertices.raw assert checks', () { try { - CkVertices.raw( + ui.Vertices.raw( ui.VertexMode.triangles, Float32List.fromList(const [0.0]), ); - throw ArgumentError('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).'); + throw AssertionError('Vertices.raw did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"positions" must have an even number of entries (each coordinate is an x,y pair).')); } try { - CkVertices.raw( + ui.Vertices( + ui.VertexMode.triangles, + const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], + colors: const [ui.Color.fromRGBO(255, 0, 0, 1.0)], + ); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"colors" length must be half the length of "positions".')); + } + try { + 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]), ); - throw ArgumentError('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.'); + throw AssertionError('Vertices.raw did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } - CkVertices.raw( // This one does not throw. + ui.Vertices.raw( // This one does not throw. ui.VertexMode.triangles, Float32List.fromList(const [0.0, 0.0]), ).dispose(); - CkVertices.raw( // This one should not throw. + 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]), diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 680f639a8481f..bb5b8d6a4685c 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -9,6 +9,26 @@ import 'package:litetest/litetest.dart'; void main() { test('Vertices checks', () { + try { + Vertices( + VertexMode.triangles, + const [Offset.zero, Offset.zero, Offset.zero], + textureCoordinates: const [Offset.zero], + ); + throw AssertionError('Vertices did not throw the expected 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)], + ); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"positions" and "colors" lengths must match.')); + } try { Vertices( VertexMode.triangles, From cfcf8490e0160aa3412891d53124baadb4e89f56 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 20:47:09 -0700 Subject: [PATCH 11/34] fix catch for ArgumentError --- testing/dart/painting_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index bb5b8d6a4685c..1ed9f29f07165 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -15,8 +15,8 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], textureCoordinates: const [Offset.zero], ); - throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e) { + throw ArgumentError('Vertices did not throw the expected error.'); + } on ArgumentError catch (e) { expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); } try { @@ -25,8 +25,8 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], colors: const [Color.fromRGBO(255, 0, 0, 1.0)], ); - throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e) { + throw ArgumentError('Vertices did not throw the expected error.'); + } on ArgumentError catch (e) { expect('$e', contains('"positions" and "colors" lengths must match.')); } try { From 217de1d211456746fe49849969ca82b603bfb2a7 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 21:06:19 -0700 Subject: [PATCH 12/34] added vertices checks to html render_vertices.dart, formatting --- lib/web_ui/lib/src/engine/html/render_vertices.dart | 8 ++++++-- lib/web_ui/test/ui/vertices_test.dart | 6 +++--- testing/dart/painting_test.dart | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) 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..ec7b04d40c686 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.any((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.any((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/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 90704c70fa4e8..b95fef370ad50 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -69,7 +69,7 @@ void testMain() { throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); - } + } try { ui.Vertices( ui.VertexMode.triangles, @@ -79,7 +79,7 @@ void testMain() { throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { expect('$e', contains('"colors" length must be half the length of "positions".')); - } + } try { ui.Vertices( ui.VertexMode.triangles, @@ -120,7 +120,7 @@ void testMain() { throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { expect('$e', contains('"colors" length must be half the length of "positions".')); - } + } try { ui.Vertices.raw( ui.VertexMode.triangles, diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 1ed9f29f07165..38d2a7a73b593 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -18,7 +18,7 @@ void main() { throw ArgumentError('Vertices did not throw the expected error.'); } on ArgumentError catch (e) { expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); - } + } try { Vertices( VertexMode.triangles, @@ -28,7 +28,7 @@ void main() { throw ArgumentError('Vertices did not throw the expected error.'); } on ArgumentError catch (e) { expect('$e', contains('"positions" and "colors" lengths must match.')); - } + } try { Vertices( VertexMode.triangles, From 8e3dcec27cc5855bff7bc9c0fc02f2ac784015e0 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 22:10:18 -0700 Subject: [PATCH 13/34] tweaks to tests to properly identify exception text --- lib/web_ui/test/ui/vertices_test.dart | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index b95fef370ad50..1ed1eac0eae43 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -60,6 +60,7 @@ void testMain() { }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 test('Vertices assert checks', () { + /* HTML renderer's SurfaceVertices() does not support textureCoordinates, so we cant test this universally across web try { ui.Vertices( ui.VertexMode.triangles, @@ -68,8 +69,9 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); + expect('$e', contains(r'\\"positions\\" and \\"textureCoordinates\\" lengths must match.')); } + */ try { ui.Vertices( ui.VertexMode.triangles, @@ -78,7 +80,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"colors" length must be half the length of "positions".')); + expect('$e', contains(r'\\"colors\\" length must be half the length of "positions".')); } try { ui.Vertices( @@ -88,7 +90,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"indices" values must be valid indices in the positions list.')); + expect('$e', contains(r'\\"indices\\" values must be valid indices in the positions list.')); } ui.Vertices( // This one does not throw. ui.VertexMode.triangles, @@ -109,7 +111,7 @@ void testMain() { ); throw AssertionError('Vertices.raw did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"positions" must have an even number of entries (each coordinate is an x,y pair).')); + expect('$e', contains(r'\\"positions\\" must have an even number of entries (each coordinate is an x,y pair).')); } try { ui.Vertices( @@ -119,7 +121,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"colors" length must be half the length of "positions".')); + expect('$e', contains(r'\\"colors\\" length must be half the length of "positions".')); } try { ui.Vertices.raw( @@ -129,7 +131,7 @@ void testMain() { ); throw AssertionError('Vertices.raw did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"indices" values must be valid indices in the positions list.')); + 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, From 0b2f9407280fe38ea0665e397000cf815de15bdb Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 1 Jun 2023 22:40:32 -0700 Subject: [PATCH 14/34] fix trailing space --- lib/web_ui/test/ui/vertices_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 1ed1eac0eae43..d33e291888752 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -60,7 +60,7 @@ void testMain() { }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 test('Vertices assert checks', () { - /* HTML renderer's SurfaceVertices() does not support textureCoordinates, so we cant test this universally across web + /* HTML renderer's SurfaceVertices() does not support textureCoordinates, so we cant test this universally across web try { ui.Vertices( ui.VertexMode.triangles, From 1ad8364b318f3b6fe4d64d13abf8b6de31bc7abc Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 07:21:46 -0700 Subject: [PATCH 15/34] match assertionerror extra slashes --- lib/web_ui/test/ui/vertices_test.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index d33e291888752..0a51a1c79fa91 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -69,7 +69,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"positions\\" and \\"textureCoordinates\\" lengths must match.')); + expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); } */ try { @@ -80,7 +80,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"colors\\" length must be half the length of "positions".')); + expect('$e', contains(r'\"colors\" length must be half the length of "positions".')); } try { ui.Vertices( @@ -90,7 +90,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"indices\\" values must be valid indices in the positions list.')); + expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); } ui.Vertices( // This one does not throw. ui.VertexMode.triangles, @@ -111,7 +111,7 @@ void testMain() { ); throw AssertionError('Vertices.raw did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"positions\\" must have an even number of entries (each coordinate is an x,y pair).')); + expect('$e', contains(r'\"positions\" must have an even number of entries (each coordinate is an x,y pair).')); } try { ui.Vertices( @@ -121,7 +121,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"colors\\" length must be half the length of "positions".')); + expect('$e', contains(r'\"colors\" length must be half the length of "positions".')); } try { ui.Vertices.raw( @@ -131,7 +131,7 @@ void testMain() { ); throw AssertionError('Vertices.raw did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\\"indices\\" values must be valid indices in the positions list.')); + 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, From 010c78ae17374908c92e9909acb4abd600549553 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 07:25:04 -0700 Subject: [PATCH 16/34] correct assertion match --- lib/web_ui/test/ui/vertices_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 0a51a1c79fa91..ec812d13628cb 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -80,7 +80,7 @@ void testMain() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains(r'\"colors\" length must be half the length of "positions".')); + expect('$e', contains(r'\"positions\" and \"colors\" lengths must match.')); } try { ui.Vertices( From 1bc170a93ab6dcccb2adc7d25837417041db7b48 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 08:07:57 -0700 Subject: [PATCH 17/34] fix vertices.raw test --- lib/web_ui/test/ui/vertices_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index ec812d13628cb..45a07359aae3a 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -114,10 +114,10 @@ void testMain() { expect('$e', contains(r'\"positions\" must have an even number of entries (each coordinate is an x,y pair).')); } try { - ui.Vertices( + ui.Vertices.raw( ui.VertexMode.triangles, - const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], - colors: const [ui.Color.fromRGBO(255, 0, 0, 1.0)], + Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), + colors: Int32List.fromList(const [0xffff0000]), ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { From edf7ba0921713b3b7ac99f193c85c58b7d19925c Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 08:56:37 -0700 Subject: [PATCH 18/34] added debug code to print stacktrace so see what is generating unexpected assert --- lib/web_ui/test/ui/vertices_test.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 45a07359aae3a..e5079fa2d0c6a 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -89,7 +89,10 @@ void testMain() { indices: Uint16List.fromList(const [0, 2, 5]), ); throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e) { + } on AssertionError catch (e,st) { + if(!'$e'.contains(r'\"indices\" values must be valid indices in the positions list.')) { + print('FIND STACK $st'); + } expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); } ui.Vertices( // This one does not throw. @@ -120,7 +123,10 @@ void testMain() { colors: Int32List.fromList(const [0xffff0000]), ); throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e) { + } on AssertionError catch (e, st) { + if(!'$e'.contains(r'\"colors\" length must be half the length of "positions".')) { + print('FIND STACK $st'); + } expect('$e', contains(r'\"colors\" length must be half the length of "positions".')); } try { From cfe76fb695685fb9d7c9c8373039f084a1dc78b0 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 09:36:55 -0700 Subject: [PATCH 19/34] move vertices checks in painting to asserts,fix indices asserts --- lib/ui/painting.dart | 11 ++++++-- .../lib/src/engine/canvaskit/vertices.dart | 4 +-- .../lib/src/engine/html/render_vertices.dart | 4 +-- .../engine/skwasm/skwasm_impl/vertices.dart | 4 +-- lib/web_ui/test/ui/vertices_test.dart | 12 +++------ testing/dart/painting_test.dart | 26 +++++++++---------- 6 files changed, 31 insertions(+), 30 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 77a15074b2e1f..3d8a0eccb4ea8 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4744,7 +4744,10 @@ base class Vertices extends NativeFieldWrapperClass1 { List? colors, List? textureCoordinates, List? indices, - }) { + }) : 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.') { + /* THESE CHECKS WILL BE OBSOLETE */ if (colors != null && colors.length != positions.length) { throw ArgumentError('"positions" and "colors" lengths must match.'); } @@ -4828,7 +4831,11 @@ base class Vertices extends NativeFieldWrapperClass1 { Int32List? colors, Float32List? textureCoordinates, Uint16List? indices, - }) { + }) : 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { + /* THESE CHECKS WILL BE OBSOLETE */ if (positions.length % 2 != 0) { throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); } diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index f0f5486f677d6..76890339023f8 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -19,7 +19,7 @@ class CkVertices implements ui.Vertices { }) { 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.any((int i) => i<0 && i>=positions.length),'"indices" values must be valid indices in the positions list.'); + assert(indices==null || !indices.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.'); /*BEFORE ASSERTS if (textureCoordinates != null && textureCoordinates.length != positions.length) { @@ -54,7 +54,7 @@ class CkVertices implements ui.Vertices { 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.any((int i) => i<0 && i*2>=positions.length),'"indices" values must be valid indices in the positions list.'); + assert(indices==null || !indices.any((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 ec7b04d40c686..b34c2bd322939 100644 --- a/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -27,7 +27,7 @@ class SurfaceVertices implements ui.Vertices { List? colors, List? indices, }) : assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'), - assert(indices==null || !indices.any((int i) => i<0 && i>=positions.length),'"indices" values must be valid indices in the positions list.'), + assert(indices==null || !indices.any((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) { @@ -41,7 +41,7 @@ class SurfaceVertices implements ui.Vertices { 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.any((int i) => i<0 && i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { + assert(indices==null || !indices.any((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 73c288c438a74..0bcadcb324086 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 @@ -20,7 +20,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert }) => 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.any((int i) => i<0 && i>=positions.length),'"indices" values must be valid indices in the positions list.'); + assert(indices==null || !indices.any((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) @@ -53,7 +53,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert 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.any((int i) => i<0 && i*2>=positions.length),'"indices" values must be valid indices in the positions list.'); + assert(indices==null || !indices.any((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 e5079fa2d0c6a..adf2d961e55ee 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -89,10 +89,7 @@ void testMain() { indices: Uint16List.fromList(const [0, 2, 5]), ); throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e,st) { - if(!'$e'.contains(r'\"indices\" values must be valid indices in the positions list.')) { - print('FIND STACK $st'); - } + } 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. @@ -123,11 +120,8 @@ void testMain() { colors: Int32List.fromList(const [0xffff0000]), ); throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e, st) { - if(!'$e'.contains(r'\"colors\" length must be half the length of "positions".')) { - print('FIND STACK $st'); - } - expect('$e', contains(r'\"colors\" length must be half the length of "positions".')); + } on AssertionError catch (e) { + expect('$e', contains(r'\"colors\" length must be half the length of \"positions\".')); } try { ui.Vertices.raw( diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 38d2a7a73b593..f2698cc3ce6ad 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -15,8 +15,8 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], textureCoordinates: const [Offset.zero], ); - throw ArgumentError('Vertices did not throw the expected error.'); - } on ArgumentError catch (e) { + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { expect('$e', contains('"positions" and "textureCoordinates" lengths must match.')); } try { @@ -25,8 +25,8 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], colors: const [Color.fromRGBO(255, 0, 0, 1.0)], ); - throw ArgumentError('Vertices did not throw the expected error.'); - } on ArgumentError catch (e) { + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { expect('$e', contains('"positions" and "colors" lengths must match.')); } try { @@ -35,9 +35,9 @@ void main() { 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.'); + throw AssertionError('Vertices did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"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.')); } Vertices( // This one does not throw. VertexMode.triangles, @@ -56,9 +56,9 @@ void main() { 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).'); + throw AssertionError('Vertices.raw did not throw the expected 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( @@ -66,9 +66,9 @@ void main() { 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.'); + throw AssertionError('Vertices.raw did not throw the expected error.'); + } on AssertionError catch (e) { + expect('$e', contains('"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.')); } Vertices.raw( // This one does not throw. VertexMode.triangles, From 468c585a9109e8142245986e3ed4a8cf3d92f367 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 10:08:46 -0700 Subject: [PATCH 20/34] moving vertices checsk in paint to asserts --- lib/ui/painting.dart | 7 ++++--- testing/dart/painting_test.dart | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 3d8a0eccb4ea8..105cb88950bc3 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4747,7 +4747,7 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.') { - /* THESE CHECKS WILL BE OBSOLETE */ + /* THESE CHECKS WILL BE OBSOLETE if (colors != null && colors.length != positions.length) { throw ArgumentError('"positions" and "colors" lengths must match.'); } @@ -4764,7 +4764,7 @@ base class Vertices extends NativeFieldWrapperClass1 { ); } } - } + }*/ final Float32List encodedPositions = _encodePointList(positions); final Float32List? encodedTextureCoordinates = (textureCoordinates != null) ? _encodePointList(textureCoordinates) @@ -4835,7 +4835,7 @@ base class Vertices extends NativeFieldWrapperClass1 { 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { - /* THESE CHECKS WILL BE OBSOLETE */ + /* THESE CHECKS WILL BE OBSOLETE if (positions.length % 2 != 0) { throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); } @@ -4858,6 +4858,7 @@ base class Vertices extends NativeFieldWrapperClass1 { } } } + */ if (!_init(this, mode.index, positions, textureCoordinates, colors, indices)) { throw ArgumentError('Invalid configuration for vertices.'); } diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index f2698cc3ce6ad..f6666a610704c 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -37,7 +37,7 @@ void main() { ); throw AssertionError('Vertices did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"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.')); + expect('$e', contains('"indices" values must be valid indices in the positions list.'));//Cant have details like this in assert>>'"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.')); } Vertices( // This one does not throw. VertexMode.triangles, @@ -68,7 +68,7 @@ void main() { ); throw AssertionError('Vertices.raw did not throw the expected error.'); } on AssertionError catch (e) { - expect('$e', contains('"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.')); + expect('$e', contains('"indices" values must be valid indices in the positions list.'));//cant have details like this>>'"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.')); } Vertices.raw( // This one does not throw. VertexMode.triangles, From 2d9dbd392f29ee0a9db2f5754e00738bbb92ff55 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 10:28:06 -0700 Subject: [PATCH 21/34] remove trailing space --- lib/ui/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 105cb88950bc3..00c97813ee0da 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4835,7 +4835,7 @@ base class Vertices extends NativeFieldWrapperClass1 { 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { - /* THESE CHECKS WILL BE OBSOLETE + /* THESE CHECKS WILL BE OBSOLETE if (positions.length % 2 != 0) { throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).'); } From eeca5e1279e7fd31f0c227b1898c2678e8086c70 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 10:58:50 -0700 Subject: [PATCH 22/34] made assert expects dependent on asserts being enabled --- testing/dart/painting_test.dart | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index f6666a610704c..21632b5aa8a0b 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -8,6 +8,12 @@ import 'dart:ui'; import 'package:litetest/litetest.dart'; void main() { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + test('Vertices checks', () { try { Vertices( @@ -15,7 +21,9 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], textureCoordinates: const [Offset.zero], ); - throw AssertionError('Vertices did not throw the expected error.'); + 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.')); } @@ -25,7 +33,9 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], colors: const [Color.fromRGBO(255, 0, 0, 1.0)], ); - throw AssertionError('Vertices did not throw the expected error.'); + 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.')); } @@ -35,7 +45,9 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - throw AssertionError('Vertices did not throw the expected error.'); + 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.'));//Cant have details like this in assert>>'"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.')); } @@ -56,7 +68,9 @@ void main() { VertexMode.triangles, Float32List.fromList(const [0.0]), ); - throw AssertionError('Vertices.raw did not throw the expected error.'); + 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).')); } @@ -66,7 +80,9 @@ void main() { Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), ); - throw AssertionError('Vertices.raw did not throw the expected error.'); + 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.'));//cant have details like this>>'"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.')); } From 95323964a64f3201e10195a9746476f18bc84855 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 2 Jun 2023 13:58:28 -0700 Subject: [PATCH 23/34] remove non assert checks from vertices code, tests check if asserts are enabled and web renderer --- lib/ui/painting.dart | 42 ------------ .../lib/src/engine/canvaskit/vertices.dart | 15 ----- lib/web_ui/test/ui/vertices_test.dart | 67 ++++++++++++++----- testing/dart/painting_test.dart | 4 +- 4 files changed, 53 insertions(+), 75 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 00c97813ee0da..68ec5e66836f1 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4747,24 +4747,6 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.') { - /* THESE CHECKS WILL BE OBSOLETE - 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.', - ); - } - } - }*/ final Float32List encodedPositions = _encodePointList(positions); final Float32List? encodedTextureCoordinates = (textureCoordinates != null) ? _encodePointList(textureCoordinates) @@ -4835,30 +4817,6 @@ base class Vertices extends NativeFieldWrapperClass1 { 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { - /* THESE CHECKS WILL BE OBSOLETE - 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('"colors" length must be half the length of "positions".'); - } - if (textureCoordinates != null && textureCoordinates.length != positions.length) { - throw ArgumentError('"positions" and "textureCoordinates" lengths must match.'); - } - if (indices != null) { - final int halfPositions = positions.length >> 1; // len/2 - we already checked that positions.length is even - for (int index = 0; index < indices.length; index += 1) { - // we need to check that `indices[index] * 2 >= positions.length`, so do it without multiplying - if (indices[index] >= halfPositions) { - 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.', - ); - } - } - } - */ 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 76890339023f8..47ad40d705765 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -20,21 +20,6 @@ class CkVertices implements ui.Vertices { 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.'); - /*BEFORE ASSERTS - 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.'); - } - */ return CkVertices._( toSkVertexMode(mode), toFlatSkPoints(positions), diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index adf2d961e55ee..19a879525dc03 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); @@ -60,25 +67,30 @@ void testMain() { }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454 test('Vertices assert checks', () { - /* HTML renderer's SurfaceVertices() does not support textureCoordinates, so we cant test this universally across web - try { - ui.Vertices( - ui.VertexMode.triangles, - const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], - textureCoordinates: const [ui.Offset.zero], - ); - throw AssertionError('Vertices did not throw the expected error.'); - } on AssertionError catch (e) { - expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); + // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates + if(renderer.rendererTag != 'html') { + try { + 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.'); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); + } } - */ try { 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)], ); - throw AssertionError('Vertices did not throw the expected error.'); + if(assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } } on AssertionError catch (e) { expect('$e', contains(r'\"positions\" and \"colors\" lengths must match.')); } @@ -88,7 +100,9 @@ void testMain() { const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - throw AssertionError('Vertices did not throw the expected error.'); + if(assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } } on AssertionError catch (e) { expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); } @@ -109,17 +123,36 @@ void testMain() { ui.VertexMode.triangles, Float32List.fromList(const [0.0]), ); - throw AssertionError('Vertices.raw did not throw the expected error.'); + if(assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } } 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 { + 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.'); + } + } on AssertionError catch (e) { + expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.')); + } + } try { 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]), ); - throw AssertionError('Vertices did not throw the expected error.'); + if(assertsEnabled) { + throw AssertionError('Vertices did not throw the expected assert error.'); + } } on AssertionError catch (e) { expect('$e', contains(r'\"colors\" length must be half the length of \"positions\".')); } @@ -129,7 +162,9 @@ void testMain() { Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), ); - throw AssertionError('Vertices.raw did not throw the expected error.'); + if(assertsEnabled) { + throw AssertionError('Vertices.raw did not throw the expected assert error.'); + } } on AssertionError catch (e) { expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.')); } diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 21632b5aa8a0b..da7abfaaf0aa9 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -49,7 +49,7 @@ void main() { 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.'));//Cant have details like this in assert>>'"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.')); + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } Vertices( // This one does not throw. VertexMode.triangles, @@ -84,7 +84,7 @@ void main() { 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.'));//cant have details like this>>'"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.')); + expect('$e', contains('"indices" values must be valid indices in the positions list.')); } Vertices.raw( // This one does not throw. VertexMode.triangles, From 8b06199d94eb0e8740fd74988abce4e3f7d9e930 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Wed, 7 Jun 2023 12:10:17 -0700 Subject: [PATCH 24/34] change to using every instead of any --- lib/ui/painting.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 68ec5e66836f1..7703c198d105f 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4746,7 +4746,7 @@ base class Vertices extends NativeFieldWrapperClass1 { List? indices, }) : 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.') { + 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) @@ -4816,7 +4816,7 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { + assert(indices==null || !indices.every((int i) => i >= 0 && i*2 Date: Wed, 7 Jun 2023 12:14:56 -0700 Subject: [PATCH 25/34] fix spacing --- lib/ui/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 7703c198d105f..0edf889b29644 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4816,7 +4816,7 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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 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.'); } From f82de2b2175b24b492d4373ffd3e0e5a9e9eb37b Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Wed, 7 Jun 2023 12:21:11 -0700 Subject: [PATCH 26/34] change any use to every in vertices checks --- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/html/render_vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index 47ad40d705765..7b19506e10a91 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -19,7 +19,7 @@ class CkVertices implements ui.Vertices { }) { 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.'); + 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), @@ -39,7 +39,7 @@ class CkVertices implements ui.Vertices { 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.'); + 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 b34c2bd322939..f1305d8bc9b57 100644 --- a/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -27,7 +27,7 @@ class SurfaceVertices implements ui.Vertices { List? colors, List? indices, }) : assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'), - assert(indices==null || !indices.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.'), + 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) { @@ -41,7 +41,7 @@ class SurfaceVertices implements ui.Vertices { 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.') { + 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 0bcadcb324086..641c6cacd3906 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 @@ -20,7 +20,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert }) => 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.any((int i) => i<0 || i>=positions.length),'"indices" values must be valid indices in the positions list.'); + 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) @@ -53,7 +53,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert 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.any((int i) => i<0 || i*2>=positions.length),'"indices" values must be valid indices in the positions list.'); + 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) From 3f1f1cb884e3d0e645377aa7a231aa087350d595 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Wed, 7 Jun 2023 13:38:50 -0700 Subject: [PATCH 27/34] remove negation on every --- lib/ui/painting.dart | 4 ++-- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/html/render_vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 0edf889b29644..4b64d5026edeb 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4746,7 +4746,7 @@ base class Vertices extends NativeFieldWrapperClass1 { List? indices, }) : 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.') { + 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) @@ -4816,7 +4816,7 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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.') { + 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 7b19506e10a91..ded75ca5656e1 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -19,7 +19,7 @@ class CkVertices implements ui.Vertices { }) { 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.'); + 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), @@ -39,7 +39,7 @@ class CkVertices implements ui.Vertices { 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.'); + 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 f1305d8bc9b57..210a4070273af 100644 --- a/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -27,7 +27,7 @@ class SurfaceVertices implements ui.Vertices { List? colors, List? indices, }) : 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.'), + 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) { @@ -41,7 +41,7 @@ class SurfaceVertices implements ui.Vertices { 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.') { + 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 641c6cacd3906..c056c440bc464 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 @@ -20,7 +20,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert }) => 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.'); + 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) @@ -53,7 +53,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert 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.'); + 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) From 371b2e6b283bc179276636259d97ce7737ceeb93 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 8 Jun 2023 11:03:53 -0700 Subject: [PATCH 28/34] fix spacing around == --- lib/ui/painting.dart | 4 ++-- lib/web_ui/lib/src/engine/canvaskit/vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/html/render_vertices.dart | 4 ++-- lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 4b64d5026edeb..4238ff1e8f690 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4746,7 +4746,7 @@ base class Vertices extends NativeFieldWrapperClass1 { List? indices, }) : 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.') { + 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) @@ -4816,7 +4816,7 @@ base class Vertices extends NativeFieldWrapperClass1 { }) : 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.') { + 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 ded75ca5656e1..3b6ea03ee3a32 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -19,7 +19,7 @@ class CkVertices implements ui.Vertices { }) { 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.'); + 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), @@ -39,7 +39,7 @@ class CkVertices implements ui.Vertices { 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.'); + 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 210a4070273af..3f5241814f193 100644 --- a/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -27,7 +27,7 @@ class SurfaceVertices implements ui.Vertices { List? colors, List? indices, }) : 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.'), + 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) { @@ -41,7 +41,7 @@ class SurfaceVertices implements ui.Vertices { 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.') { + 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 c056c440bc464..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 @@ -20,7 +20,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert }) => 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.'); + 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) @@ -53,7 +53,7 @@ class SkwasmVertices extends SkwasmObjectWrapper implements ui.Vert 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.'); + 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) From e42938d20d70953ad4a5ced5caa111e5efe378cf Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 8 Jun 2023 11:11:49 -0700 Subject: [PATCH 29/34] add space after if where missing --- lib/web_ui/test/ui/vertices_test.dart | 18 +++++++++--------- testing/dart/painting_test.dart | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 19a879525dc03..6b7605fff1b89 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -68,14 +68,14 @@ void testMain() { 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') { + if (renderer.rendererTag != 'html') { try { ui.Vertices( ui.VertexMode.triangles, const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], textureCoordinates: const [ui.Offset.zero], ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -88,7 +88,7 @@ void testMain() { const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], colors: const [ui.Color.fromRGBO(255, 0, 0, 1.0)], ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -100,7 +100,7 @@ void testMain() { const [ui.Offset.zero, ui.Offset.zero, ui.Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -123,21 +123,21 @@ void testMain() { ui.VertexMode.triangles, Float32List.fromList(const [0.0]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } } 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') { + if (renderer.rendererTag != 'html') { try { 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) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -150,7 +150,7 @@ void testMain() { Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), colors: Int32List.fromList(const [0xffff0000]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -162,7 +162,7 @@ void testMain() { Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } } on AssertionError catch (e) { diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index da7abfaaf0aa9..edc64401827a7 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -21,7 +21,7 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], textureCoordinates: const [Offset.zero], ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -33,7 +33,7 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], colors: const [Color.fromRGBO(255, 0, 0, 1.0)], ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -45,7 +45,7 @@ void main() { const [Offset.zero, Offset.zero, Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -68,7 +68,7 @@ void main() { VertexMode.triangles, Float32List.fromList(const [0.0]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } } on AssertionError catch (e) { @@ -80,7 +80,7 @@ void main() { Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), ); - if(assertsEnabled) { + if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } } on AssertionError catch (e) { From f987b94ce270b1be677c5b33d9b73f0d350e2392 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 8 Jun 2023 11:14:00 -0700 Subject: [PATCH 30/34] update AUTHORS --- AUTHORS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 1211ffab5ac3a..188a9b262fd6a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -23,4 +23,5 @@ Koutaro Mori TheOneWithTheBraid Twin Sun, LLC Qixing Cao -LinXunFeng \ No newline at end of file +LinXunFeng +Tim Maffett \ No newline at end of file From a9e8e0fae9b3cbb61b57265476ef79cc827eba48 Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 30 Nov 2023 08:26:32 -0800 Subject: [PATCH 31/34] add use and disposing of invalid Vertices objects when asserts not enabled --- lib/web_ui/test/ui/vertices_test.dart | 62 ++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 6b7605fff1b89..2ccabf2ebdfdb 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -70,38 +70,50 @@ void testMain() { // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates if (renderer.rendererTag != 'html') { try { - ui.Vertices( + 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 { - ui.Vertices( + 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 { - ui.Vertices( + 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.')); @@ -119,12 +131,16 @@ void testMain() { test('Vertices.raw assert checks', () { try { - ui.Vertices.raw( + 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).')); @@ -132,38 +148,50 @@ void testMain() { // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates if (renderer.rendererTag != 'html') { try { - ui.Vertices.raw( + 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 { - ui.Vertices.raw( + 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 { - ui.Vertices.raw( + 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.')); @@ -387,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); +} From 0db8427241760df8aff6c162153e42d39359ed1b Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Thu, 30 Nov 2023 08:27:08 -0800 Subject: [PATCH 32/34] dispose of invalid vertices objects (that would be created if asserts are not enabled) --- testing/dart/painting_test.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index edc64401827a7..edd4911940eb7 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -20,7 +20,7 @@ void main() { 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.'); } @@ -32,7 +32,7 @@ void main() { 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.'); } @@ -44,7 +44,7 @@ void main() { VertexMode.triangles, const [Offset.zero, Offset.zero, Offset.zero], indices: Uint16List.fromList(const [0, 2, 5]), - ); + ).dispose(); if (assertsEnabled) { throw AssertionError('Vertices did not throw the expected assert error.'); } @@ -67,7 +67,7 @@ void main() { Vertices.raw( VertexMode.triangles, Float32List.fromList(const [0.0]), - ); + ).dispose(); if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } @@ -79,7 +79,7 @@ void main() { VertexMode.triangles, Float32List.fromList(const [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]), indices: Uint16List.fromList(const [0, 2, 5]), - ); + ).dispose(); if (assertsEnabled) { throw AssertionError('Vertices.raw did not throw the expected assert error.'); } From 8197599d799dc9f962578cc5e95cdd428f8b18fc Mon Sep 17 00:00:00 2001 From: Tim Maffett Date: Fri, 1 Dec 2023 14:00:07 -0800 Subject: [PATCH 33/34] remove trailing space --- lib/web_ui/test/ui/vertices_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/ui/vertices_test.dart b/lib/web_ui/test/ui/vertices_test.dart index 2ccabf2ebdfdb..be4b6bf0d4454 100644 --- a/lib/web_ui/test/ui/vertices_test.dart +++ b/lib/web_ui/test/ui/vertices_test.dart @@ -416,7 +416,7 @@ const List _circularVertexIndices = [ 36 ]; -// This function is used when asserts are NOT enabled and vertices() allows invalid +// 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 ) { From 4050715f11c7268d9361325995abb1a688bb091f Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Tue, 6 Feb 2024 15:55:53 -0800 Subject: [PATCH 34/34] Update AUTHORS small tweak --- AUTHORS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 0e4cb2051ece3..31562cfd1e843 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,5 @@ TheOneWithTheBraid Twin Sun, LLC Qixing Cao LinXunFeng -Tim Maffett Amir Panahandeh - +Tim Maffett