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

Process for adding and promoting glTF extensions #404

Closed
pjcozzi opened this issue Sep 10, 2015 · 25 comments
Closed

Process for adding and promoting glTF extensions #404

pjcozzi opened this issue Sep 10, 2015 · 25 comments

Comments

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2015

@tparisi @mlimper for your review. Please provide feedback ASAP.

Also, see the original extensions design discussion in #61.

Summary

glTF extensions extend the base glTF model format. Extensions can introduce new properties (including properties that reference external data), new parameter semantics, and new container formats. Extensions are written against a specific version of glTF and may be promoted to core glTF in a later glTF version.

Extension Mechanics

All glTF object properties (see glTFProperty.schema.json) have an optional extensions object property that can contain new extension-specific properties. This allows extensions to extend any part of glTF, including geometry, materials, animations, etc. Extensions can also introduce new parameter semantics, reserved ids, and define previously required properties as optional, e.g., because a property is redundant due to the extension.

Extensions can't refine existing glTF properties to mean something else.

Examples include:

  • New properties: CESIUM_RTC introduces a center property:
"extensions": {
    "CESIUM_RTC": {
        "center": [6378137.0, 0.0, 0.0]
    }
}
  • New parameter semantics: CESIUM_RTC introduces the CESIUM_RTC_MODELVIEW semantic.
  • Reserved ids: EXT_binary_glTF introduces an explicitly named buffer called binary_glTF.
  • Make properties optional: EXT_binary_glTF does not require a shader object's uri property because the extension accesses the shader source using a bufferView, e.g.,
"a_shader" : {
    "extensions" : {
        "binary_glTF" : {
            "bufferview" : // ...
        }
    }
}

All extensions used in a model are listed as strings in the top-level allExtensions array, e.g.,

"allExtensions" : ["CESIUM_RTC"],
"extensions" : {
    "CESIUM_RTC" : {
      // ...
    }
}

Creating Extensions

