You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
KHR_node_visibility is implemented by PR #93722 but is not yet ratified by Khronos. In the meantime, I tried to implement this in GDScript, but I found that it was impossible for 2 reasons:
Visibility is not preserved when moving from ImporterMeshInstance3D to MeshInstance3D during import.
Invisible nodes are discarded during export.
This PR extracts the fixes to these problems the first problem from PR #93722 to allow implementing KHR_node_visibility in GDScript. EDIT: For import only.
The fix for not discarding invisible nodes may be controversial, especially since due to the lack of the user-facing settings in PR #93722, there is no exposed setting to customize this. Still, I think this is a good change. I would exclude this part when cherry-picking to 4.3 though. Or, I can remove it if I really must, and at least we can fix importing.
I would move the not discarding invisible nodes to the node visibility pr unless you want to do the full forward compatibility toggle dance. It's up to you.
The problem with adding a user-facing option is that the three options should be "Include & Required," "Include & Optional," and "Exclude."
Without KHR_node_visibility, the first two would behave the same.
This causes an option that would change meaning in a future release.
Another option is that I could add in every bit of code except the reading/writing to GLTF part, making it trivially easy for people to implement KHR_node_visibility in GDScript, but it would be mostly unused code out of the box.
I do not favour this because it doesn't default on, so any bugs will be hidden for maybe a year.
aaronfranke
changed the title
GLTF: Preserve node visibility on import and keep on export
GLTF: Preserve node visibility on import
Nov 6, 2024
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
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.
KHR_node_visibility is implemented by PR #93722 but is not yet ratified by Khronos. In the meantime, I tried to implement this in GDScript, but I found that it was impossible for 2 reasons:
Invisible nodes are discarded during export.This PR extracts the fixes to
these problemsthe first problem from PR #93722 to allow implementing KHR_node_visibility in GDScript. EDIT: For import only.The fix for not discarding invisible nodes may be controversial, especially since due to the lack of the user-facing settings in PR #93722, there is no exposed setting to customize this. Still, I think this is a good change. I would exclude this part when cherry-picking to 4.3 though. Or, I can remove it if I really must, and at least we can fix importing.