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

Flexible extension support #125

Open
jkuhlmann opened this issue Sep 17, 2020 · 2 comments
Open

Flexible extension support #125

jkuhlmann opened this issue Sep 17, 2020 · 2 comments

Comments

@jkuhlmann
Copy link
Owner

We've got two different mechanisms for extensions as of now:

  1. A number of KHR extensions are directly built into cgltf.
  2. Unknown extensions can be accessed via the cgltf_extension array in most objects.

As the number of official extensions is growing and more vendor extensions pop up, we should consider implementing a more flexible and modular approach.
I'm open for ideas on how we could change this.

This is currently my best idea here:

  • We add a map to each node that can have extensions. It maps extension name to a void* that can be casted to a struct that holds the respective extension's data.
  • Extensions can provide convenience functions taking the struct as a parameter to do the casting and map lookup in a safer way.
  • In order to fill these structs, extensions setup callback functions for a given extension name and node type in cgltf_options. These callbacks are then called when cgltf is parsing glTF data in order to allocate and fill the structs that are stored in the nodes.
  • Extensions probably need to provide deallocation/destroy functions for the structs they stored in the nodes.

At the bottom line, that would mean for all nodes that an extension can extend, it needs to support three functions: one to allocate/initialize the struct holding the extension data, one to retrieve it from the extension map in the node, one to free/destroy it.

The extension map could be, for now at least, just an array mapping a hash of the extension name to the data. Considering we probably won't have more than a handful of extensions on each node, this should be sufficient.

What do you think? Did a miss something? Do you have a better idea?

@LxLasso
Copy link

LxLasso commented Sep 20, 2020

Looks solid to me! Here are a few thoughts;

How will the extension setup callback know what it is setting up?

  • One callback per object type? VENDOR_ext_setup_asset(..), VENDOR_ext_setup_node(..), etc
  • An object type enum? VENDOR_ext_setup_asset(cgltf_object_type object_type, ..)
  • A string? VENDOR_ext_setup_asset(char* object, ..)

Is it enough to know the object type during setup or is the hierarchy / "path" also required in some situations?

Write! When setting up the callback functions in cgltf_options, I would much like to have a fourth one for writing - something like VENDOR_ext_write(cgltf_write_context* context, cgltf_extension* extension).

Extension "user data". Is there a case for extension setup or write implementations to need user data? Would probably suggest leaving it for later until a need pops up.

@pixeljetstream
Copy link
Contributor

Maybe instead of map just an extension list per object-type, similar to how Vulkan has pNext and sType. We also have a bunch of unofficial extensions here, and I agree a separate file to manage those could be nice. However several json utilities for loader/saver would need to be exposed I think. There is also pointer fixup and validation etc.
Allocation would go through the exisitng options api

That said so far just hacking cgltf itself was simple enough ;) and one gets the easye to use structs in return...

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

No branches or pull requests

3 participants