glTF icon indicating copy to clipboard operation
glTF copied to clipboard

EXT_mesh_gpu_instancing: Suggest extensionRequired usage

Open zeux opened this issue 1 year ago • 6 comments

Using this extension without specifying it as required is guaranteed to lead to significant differences in appearance between renderers that support it and renderers that don't due to how it is specified.

Fixes #2402.

zeux avatar May 23 '24 21:05 zeux

There are two sample models in this folder. Should we update it with extensionsRequired?

https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Vendor/EXT_mesh_gpu_instancing/samples/teapots_galore/teapots_galore.gltf#L286-L288

https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Vendor/EXT_mesh_gpu_instancing/samples/teapots_galore_id/teapots_galore_id.gltf#L320-L322

EDIT: Should these models be in https://github.com/KhronosGroup/glTF-Sample-Assets instead of in the extension folder?

bghgary avatar May 23 '24 21:05 bghgary

There’s an instancing example (SimpleInstancing) in glTF- Sample-Assets so I don’t know if we need a second one; not sure what the policy is wrt having examples in this repository. I plan to submit a PR for SimpleInstancing if this gets merged but I didn’t realize there are examples in this repository as well…

zeux avatar May 23 '24 21:05 zeux

@zeux Sorry, my question isn't directed at you specifically, though you are welcome to comment also of course. Maybe @lexaknyazev and others can chime in.

not sure what the policy is wrt having examples in this repository

I'm not sure either. I think this might be the only extension with sample assets inside the extension folder itself.

bghgary avatar May 23 '24 21:05 bghgary

That's 100% an oversight. Samples should be in the dedicated repo.

lexaknyazev avatar May 23 '24 21:05 lexaknyazev

I agree that the models should not be here. (Moving them to glTF-Sample-Assets will require some work (just to make clear that it's not CTRL-X/V), and ... I think that only the one with the _ID should be enough...)

javagl avatar May 24 '24 00:05 javagl

Please let me know if any further changes are needed on this PR (or if there are no plans to merge it).

zeux avatar Jul 04 '24 21:07 zeux