godot icon indicating copy to clipboard operation
godot copied to clipboard

Implement `KHR_node_visibility` in the GLTF module

Open aaronfranke opened this issue 1 year ago • 8 comments
trafficstars

Implements KHR_node_visibility https://github.com/KhronosGroup/glTF/pull/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 (EDIT: Required by default, but users can customize this on export). 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

aaronfranke avatar Jun 28 '24 22:06 aaronfranke

We had a lengthy discussion of the support of the visibility extension if we make it required.

Required extension for something cosmetic is a bad idea IMHO

Required extensions will make godot look bad and create a support headache

fire avatar Jun 28 '24 22:06 fire

Updated the PR to make this configurable on export:

Screenshot 2024-06-28 at 4 14 08 PM Screenshot 2024-06-28 at 4 15 35 PM

aaronfranke avatar Jun 28 '24 23:06 aaronfranke

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 avatar Aug 26 '24 21:08 akien-mga

@akien-mga Indeed, we can wait for approval. I doubt this will change but I cannot absolutely guarantee that.

aaronfranke avatar Aug 26 '24 21:08 aaronfranke

We discussed on Thursday that the moment Khronos ratifies the extension we are clear to merge.

fire avatar Aug 26 '24 21:08 fire

Marking as draft to prevent accidental merging before ratification. However, the code is complete and ready for review (well, it has already been reviewed, but more review is welcome too).

aaronfranke avatar Sep 28 '24 11:09 aaronfranke

Honestly, it is only a single bool, its not that deep. and, it has a lot of value to godot when blender and godot share this eye icon in the same panel. I think it's worth supporting and then hotfixing later if Khronos happen to change their mind.

yankscally avatar Nov 06 '24 01:11 yankscally

KHR_node_visibility support has been added to the BabylonJS game engine: https://github.com/BabylonJS/Babylon.js/pull/15754

aaronfranke avatar Nov 29 '24 15:11 aaronfranke

My thinking at this time is we should add this to 4.5 and if KHR_mode_visibility is not released we ship it in Godot Engine 4.5.

Any arguments?

fire avatar Mar 08 '25 19:03 fire

I think the fact that BabylonJS added visibility should be a good argument that Godot could add it as well. Is there a reason to wait for ratification?

lyuma avatar Apr 13 '25 20:04 lyuma

@lyuma I would be happy to go through with this. Can you give it a review/approval?

aaronfranke avatar Apr 13 '25 20:04 aaronfranke

Thanks!

Repiteo avatar Apr 24 '25 22:04 Repiteo