TODO: Link to a template extension (I'll do this ASAP)

Promoting Extensions

Vendor extensions

A list of vendor prefixes is maintained in the glTF GitHub repo wiki. Any vendor can request an extension prefix by submitting an issue to the glTF repo requesting one. All their extension names will start with the prefix, e.g., CESIUM_.

To include a vendor extension in the glTF Extensions Registry, a vendor opens a pull request in this repo. The extension is not yet (or sometimes ever) ratified by Khronos and therefore are not covered by the Khronos IP framework.

Multi-vendor extensions

When an extension is implemented by more than one vendor, it can receive the reserved EXT_ prefix. Submit an issue to the glTF repo to request promoting a vendor extension to a multi-vendor extension. The extension is not ratified by Khronos and therefore are not covered by the Khronos IP framework.

Khronos extensions

Khronos extensions use the reserved KHR_ prefix and are ratified by Khronos and therefore are covered by the Khronos IP framework. Khronos members can submit an extension for ratification, which is then voted on by the Khronos Board of Promoters.

Extensions and Extras

In addition to extensions, the extras object can also be used to extend glTF. This is completely separate from extensions.

All glTF object properties allow adding new properties to an extras object sub-property, e.g.,

"a-vertex-shader-id" : {
    "name": "user-defined vertex shader name",
    "uri" : "vertexshader.glsl",
    "type": 35633,
    "extras" : {
        "Application specific" : "The extra object can contain any properties."
    }
}

This enables glTF models to contain application-specific properties without creating a full glTF extension.

The initial prefixes wiki would look like:

  • KHR_ - Khronos extensions
  • CESIUM_ - Cesium extensions
  • EXT_ - Multi-vendor extensions
  • WEB3D_?
@jdmarble
Copy link

There seems to be an inconsistency. Do Khronos extensions start with "KHR_" or "ARB_"?

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

Good eye, thanks @jdmarble. Fixed. First I had _ARB, but then I changed it to KHR_.

@fabrobinet
Copy link
Contributor

define previously required properties as optional, e.g., because a property is redundant due to the extension.

This is the only bit that I am not sure about. If this came up because the work on some extensions ...then you should either consider putting those properties as optional in the core spec or change the way that extension works.
Allowing extensions to change the required/optional properties seems to be quite confusing to me long term... That could be manageable for files relying on just one extension, but how will that work if one file requires multiple extensions with different required/optional properties...?

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

@fabrobinet thanks for the feedback.

The specific case this came up in is the binary glTF extension. In this extension, a shader now uses a bufferView to access its shader source so the uri is not required because it is redundant and potentially ambiguous.

Perhaps, instead the extension could say that the bufferView takes precedence over uri and the uri can be ignored (which is almost the same thing as saying it is optional, but is safer for the multiple extension case). To be conformant, uri would still need to be a valid URI (empty data uri I suppose), which is silly, but tolerable I suppose. What do you think?

@mlimper
Copy link
Contributor

mlimper commented Sep 14, 2015

The name "allExtensions", followed by the "extensions" object, could be found a bit confusing by people not familiar with the format - could maybe be named "usedExtensions" instead?

Apart from that, I agree with @fabrobinet, defining previously required properties as optional later on could become a bit difficult... it would also make it harder to write validators for glTF files, for example.

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

The name "allExtensions", followed by the "extensions" object, could be found a bit confusing by people not familiar with the format - could maybe be named "usedExtensions" instead?

I agree, it is slightly better. This was also suggested in #61 (comment).

Anyone else have feedback on "allExtensions" vs. "usedExtensions"?

@tparisi
Copy link
Contributor

tparisi commented Sep 14, 2015

I like

extensionsUsed

@tparisi
Copy link
Contributor

tparisi commented Sep 14, 2015

@pjcozzi also can I see an example in practice?

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

extensionsUsed is OK with me. It would look like:

"extensionsUsed" : ["one", "two"],
"extensions" : {
    "one" : {
        // properties specific to "one" and apply to the entire asset
    }
}
// elsewhere, extension "two" is used

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

I agree with @fabrobinet, defining previously required properties as optional later on could become a bit difficult... it would also make it harder to write validators for glTF files, for example.

@mlimper can you update the binary glTF extension spec? Or give me commit access to your fork and I can do it.

@pjcozzi pjcozzi added the tools label Sep 14, 2015
@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 14, 2015

At this point, I think we have converged: extensions can't change parameters to be optional and we will rename "allExtensions" to "extensionsUsed". Please let me know if there is any other feedback and I will tighten up the writeup for the spec by tomorrow.

@mlimper
Copy link
Contributor

mlimper commented Sep 15, 2015

I agree with @fabrobinet, defining previously required properties as optional later on could become a bit difficult... it would also make it harder to write validators for glTF files, for example.

@mlimper can you update the binary glTF extension spec? Or give me commit access to your fork and I can do it.

I checked the current state, so far didn't find any part that explicitly talks about changing existing properties, or that uses "allExtensions". I'll keep an eye on it though during further editing, you now also have write access to the fork.

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 15, 2015

@mlimper I made the update in 6bf2fbe.

@tparisi
Copy link
Contributor

tparisi commented Sep 15, 2015

Guys this is a nit, but I am not crazy about the extension .bgltf . Can we consider a different name?

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 15, 2015

@tparisi what do you suggest for the extension? The only suggestion I have is .bg (Binary glTF).

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 15, 2015

Also, please move all Binary glTF discussion to #400 so we can keep this issue focused on general extensions for the archive. Sorry for diverting it.

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 15, 2015

Based on this discussion, check out the full extensions info in the spec-extensions branch:

https://github.com/KhronosGroup/glTF/tree/spec-extensions/extensions

Any other feedback before I open the pull request into the spec-1.0 branch?

@tparisi
Copy link
Contributor

tparisi commented Sep 15, 2015

Looks great. Please issue PR and I will link to the extensions doc from the spec's Concepts section.

Tony Parisi
CTO at Large
415.902.8002

On Sep 15, 2015, at 4:16 PM, Patrick Cozzi [email protected] wrote:

Based on this discussion, check out the full extensions info in the spec-extensions branch:

https://github.com/KhronosGroup/glTF/tree/spec-extensions/extensions

Any other feedback before I open the pull request into the spec-1.0 branch?


Reply to this email directly or view it on GitHub.

@mlimper
Copy link
Contributor

mlimper commented Sep 16, 2015

Looks great to me! Just one small question, can "Dependencies" in the extensions template also contain dependencies to other extensions?

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 16, 2015

can "Dependencies" in the extensions template also contain dependencies to other extensions?

Yes, for sure. As we introduce new extensions (like SRC, mesh compression, external JSON), I expect us to flush out the extension template a bit more. I'd actually like to get more extensions queued up as soon as possible so we can come across any issues early.

@pjcozzi pjcozzi removed the tools label Sep 17, 2015
@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 17, 2015

The results of this discussion are all captured in the glTF Extension Registry: https://github.com/KhronosGroup/glTF/tree/spec-1.0/extensions

@pjcozzi pjcozzi closed this as completed Sep 17, 2015
@tparisi
Copy link
Contributor

tparisi commented Sep 17, 2015

Hey guys just a naming nit: in the interest of brevity how about we call the top-level dictionary extensions instead of extensionsUsed ? @pjcozzi @mlimper

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 17, 2015

extensionsUsed is an array, which is the list of extensions used in the asset so a runtime can quickly check if it can render the model.

There is also a top-level extensions, which is where extensions to the top-level asset are stored (all glTF object properties have an optional extensions object property).

@tparisi
Copy link
Contributor

tparisi commented Sep 18, 2015

Oh I see - the top-level can also have extensions. Never mind; just trying for brevity and simplicity wherever possible.

@pjcozzi
Copy link
Member Author

pjcozzi commented Sep 18, 2015

Exactly...and, in general, I am all for your quest for brevity and simplicity.

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

No branches or pull requests

5 participants