Created accessor bufferViews should be byte aligned

[ggetz]

This is why Cesium_Air.gltf was not rendering in CesiumGS/cesium#6805

The bufferViews created when splitting accessors with different byte strides must be byte aligned.

[lilleyse]

What is the error in Cesium? I took a look at Cesium_Air after going through gltf-pipeline and it seems like Cesium should still render it despite the validation errors.

The model seems to meet this condition at least:

The offset of an accessor into a bufferView (i.e., accessor.byteOffset) and the offset of an accessor into a buffer (i.e., accessor.byteOffset + bufferView.byteOffset) must be a multiple of the size of the accessor's component type.

Though doesn't follow

For performance and compatibility reasons, each element of a vertex attribute must be aligned to 4-byte boundaries inside bufferView (i.e., accessor.byteOffset and bufferView.byteStride must be multiples of 4).

Which is not good, but Cesium should still render it.

The reason I bring this all up is because editing the byte offset is not enough, padding also needs to get added to the buffer which could be a longer fix.

[ggetz]

Sandcastle example

Getting the following WebGL errors:

bucket.html:1 [.Offscreen-For-WebGL-0000008F71D9A340]GL ERROR :GL_INVALID_OPERATION : glVertexAttribPointer: offset not valid for type
45115:48:29.071 [.Offscreen-For-WebGL-0000008F71D9A340]GL ERROR :GL_INVALID_OPERATION : glDrawElements: attached to enabled attrib 0 : no buffer

The byteOffset was being set to 2, which was what was causing the error because of the later

For performance and compatibility reasons, each element of a vertex attribute must be aligned to 4-byte boundaries inside bufferView (i.e., accessor.byteOffset and bufferView.byteStride must be multiples of 4).

[lilleyse]

@ggetz I stared at this for a really long time. The fix here is pretty close, but I think it needs to be

currentByteOffset = (i < accessorsLength - 1) ? accessors[i + 1].byteOffset : undefined;

Which is setting the current byte offset to next accessor's byte offset rather than the end of the current accessor. Cesium_Air brought up a discrepancy where these two values are not always the same if there is padding between the current accessor and the next accessor.

The % 4 approach works a lot of the time but has some edge cases (say if the current accessor ends at byte 1, next accessor starts at byte 1 too (both are using BYTE component type), currentByteOffset would be set to 2 and the next accessor's byte offset would be computed as -1.

This approach should work in Cesium without requiring the more involved changes of always aligning byte offsets and byte strides to 4 bytes, which I still think involves buffer copies of some sort.

[ggetz]

Got it, thanks @lilleyse! This means we don't have to pad the buffer correct? That's good news!

[lilleyse]

Yeah no padding required for this fix thankfully.

[ggetz]

Thanks again @lilleyse, updated. I can confirm Cesium Air now loads in CesiumGS/cesium#6805 with this change.

[lilleyse]

Nice!

[lilleyse]

Opened #426 so we can tackle the proper fix some other time.