glTF: Fix parsing buffer data with application/gltf-buffer and image/* MIME types #42501
Conversation
This file contains 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
…* MIME types See KhronosGroup/glTF#944 for context on the application/gltf-buffer MIME type. The glTF 2.0 spec supports `image/jpeg` and `image/png` which can also be base64-encoded in buffer URIs. Fixes godotengine#33796.
akien-mga
added
bug
topic:import
cherrypick:3.x
|
Note to self: this needs to be cherry-picked together with #42504 which properly fixes the issue, and fixes up some of the changes done here (no need to handle image MIME types for |
akien-mga
added a commit
to akien-mga/godot
that referenced
this pull request
Oct 5, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes godotengine#33796 (and fixes up godotengine#42501).
|
Cherry-picked for 3.2.4. |
akien-mga
removed
the
cherrypick:3.x
akien-mga
added a commit
that referenced
this pull request
Oct 5, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes #33796 (and fixes up #42501). (cherry picked from commit 2e99d0b)
MarcusElg
pushed a commit
to MarcusElg/godot
that referenced
this pull request
Oct 19, 2020
The glTF 2.0 spec only makes `mimeType` mandatory for `bufferView` image data, so the previous logic to handle URIs with base64-encoded images could fail if `mimeType` is undefined. The logic was documented and refactored to better handle the spec, notably: - `uri` and `bufferView` are now mutually exclusive, and only the latter fails if `mimeType` is undefined. - `uri` with a file path will now respect the `mimeType` if defined, and thus attempt loading the file with the specified format (even if its extension is not the one expected for this format). So we can support bad extensions (PNG data with `.jpg` extension) or custom ones (PNG data in `.img` file for example). - `uri` with base64 encoded data will infer MIME type from `data:image/png` or `data:image/jpeg` if it was not documented in `mimeType` initially. - `uri` with base64 encoded data, no `mimeType` and `application/octet-stream` or `application/gltf-buffer` will fall back to trying both PNG and JPEG loaders. Fully fixes godotengine#33796 (and fixes up godotengine#42501).
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.
See KhronosGroup/glTF#944 for context on the
application/gltf-buffer MIME type.
The glTF 2.0 spec supports
image/jpegandimage/pngwhich can also bebase64-encoded in buffer URIs.
Fixes #33796.
I made it more forgiving as I doubt we'll see situations where a URI starts with
data:is meant to be loaded as a file path. But it should throw an error if it's not one of the supported MIME types so we can investigate further.