-
Notifications
You must be signed in to change notification settings - Fork 1.2k
KHR_skin_strict #1747
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
Open
donmccurdy
wants to merge
3
commits into
KhronosGroup:main
Choose a base branch
from
donmccurdy:feat-KHR_skin_strict
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
KHR_skin_strict #1747
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # KHR\_skin\_strict | ||
|
|
||
| ## Contributors | ||
|
|
||
| * TBD | ||
|
|
||
| ## Status | ||
|
|
||
| Draft | ||
|
|
||
| ## Dependencies | ||
|
|
||
| Written against the glTF 2.0 spec. | ||
|
|
||
| ## Overview | ||
|
|
||
| `KHR_skin_strict` introduces no new features or schema to the glTF file format. Instead, it defines a strict subset of the existing skinning specification, and imposes requirements — beyond those of the original specification — on tools creating glTF assets with a `skins` entry. | ||
|
|
||
| Implementations of skinned mesh animation vary between engines, for historical and performance reasons. This stricter skinning definition increases portability and simplicity for client implementations, at the cost of some flexibility and increased responsibilities for authoring tools. | ||
|
|
||
| When used, `KHR_skin_strict` must be an *optional* extension. Because it is a subset of the original specification, clients that do not recognize the extension can always safely read files that include the extension, so inclusion in the `extensionsRequired` list is neither necessary nor allowed. Presence of the `KHR_skin_strict` extension acts as a hint, for clients that recognize it, that the safer/stricter skinning requirements have been met. | ||
|
|
||
| Client implementations may choose to require that glTF assets containing one or more skins include the `KHR_skin_strict` extension, and reject assets that do not. Authoring tools are advised to include the extension whenever possible. | ||
|
|
||
| ## Requirements | ||
|
|
||
| * All skins *must* define the `skeleton` property. | ||
| * The `skeleton` node for a skin *must* be a joint, and the direct parent of the entire joint subtree for that skin. | ||
| * All joints in a skin *must* define a single fully-connected subtree, sharing a single root joint, with no non-joint nodes coming between the joints of the tree. | ||
| * A joint node *must* belong only to a single skin. | ||
| * A joint node *must not* define any other functional role, such as `node.mesh`, `node.camera`, or `node.skin`. Likewise, joint nodes cannot use extensions that instantiate resources in the scene (like `KHR_lights_punctual`). | ||
|
|
||
| > **NOTE:** A node is considered a joint for a skin if and only if the node is included in the skin's `joints` list. | ||
|
|
||
| ## Extending assets | ||
|
|
||
| The `KHR_skin_strict` extension is listed in `extensionsUsed`, but has no other explicit presence in the schema. | ||
|
|
||
| ```json | ||
| { | ||
| "extensionsUsed": ["KHR_skin_strict"], | ||
| ... | ||
| } | ||
| ``` | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
A node can't be both a joint itself and a parent of the subtree of all joints. Presumably this means (combining it with the following bullet point) "The joint nodes in a skin must form a subtree of the node graph and the skeleton node must be the root of that subtree"?
Why is it important for the skeleton node to be a joint? For models exported from Blender, a slight weakening is true: the joint nodes together with the skeleton node form a subtree of the node graph and the skeleton is the root of that subtree.
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.
Your clarification is what I'd meant to say, yes.
Maybe it's not necessary? But some of the side effects are useful:
I think we'd all agree that having a camera or a light as the skeleton root node would not be in the spirit of this extension. But perhaps the skeleton root node should be allowed to be a mesh — such that the skinned mesh is itself the parent of its bones?
Having the skeleton root sometimes be a joint, and sometimes not, seems like another moving piece to consider eliminating if it serves no purpose, but I'm not sure. More feedback on this would be welcome.
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.
Found out this was already discussed in #1669.
I'm not sure what exact balance you're going for between conservative/liberal here is. I just wondered if it could be motivated purely by a technical benefit, like skins being distinct subtrees is motivated by being able to calculate a bind pose TRS for joints.
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'm not sure... going to wait for more feedback on this question.