Implement support for KHR_mesh_quantization #7157
Merged
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
This change advertises support for KHR_quantized_geometry and allows to specify it as part of required extensions when loading glTF files. The reason why the actual extension class is empty is that the extension doesn't define any new JSON metadata, and merely allows the use of various quantized attributes in glTF file - and as far as I can tell Babylon.js already supports these quantized formats either by directly using the encoded data, or by decoding it into floating-point arrays.
zeux
changed the title
bghgary
reviewed
Nov 13, 2019
| */ | ||
| public enabled: boolean; | ||
|
|
||
| private _loader: GLTFLoader; |
There was a problem hiding this comment.
There is no need to save the loader since it's only used in the constructor.
Comment on lines
28
to
31
| /** @hidden */ | ||
| public dispose() { | ||
| delete this._loader; | ||
| } |
There was a problem hiding this comment.
Can be removed once the private _loader field is removed.
There was a problem hiding this comment.
I've tried to remove this actually and I can't - the TypeScript compiler (iirc) requires that there's an implementation of dispose. It can be empty though, I'll fix this.
There was a problem hiding this comment.
Ahh, yes, the interface function is required.
| const NAME = "KHR_quantized_geometry"; | ||
|
|
||
| /** | ||
| * [Specification](https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_quantized_geometry) |
There was a problem hiding this comment.
Can't verify since the spec hasn't been merged.
Also clean up unnecessary member variable
zeux
changed the title
bghgary
approved these changes
Nov 14, 2019
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.
This change advertises support for
KHR_quantized_geometryKHR_mesh_quantizationand allows to specify it as part of required extensions when loading glTF files.
The reason why the actual extension class is empty is that the extension
doesn't define any new JSON metadata, and merely allows the use of
various quantized attributes in glTF file - and as far as I can tell
Babylon.js already supports these quantized formats either by directly
using the encoded data, or by decoding it into floating-point arrays.
Extension spec: KhronosGroup/glTF#1673
Validator support: KhronosGroup/glTF-Validator#124
Example model: BrainStem.glb.zip
Note that based on prior testing, Babylon.js de-facto supports this. This change is required because the extension is non-optional and Babylon.js refuses to load files with required extensions that aren't explicitly supported.