search

KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

Incorrect definition of vertex ID in DrawElementsOneInstance #85

Closed LtSten closed 2 years ago

LtSten commented 2 years ago

Considering §10.4 of the Core 4.6 spec, with regards to DrawElementsOneInstance, we have

The vertex ID of the ith element transferred is the sum of basevertex and the value stored in the currently bound element array buffer at offset indices + i

As defined in §10.3.10 (which subsequently defers to §10.3.9), offsets are expressed in basic machine units; we note this is consistent with the passing of cmd->firstIndex * size-of-type as indices to DrawElementsInstancedBaseVertexBaseInstance in the definition of DrawElementsIndirect.

Returning to DrawElementsOneInstance, the correct offset to apply to the element array buffer is therefore not indices + i but indices + i * size-of-type, since the former fails (in practice) if type is not UNSIGNED_BYTE. This appears to be a slip introduced in the spec for 4.3, prior to which indices[i] was meaningful (if subtly questionable since it is formally void*, not type*):

The ith element transferred by DrawElementsOneInstance will be taken from element indices[i] of each enabled non-instanced array.

pdaniell-nv commented 2 years ago

We discussed this in the 2022-01-19 meeting and agree the spec should be fixed here. The sentence in the 2nd paragraph on page 369 of the OpenGL 4.6 core spec should say:

The vertex ID of the ith element transferred is the sum of basevertex and the value stored in the currently bound element array buffer at offset indices + i * size-of-type.

pdaniell-nv commented 2 years ago

Assigned to @oddhack to make this spec fix on the next round of updates.

oddhack commented 2 years ago

This appears equally applicable to ES 3.2 so planning to include the fix there as well, unless I missed something.

pdaniell-nv commented 2 years ago

This appears equally applicable to ES 3.2 so planning to include the fix there as well, unless I missed something.

Sounds good, thanks.

oddhack commented 2 years ago

This should be fixed in the spec update of May 5, 2022.