KhronosGroup / glTF-Sample-Models

glTF Sample Models
3.13k stars 1.31k forks source link

Buggy model doesn't follow the 4 bytes alignment requirement of the spec for indices #356

Closed CrankFlash closed 2 years ago

CrankFlash commented 2 years ago

According to the spec https://www.khronos.org/registry/glTF/specs/2.0/glTF-2.0.html#data-alignment :

each element of a vertex attribute MUST be aligned to 4-byte boundaries inside a bufferView (i.e., accessor.byteOffset and bufferView.byteStride MUST be multiples of 4). <

Now this rule is specified for vertex attributes, but doesn't say anything about indices. Because this rule is specified for performance reasons, I would assume the same is true for index buffer offsets.

But in the buggy model, the 3rd index buffer has an offset of bufferView.Offset + accessor.byteOffset = 5896152 + 40026 = 5936178 and 5936178 % 4 != 0 . Which makes it not aligned to a 4byte address boundary.

Is this intentional, or an actual bug ? In HLSL the ByteAddressBuffer.Load() instruction will only load 4bytes aligned address (ByteAddressBuffer::Load()) which in this specific case will mean the index read will be offset by 2 bytes.

So far, only the buggy model has this problem. Every other asset I tried from the repo works just fine. This is its export tag :

"generator": "COLLADA2GLTF", "version": "2.0"

lexaknyazev commented 2 years ago

That index accessor is of uint16 type, so only 2-byte alignment is enforced. Given that glTF 2.0 also supports uint8 index buffers, some apps would need to mangle that data anyway.

CrankFlash commented 2 years ago

Thanks for the quick reply. That makes sense. I'll just have to handle it then.

Is this specified in the spec ? I can't seem to find it.