-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add versioning info #899
Add versioning info #899
Conversation
"description": "FLOAT" | ||
}, | ||
{ | ||
"type": "integer" |
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.
Why is this needed? Are there integers that are valid here, other than the ones listed above?
"enum": [ "MAT4" ] | ||
}, | ||
{ | ||
"type": "string" |
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.
Same comment here with string
. I'm not a JSON Schema expert, but I tried actually loading this branch into VSCode's JSON validator, and having this type
at the end of the enum list allows arbitrary strings (or arbitrary ints in the comment above) to validate as correct. Removing this type
means it will only validate with an exact match to one of the named enum values.
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.
For example:
"componentType": 99999,
"type": "UNKNOWN"
This validates as allowed, because it matches anyOf
integer
and anyOf
string
.
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.
There are a couple of ways these schemas will get used;
- Viewers/Loaders can validate a file before trying to display it. The additional objects in anyOf allow loaders to validate a future version and continue to load it while ignoring any new elements.
- Validators like in vs-code or glTF-validator may want to ensure the file is exactly 2.0 and not 2.1. Loosening up the schema will not be useful for this type of use case. If strict validations are required then these tools will need to use a stricter version of the schema that is specific to a version and doesn't do the forwards compatibility.
Do we need to consider making a "strict" version of the schema? By default the schema should be flexible because its still valid 2.0 with the forwards compat capabilities. We could do some script to create the stricter version that specifically checks for 2.0 without forwards compat.
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.
I see. What behavior should a 2.0-capable loader take if it finds a "future" value that it doesn't understand? Will it just look for a default value in the schema?
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.
The short answer is - it can be ignored. The long answer is - future updates will have to be designed such that new values can be ignored safely. Just pasted some examples we have been discussing below that goes into more detail.
The working group has been discussing this. Pasting it here so everyone can read. The examples are speculations of features that could be added in future versions and are only meant to show potential approaches of adding them. To support forwards compatibility, whenever a new feature is added the spec needs to allow for the following requirements:
Requirement 1. is essentially backwards compatibility- you cannot change existing behavior in a spec update. For requirement 2.a a glTF 2.0 parser should still be able to read a 2.1 file and completely ignore any new features. For example say glTF 2.1 adds lighting capability by:
(This is a completely made up example to show how this will work)
A 2.0 parser can ignore any additional properties it finds whereas a 2.1 parser can correctly interpret those additional parameters. For requirement 2.b any new features are ignored but a fall back behavior to 2.0 should exist.
A 2.1 client can read and understand the “draco_mesh” element. A 2.0 client can simply ignore it and use the uncompressed geometry. Requirement 3. will technically break forwards compatibility, but asset creators may want to choose to explicitly opt out of compatibility and require a specific version. The draco mesh compression is a good example where someone could choose to require a minimum version. For a client-server type of scenario where a JSON file points to the bin files, you can have both the compressed mesh and uncompressed mesh and a client can just download the one it understands. However, for a self contained binary glTF file scenario, you may only want to keep the compressed version since the whole point was to reduce the file size. In this scenario an author could choose to explicitly require a minimum version of 2.1. The JSON for such a file must still contain the required elements so that it validates, but it could just point to an empty geometry which will never gets used. Requirements 2.a and 2.b are satisfied by allowing additional properties in all nodes of the JSON. Enums provide an interesting challenge and example below show how to update them in a forwards compatible manner :
Only a client that understands glTF 2.1 will understand the additionalsamplers property and will read the sampler which uses spline interpolation. Older samplers should simply ignore the additionalSamplers property and read the 2.0 sampler property to use linear interpolation. |
@sbtron @bghgary @pjcozzi
Same question about |
@lexaknyazev following Khronos versioning, the format for I don't think we will have other variations other than perhaps |
specification/2.0/README.md
Outdated
@@ -131,6 +132,14 @@ Version 2.0 of glTF does not define compression for geometry and other rich data | |||
|
|||
> The 3D Formats Working Group is developing partnerships to define the codec options for geometry compression. glTF defines the node hierarchy, materials, animations, and geometry, and will reference the external compression specs. | |||
|
|||
## Versioning | |||
|
|||
Any updates made to glTF in a minor version will be backwards and forwards compatible. Backwards compatibility will ensure that any client implementation that supports loading a glTF 2.x file will also be able to load a glTF 2.0 file. Forwards compatibility will allow a client implementation that only supports glTF 2.0 to load glTF 2.x files while gracefully ignoring any new features it does not understand. |
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.
Perhaps "minor or revision version"
specification/2.0/README.md
Outdated
@@ -131,6 +132,14 @@ Version 2.0 of glTF does not define compression for geometry and other rich data | |||
|
|||
> The 3D Formats Working Group is developing partnerships to define the codec options for geometry compression. glTF defines the node hierarchy, materials, animations, and geometry, and will reference the external compression specs. | |||
|
|||
## Versioning | |||
|
|||
Any updates made to glTF in a minor version will be backwards and forwards compatible. Backwards compatibility will ensure that any client implementation that supports loading a glTF 2.x file will also be able to load a glTF 2.0 file. Forwards compatibility will allow a client implementation that only supports glTF 2.0 to load glTF 2.x files while gracefully ignoring any new features it does not understand. |
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.
Throughout this section, replace "glTF 2.0 file" with "glTF 2.0 asset", which is more consistent with the rest of the spec. (Looks like the spec itself could use a tweak or two here as well if you do a quick search).
specification/2.0/README.md
Outdated
@@ -176,6 +185,9 @@ Each glTF asset must have an `asset` property. In fact, it's the only required t | |||
} | |||
``` | |||
|
|||
> **Implementation Note:** Client implementations should first check whether a `minVersion` property is specified and ensure both major and minor versions can be supported. If no `minVersion` is specified, then clients should check the `Version` property and ensure the major version is supported. Clients that load [GLB format](GLB_FORMAT.md) should also check for the `minVersion` and `Version` properties in the JSON chunk as the version specified in the GLB header only refers to the GLB container version. |
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.
Throughout, Version
-> version
. Property names should be camelCase.
+1 from me. Just trivial comments. |
@pjcozzi |
OK with me, but let's confirm on the call today about dropping |
}, | ||
{ | ||
"enum": [ "STEP" ], | ||
"description": "When interpolation is `\"STEP\"`, animated value remains constant to the value of the first point of the timeframe, until the next timeframe." | ||
"enum": [ "STEP" ] |
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.
We should make a note to restore these once wetzel is upgraded to handle them. The whole point of my lobbying for the change from non-standard to anyOf
was specifically to get these descriptions per-enum-value.
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.
Agreed. Descriptions are handled, but long descriptions make the generated text hard to read. Since these didn't have descriptions initially, I just put them back to the original for now.
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.
I opened #955 for this.
Added explanation of how we envision glTF will version with backwards and forwards compatibility in the 2.x timeframe.
https://github.com/sbtron/glTF/tree/Versioning/specification/2.0#versioning
Updated schema to allow for forwards compatibility