Assert when glDrawElements cannot draw anything because of missing vertex bounds

[paradust7]

Even with FULL_ES2, glDrawElements cannot handle the case where the indices are in a buffer but the vertices are client-side. Nothing will be drawn in this case. Add an assertion to make this situation visible when debugging.

Fixes #26460

[sbc100]

Does this assert belong here inside this for loop? Could it not go outside the loop?

[sbc100]

Should this be !count || !GLctx.currentElementArrayBufferBinding ? (i.e. I'm not sure what the count || part is doing here.

[paradust7]

It is written as intended, but there may be an edge case I didn't consider.

preDrawHandleClientVertexAttribBindings is called in two places. It is called by glDrawArrays, where the vertex count is always passed by the user. And in glDrawElements, where the vertex count is computed if it can be, or otherwise passed as 0.

The error case is only for glDrawElements, when the vertex count cannot be computed, so that the count is passed as 0, even though the real count is non-zero.

I added the "count ||" to make sure the assert doesn't fire for glDrawArrays. But it could still fire if glDrawArrays is called with a count of 0.

I'll get a fix for this ready.

[juj]

Pondering on this a bit, it does seem that

assert(count == 0 || !GLctx.currentElementArrayBufferBinding, 'must use array buffers when using element buffer');

would work well.

With

assert(count || !GLctx.currentElementArrayBufferBinding, 'must use array buffers when using element buffer');

the assert would only ever fire if count == 0 && index buffer bound (i.e. we are not drawing anything and there is an index buffer bound)

The assert logic in general that verifies that there cannot be a GPU-side index buffer bound when the rendering has any CPU-side vertex attribute pointers seems sound. (we'd have to track the min/max vertex index at buffer upload time, like browsers internally also do).

Does this assert belong here inside this for loop?

Yeah, it must be inside the loop, since the assert only applies if there is at least one vertex attribute stream found that has a client-side buffer pointer (i.e. attribute stream index for which cb.clientside == true)

[paradust7]

@juj The original issue explains the case I'm trying to fix:

#26460

The problem case occurs when the element buffer is not client-side, but one or more array buffers is. In this situation, glDrawElements always passes count=0 to preDrawHandleClientVertexAttribBindings, because it doesn't know the real vertex count. This results in nothing being drawn.

I think it would be cleaner to pass count=null when the vertex bounds are unknown, and check for null explicitly in the loop. The only downside of this is that the null check has to occur even when assertions are disabled, but the check is pretty cheap.

[juj]

In this situation, glDrawElements always passes count=0 to preDrawHandleClientVertexAttribBindings, because it doesn't know the real vertex count. This results in nothing being drawn.

Ohh, gotcha. Looks good to me then. Might be good to drop this note as a comment before the assert?

[sbc100]

I'm afraid I don't really know this code well enough to know for sure if this correct. @juj WDYT?