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

Define conventions and best practices for naming and data layout in glTF extensions #2311

Open
aaronfranke opened this issue Aug 22, 2023 · 6 comments

Comments

@aaronfranke
Copy link
Contributor

aaronfranke commented Aug 22, 2023

When designing a new glTF extension, it would be nice to have guidelines for how to structure the data "the glTF way". These guidelines would be helpful to everyone for both Khronos extensions and vendor extensions.

Let me give an example. Does Khronos recommend using a "type" string enum to describe the behavior of an object, or are boolean flags preferred? For example, glTF cameras are defined using "type": "perspective" or "type": "orthographic". Is this the recommended way extensions should define types? What about "isOrthographic" or "isPerspective" booleans? Similarly, KHR_lights_punctual uses "type". Context: There is ongoing debate on whether physics body motion should have its behavior type defined with a string-based enum like "type": "dynamic" or "type": "kinematic", or if it should use an "isKinematic" boolean. Since "not kinematic" isn't the same meaning as "dynamic", an enum makes sense to me.

Another example, type-specific data. From looking at glTF cameras, it seems like the glTF way is to combine a type enum with sub-JSON, like { "type": "perspective", "perspective": { ... } }. However, I have seen other proposed layouts, including key-only { "perspective": { ... } } and flat { "type": "perspective", ... }. Which is preferred for new extensions?

Yet another example, extension name plurality. Do we want extension names to be singular, plural, or does it depend on the context? The current list of extensions are mixed. For example, we have extensions prefixed with KHR_materials_ and KHR_texture_. This is a weird inconsistency, why not KHR_material_ or KHR_textures_? There is no difference in usage, I looked at both KHR_materials_sheen and KHR_texture_basisu, each instance of the extension defines properties for only one thing (a material or a texture).

Last example, extension name prefixes. Do we want to group together related extensions with the same prefix? For example, KHR_materials_, KHR_mesh_, and KHR_texture_. The names of the physics extensions is currently an ongoing debate, I personally would like to see them all prefixed with KHR_physics_, but there is resistance to this.

To be clear, I am not suggesting that we rename or change existing ratified extensions, but I am suggesting to create guidelines to be consistent for all new extensions. It's better to have a guideline that all extension authors can follow rather than having debates about conventions for each extension. It's not productive to be stuck debating these minor details when we could instead agree on a guideline for all new extensions to follow.


The conventions and best practices for glTF extensions would have the following goals:

  • Ensure that the data is laid out in a consistent way between extensions, so that extension implementers are not surprised when implementing a new extension.

  • Ensure that data in an extension is designed with a similar level of flexibility as other extensions or glTF itself.

  • Minimize the friction if an extension is moved into a new namespace, such as if Khronos adopts the extension. If the original spec used a boolean flag, and Khronos uses a type enum, that's more friction compared to if the original spec started out with the data formatted in a way that Khronos considers best practice for glTF.

@javagl
Copy link
Contributor

javagl commented Aug 22, 2023

Short notes:

  • The cameras example is indeed one that is structurally very different from everything else in glTF. This may be justified, handwavingly, by the fact that the types simply have different properties (with znear and zfar being the common ones). One could imagine completely different structures here, maybe with some other combinations of 'optional' vs. 'required' properties. There's probably no silver bullet for this. One can only try to do something that "Makes Sense™".
  • This does indeed raise questions about that type thingy. Strictly speaking, the camera.type could be omitted, because it can be determined by the perspective or orthographic being present. Iff such a 'type' is supposed to be defined, then one can still argue that a type:string is easier to evolve, easier to make consistent, and scales better when new types are added in the future. (You don't want to end up with a bunch of isA, isB, ... isZ boolean fields, where a type:string could suffice)
  • The naming of extensions is one of the few things that already is defined, at https://github.com/KhronosGroup/glTF/tree/main/extensions#naming . Yes, it is not perfectly consistent. We were young :)
  • The task to describe some of these things in more detail is... somehow on my plate, via Documenting the extension development process #2225 , but... there didn't seem to be sooo much interest in that (although issues like this serve as a small 'bump' that may move this up on my TODO List...)

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Aug 22, 2023

Thanks for replying! I do indeed want to bump this up on your TODO list for this year, but also we can wait a few months for people to reply to this issue with feedback.

structurally very different from everything else in glTF.

The structure of KHR_lights_punctual is also similar, although it has a much flatter hierarchy, with color, intensity, and range properties on the base JSON, and a sub-JSON to describe cone angles. (also side note, it has a name property, which is kinda weird since most non-node things in glTF don't do this, they are only referred to by index). I was mistaken, most schemas allow for names, they just aren't used for reference.

The naming of extensions is one of the few things that already is defined

I see that the text you linked only covers the "scope" plurality, but not the "feature" plurality.

@donmccurdy
Copy link
Contributor

it has a name property, which is kinda weird since most non-node things in glTF don't do this...

Almost anything in glTF can have a name, even though it's not a unique identifier to be used for references — mesh primitives are the only important exception that comes to mind.

I see that the text you linked only covers the "scope" plurality, but not the "feature" plurality...

I'm not sure a rule would serve us well for the "feature". Generally the feature serves as a modifier to the scope. If it were a noun I suppose it should probably default to singular, but exceptions like variants make more sense plural and that's OK.


Also relevant:

@aaronfranke
Copy link
Contributor Author

aaronfranke commented Sep 5, 2023

Another point, we should consider standardizing references to vanilla glTF data structures.

For example, if a "mesh" key exists on an object, it should refer to an item in the "meshes" array. Conversely, if only one key is needed for referring to a mesh, it should be named "mesh". The same should be recommended with names such as "image", "texture", "material", "buffer", "bufferView", "accessor","node", etc. The exception would be if an extension needs to refer to 2 or more of these resources, then they may be named differently.

As far as I know, existing extensions are already consistent in this aspect (although in vanilla glTF itself there are some cases where this isn't followed). For example the WIP physics extensions are using "convex": { "mesh": 0 } and "trimesh": { "mesh": 0 } to refer to meshes in convex and trimesh physics shapes, which is good. It would make sense to put this down officially as recommended best practice.

@alteous
Copy link

alteous commented Nov 13, 2023

I'm using the "type": "foo" idiom in #2343 for consistency with the camera type, as mentioned in the original post. I agree it could technically be omitted.

@aaronfranke
Copy link
Contributor Author

For discussion about a specific case involving the best practice of using "type" instead of booleans, take a look at the bottom few posts of this discussion: eoineoineoin/glTF_Physics#4 (comment)

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

4 participants