-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add note about attribute name disambiguation #2514
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
base: main
Are you sure you want to change the base?
Add note about attribute name disambiguation #2514
Conversation
@@ -137,8 +137,11 @@ All glTF object properties (see [glTFProperty.schema.json](../specification/2.0/ | |||
|
|||
Extensions can't remove existing glTF properties or redefine existing glTF properties to mean something else. | |||
|
|||
Examples include: | |||
* **New properties**: `KHR_texture_transform` introduces a set of texture transformation properties, e.g., | |||
Extensions may add new properties and values, such as attribute semantics or texture mime types. In all Khronos (KHR) extensions, and as best practice for vendor extensions, these feature additions are designed to allow safe fallback consumption in tools that do not recognize an extension in the `extensionsUsed` array. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
texture mime types
Should be "media types"
In all Khronos (KHR) extensions, and as best practice for vendor extensions, these feature additions are designed
This sounds not quite right. We cannot say "are designed" (a fact of now that may not hold true in the future) in relation to "best practice for vendor extensions" (merely a suggestion). Please rephrase.
Please also add a short "Extension naming" section before "Extension mechanics" stating that an extension name consists of the uppercase prefix followed by an underscore and the feature name spelled with snake case convention (with a note that some old extensions may not follow this rule for historical reasons); also that section should state that extension names should not have special characters beyond underscore for splitting words, and in particular, they must not contain a colon symbol as it's used for disambiguating extension-defined vertex attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That part of the text was taken from the current state, which also already contains most of the information about the extension naming at https://github.com/KhronosGroup/glTF/blob/65f4ed6b1b78d61991a9f14e25e9e817867d04bc/extensions/README.md#naming
Moving that further to the top probably makes (as well as adding the hint about the :
there). And I'll try to come up with a rewording for the original text part...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. The current naming section should certainly be moved higher and btw all links in it are broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather as a note to myself: There are other broken in-document links at https://github.com/KhronosGroup/glTF/blob/65f4ed6b1b78d61991a9f14e25e9e817867d04bc/extensions/README.md#adding-additional-features-to-gltf
I'll probably do a broader cleanup here, and move that information into some dedicated 'ExtensionDevelopment.md' or so.
Fixes #2111 :
As discussed in the issue and during the 3D Formats calls, the disambiguation between attribute semantic names that are defined by different extensions should happen by prefixing the attribute name with the full name of the extension, followed by a
:
colon.This PR adds the corresponding information to the
extensions/README.md
, in the "Extension Mechanics" section. Apart from minor restructuring, this mainly adds a section "New Attributes" that defines these rules for extension-defined attributes, and gives an example.Direct link to the new state of the "Extension Mechanics" section, with the "New attributes" section
I considered to use a real-world example instead of that
EXT_example_extension
. But given that there is not yet any extension merged that requires this disambiguation, I chose to use a dummy example name.