Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplifying targets for extras/extensions? #118

Closed
zeux opened this issue Jun 23, 2020 · 3 comments · Fixed by #231
Closed

Simplifying targets for extras/extensions? #118

zeux opened this issue Jun 23, 2020 · 3 comments · Fixed by #231

Comments

@zeux
Copy link
Contributor

zeux commented Jun 23, 2020

As I've been reading extras/extensions code more I've been wondering:

Would it be worthwhile to reduce the scope of extras/extensions by removing them from places where they aren't obviously necessary?

Three specific targets come to mind:

  • cgltf_accessor_sparse, as I don't think extending sparse accessors is particularly interesting
  • cgltf_texture_view, as use of extras/extensions here makes adds friction to adding more texture views in terms of code size
  • cgltf_pbr_metallic_roughness (which has extras but not extensions), as it's inconsistent with other material structs

The motivation for the last two is that it would make it slightly less painful to add support for new KHR_ material extensions, since adding texture views and new material structs wouldn't require as much extra code.

I'm not aware of existing extensions that extend either of the above, except for KHR_texture_transform which is supported natively. Obviously there may exist unpublished extensions that do so...

(I had one other crazy idea but that's probably not actually a very good one, where extensions/extras could be stored in separate arrays along the lines of struct { void* key; char* name; char* value } where key would point to the cgltf_ element that it's attached to, which would make it easier to generalize the parsing and destruction of associated data - but that would make it harder to use these so it's probably not a great tradeoff)

@jkuhlmann
Copy link
Owner

Looks like like that wouldn't cause too much pain to remove extensions for those three targets. We could just do it and re-add in case anyone needs it in the future.

As for your idea of storing the extensions and extras separately: If we added some functions to add/remove/read that data it wouldn't make the usage for the end user that much harder, would it?

@zeitt
Copy link
Contributor

zeitt commented Jul 7, 2020

Not sure why I didn't add extensions to cgltf_pbr_metallic_roughness (I planned to add to everything that has extra data, exclusing what is already and extension). However, since those three doesn't have any public extensions I don't see any problem removing extensions from those.

And I think storing extensions (&extras) separately might not be good idea, as everything else is accessed similarly as one would access it from glTF json (indices are just replaced with pointers).

@jkuhlmann
Copy link
Owner

I've put together my current thoughts on extensions in #125. Feedback would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants