KhronosGroup / OpenGL-API

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

Default values for VERTEX_BINDING_* state seem wrong #44

Closed baldurk closed 5 years ago

baldurk commented 5 years ago

In Table 23.4: Vertex Array Object State in the OpenGL 4.6 spec, the table lists the state for vertex bindings:

Get value Type Get Command Initial Value Description Sec.
VERTEX_BINDING_OFFSET 16 × Z GetInteger64i_v 0 Byte offset of the first element in data store of the buffer bound to vertex binding i 10.3
VERTEX_BINDING_STRIDE 16 × GetIntegeri_v 16 Stride between elements in vertex binding i 10.3
VERTEX_BINDING_DIVISOR 16 × Z GetIntegeri_v 16 Instance divisor for vertex binding i 10.3
VERTEX_BINDING_BUFFER 16 × Z GetIntegeri_v 16 Name of buffer bound to vertex binding i 10.3

The bolded values seem strange to me. A default stride of 16 might make sense since the default format is GL_FLOAT / 4 which is 16 bytes, though VERTEX_ATTRIB_ARRAY_STRIDE defaults to 0 meaning tightly packed anyway. Either way the divisor and buffer definitely don't seem like they should default to 16. I'd guess these values should perhaps be 0, 0, i respectively.

As a side note it's a little confusing to have the vertex attrib array state in table 23.3 when the spec says that some of it is effectively bound to be identical to the vertex bindings - e.g. VERTEX_ATTRIB_ARRAY_STRIDE I think is always equal to VERTEX_BINDING_STRIDE for whichever binding VERTEX ATTRIB BINDING points to. I'm not sure how best to express that though but it would be nice to clarify that these two are tied together, similar to how the actual spec for those functions expresses the old glVertexAttribPointer functions in terms of setting the attrib and the binding - old in terms of new.

Maybe slightly related to #42.

pdaniell-nv commented 5 years ago

We discussed this in the OpenGL/ES meeting yesterday and agree the defaults for VERTEX_BINDING_DIVISOR and VERTEX_BINDING_BUFFER should have been zero. Assigned to @oddhack to make the change.

oddhack commented 5 years ago

Fixed in gitlab, closing. Will go out with the next public spec update.