Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decode meshopt in buffer structure #160

Merged
merged 5 commits into from
Nov 30, 2024
Merged

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Nov 27, 2024

Addresses CesiumGS/3d-tiles-validator#323 :

The BinaryBufferDataResolver is a convenience class that was supposed to offer a simple functionality:

  • Throw in some buffer[]/bufferView[] JSON definitions
  • Receive some buffersData[]/bufferViewsData[] (NodeJS-Buffer) objects with the binary data

This was used for resolving the buffer/bufferView structure within glTF, but also within binary .subtree files.

The linked issue brought up the case where glTF data uses the EXT_meshopt_compression extension. In this case, obtaining the NodeJS-Buffer objects for a given buffer view is not trivial. First of all, the meshopt-compressed data has to be decoded. Additionally, there may be so-called "fallback" buffers in this structure, which are buffers that do not have a uri.

This PR extends this class to handle EXT_meshopt_compression. I'm not perfectly happy with that. This was supposed to be a somewhat "agnostic" class that is now tied to glTF. And ... the handling does not make sense for subtree data (unless... we allow EXT_meshopt_compression for .subtree data...).

But given the intended usage (and expectations about convenience that users might have when using this class on glTF buffer/bufferView structures), it may be reasonable to add this here.

I'll do further tests (and add specs), so that this may become part of a release soon, so that we can update the validator.

@javagl javagl marked this pull request as ready for review November 29, 2024 17:17
@javagl
Copy link
Contributor Author

javagl commented Nov 29, 2024

I cleaned up and documented the BinaryBufferDataResolver to handle Meshopt-compressed buffers. And I added specs using a TriangleMeshopt.gltf.

When applying it (as part of the 3d-tiles-validator) to a real-world data set, I encountered a minor issue, namely that the byteOffset of a buffer view is not required. This was not handled properly initially, but now I added a dedicated test with TriangleMeshoptNoByteOffset.gltf.

@lilleyse lilleyse merged commit a2900b0 into main Nov 30, 2024
2 checks passed
@lilleyse lilleyse deleted the handle-meshopt-in-buffer-structure branch November 30, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants