[dart:ui] remove expensive index assertion in Vertices.
Iterating through every index value is really, really expensive. Especially if users are suppllying hundreds of thousands of vertices.
Where are we expecting this validation to occur then? At some point we have to protect against reading past the ends of the data arrays.
We don't read the indices though, we pass it to the gpu driver. I suppose we should validate that reasonable things happen there.
verifying on GLES/Vulkan/Metal backend, no crashes. We don't ever read the index data on the CPU
We should add a test case that using out of range index values works fine if that is the expected behavior.
We may not currently read them on the CPU, but might in the future. One example is the computation of the bounds of the vertices. Currently their bounds are the bounds of all vertices, but if we have 100k vertices and only 100 indices then there is wasted effort there - so we might in the future switch to using the indices for bounds (possibly with a quick test as to whether there are more vertices than indices or vice versa) - at which point we want to make sure that there is a test case that would remind us that OOB indices are specifically allowed...
I can add a test at the DL-impeller level and produces a golden. SG?
For rendering - as long as it goes through DlBuilder. ui.Canvas is basically a pass through, but starting at Builder we have increasing reasons to inspect the data.
Another thing to verify is constructing the Dart Vertices object itself - the constructor might decide to prune the data at some point if indices is small? (Probably not....? But good to have a test that makes sure it doesn't trip over a bad index just in case)
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).
If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.
Changes reported for pull request #53558 at sha 819f5c075c1dc7c2ea4bb16c13e246df7ea551c0
PTAL @flar
Golden file changes are available for triage from new commit, Click here to view.
Changes reported for pull request #53558 at sha c863b4f431472c7f7c8adc8fd103be12478ce194
Jim is out this week. maybe @jtmcdole PTAL?
@jonahwilliams Can we land this?
@chinmaygarde did you mean to close this? We should be able to land it, didn't realize it got reviewed.
Oh, yeah. Definitely a mistake. Sorry.
no worries!