Throw exception when unsupported extension is required#2628
Open
javagl wants to merge 1 commit intojMonkeyEngine:masterfrom
Open
Throw exception when unsupported extension is required#2628javagl wants to merge 1 commit intojMonkeyEngine:masterfrom
javagl wants to merge 1 commit intojMonkeyEngine:masterfrom
Conversation
riccardobl
reviewed
Feb 22, 2026
| * encounters an asset that contains an extension in its <code>extensionsRequired</code> declaration that | ||
| * is not supported. | ||
| */ | ||
| private boolean strictExtensionCheck; |
Member
There was a problem hiding this comment.
Suggested change
| private boolean strictExtensionCheck; | |
| private boolean strictExtensionCheck = true; |
better setting it here imo
Member
|
Thanks!
I think there is no problem in adding this to an unit test here . |
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.
Fixes #2617
Current state
Until now, the glTF loader only printed a
SEVEREerror message when an unsupported extension was declared in theextensionsRequiredof a glTF asset. As a result, it could happen that ...In both cases, the error could show up as a arbitrary exception during loading or rendering. It could be a
NullPointerException,IndedOutOfBoundsException, or whatever - with no clear indication for what caused this exception.New state
This PR changes the behavior as follows:
extensionsRequired, then the default behavior is that the loader immediately bails out with anAssetLoadException, with an appropriate error message to indicate the reason for the errorGltfModelKey#setStrict(boolean)function. When explicitly settinggltfModelKey.setStrict(false), then any unsupported extension will still (only) cause aSEVEREerror message (and likely, but not necessarily, cause an exception later...)Testing
I'm not entirely sure about the best test procedure here. I tested it locally, in two ways:
I tested it by loading the SheenWoodLeatherSofa that was mentioned in #2385 . It now causes the exception
Instead of that random NPE that it caused until now.
I also created a dedicated tests for the extension declaration handling. For this, I created the following asset, which is just the (embedded) version of a single triangle, with extension declarations for testing:
{ "extensionsUsed": [ "KHR_texture_transform", "EXAMPLE_unsupported_extension" ], "extensionsRequired": [ "KHR_texture_transform", "EXAMPLE_unsupported_extension" ], "scene" : 0, "scenes" : [ { "nodes" : [ 0 ] } ], "nodes" : [ { "mesh" : 0 } ], "meshes" : [ { "primitives" : [ { "attributes" : { "POSITION" : 1 }, "indices" : 0 } ] } ], "buffers" : [ { "uri" : "data:application/octet-stream;base64,AAABAAIAAAAAAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAACAPwAAAAA=", "byteLength" : 44 } ], "bufferViews" : [ { "buffer" : 0, "byteOffset" : 0, "byteLength" : 6, "target" : 34963 }, { "buffer" : 0, "byteOffset" : 8, "byteLength" : 36, "target" : 34962 } ], "accessors" : [ { "bufferView" : 0, "byteOffset" : 0, "componentType" : 5123, "count" : 3, "type" : "SCALAR", "max" : [ 2 ], "min" : [ 0 ] }, { "bufferView" : 1, "byteOffset" : 0, "componentType" : 5126, "count" : 3, "type" : "VEC3", "max" : [ 1.0, 1.0, 0.0 ], "min" : [ 0.0, 0.0, 0.0 ] } ], "asset" : { "version" : "2.0" } }`The default behavior here is also the expected one:
When loading this with
gltfModelKey.setStrict(false), then the result is only a log message:SCHWERWIEGEND: Extension EXAMPLE_unsupported_extension is required for this file. The behavior of the loader is unspecified.The main point here is: I'm not sure how to "consolidate" these tests. That
TriangleUnsupportedExtensionRequired.gltffile is currently not checked in (but it is small, so it could probably be added...).And I did the tests with manual tweaks to the
TestGltfLoadingclass. I can always check in "my local, current state" of this class. But I wondered whether there is a way to streamline the different tests that should be done, without someone having to twiddle with// commented-outlines.However, for the time being, here is my current local state of the
TestGltfLoading, in case someone wants to try it out locally: