Implement KHR_node_visibility in the GLTF module
#93722
Open
+113
−21
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
fire
approved these changes
Jun 28, 2024
fire
reviewed
Jun 28, 2024
|
We had a lengthy discussion of the support of the visibility extension if we make it required.
|
aaronfranke
force-pushed
the
gltf-khr-node-visibility
branch
from
b8c324c
to
37f1c31
Compare
|
Updated the PR to make this configurable on export: 
|
AThousandShips
approved these changes
Jun 29, 2024
aaronfranke
force-pushed
the
gltf-khr-node-visibility
branch
from
37f1c31
to
f5914c3
Compare
aaronfranke
force-pushed
the
gltf-khr-node-visibility
branch
from
f5914c3
to
4daccf4
Compare
aaronfranke
force-pushed
the
gltf-khr-node-visibility
branch
from
4daccf4
to
aab1a20
Compare
akien-mga
changed the title
KHR_node_visibility in the GLTF module
|
Shouldn't we wait for Khronos to approve the draft proposal before merging its implementation? If they reject it, but we merge it, we'd be stuck maintaining a non-standard extension. |
|
@akien-mga Indeed, we can wait for approval. I doubt this will change but I cannot absolutely guarantee that. |
|
We discussed on Thursday that the moment Khronos ratifies the extension we are clear to merge. |
aaronfranke
force-pushed
the
gltf-khr-node-visibility
branch
from
aab1a20
to
2549d1a
Compare
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.
Implements
KHR_node_visibilityKhronosGroup/glTF#2410 in Godot, which allows glTF nodes to be marked as not visible. Both import and export are supported.Details: The glTF visibility system works the same as Godot's visibility system, only affecting visuals, which makes this really easy to implement. Since this extension was extremely simple, I decided to just implement it in GLTFDocument rather than writing a GLTFDocumentExtension class for it. I also made it a required extension on export, to ensure that user assets are always rendered correctly. However most scenes don't contain invisible objects, so this won't impact most scenes.
Aside from the GLTF module changes, this PR also adds 2 lines to the general scene importer code to ensure visibility is preserved when converting ImporterMeshInstance3D nodes to MeshInstance3D nodes.
Test file: cube_visibility_example.zip