Repository navigation
docs: clarify external buffer and image loading options - #162
Merged
Merged
Conversation
spnda
requested changes
Oct 13, 2026
spnda
left a comment
spnda
left a comment
Owner
There was a problem hiding this comment.
Overall this is a great change, thank you very much. The documentation definitely needs some additional work and attention from myself. I just have a couple nits. Also, great usage of Catch2 I had no idea you could structure the control flow like that, very cool.
spnda
added
documentation
Contributor
Author
|
Addressed all three points in a4daf78:
On the updated head, the three loading-option cases pass with 86 assertions, and 49 existing cases pass with 1,153 assertions. The documentation build also passes with no added Sphinx warnings. The full sample-asset suite was not run. The new GitHub workflows currently show |
spnda
approved these changes
Oct 14, 2026
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The loading option comments do not explain which
DataSourcealternatives remain possible after parsing. This adds a practical buffer/image source table and updates both option comments to distinguish local files, embedded data, remote URIs, and custom allocation callbacks. It also explains missing-file errors, buffer-view images, and the difference between reading image bytes and decoding pixels.Three parser tests cover the independence of the two options, embedded and remote sources with default/custom allocations, and missing local files. They intentionally pass on the unchanged base parser: this documents existing behavior rather than changing resource loading.
The review follow-up states that buffer data URIs are decoded independently of the loading option, spells out both meshopt extension names, and checks the buffer-view image variant before accessing it.
Validation on Linux with GCC 14.2:
fastgltf_tests "[loading-options]": 3 cases / 86 assertions passed.git diff --checkpassed.Only the required sample assets were downloaded for these checks; the complete sample-asset suite was not run.
The GCC 15 and Clang 22 C++ modules jobs also fail on the unchanged upstream base with the same compiler diagnostics; the module workflow and build configuration are unchanged by this PR.
Fixes #146.