assertion failure on empty indices in ui.Vertices constructor #32434
Conversation
| } | ||
|
|
||
| if (indices.data()) { | ||
| if (indices.data() && indices.num_elements() > 0) { |
There was a problem hiding this comment.
Should the num_elements check also apply to texture_coordinates, colors, and positions?
There was a problem hiding this comment.
We talked this out offline, but the data conditions here are:
pointers, texCoords, colors are all the same size (well, 2 of them are x,y so they are twice as long, but basically their counts are tied together). This is because all of that data is tied together 1:1:1.
Indices has its own count because it can have a different size than the others.
The Builder constructor (for both SkVertices and DlVertices) takes flags to indicate if the arrays are being delivered, but the indices passes in the count as an indicator (I could have switched this to a boolean, but kept the same conventions as SkVertices).
So, pointers is copied unconditionally since it is required, even if it is zero length.
texCoords and colors are copied on the same condition as their flags were set - by nullness.
indices is copied on the same condition as its indicator in the builder constructor (non-null and count > 0).
I just noticed a simplification. elements() > 0 covers the null case as well. But I'm not going to redo the PR as it needs to propagate. The optimization is minimal.
flar
added
the
waiting for tree to go green
…uctor (#32434) (#32437) Co-authored-by: Jim Graham <[email protected]>
…constructor (#32434) (#32543) Co-authored-by: Jim Graham <[email protected]>
5 participants
Fixes a problem in an upstream test where an empty indices array was specified in a ui.Vertices constructor leading to a crash when the DlVertices::Builder did not expect us to deliver any vertices at all (even though the count was 0).
Fixes b/228223653