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

Import: prettify bones #946

Merged
merged 9 commits into from
Mar 9, 2020
Merged

Import: prettify bones #946

merged 9 commits into from
Mar 9, 2020

Conversation

scurest
Copy link
Contributor

@scurest scurest commented Feb 25, 2020

Depends on #945. You can see how the local rotation stuff gets used if you want.


Fixes #324.
Also see #455.

This attempts to rotate/resize edit bones so they're more readable. It's based on some simple heuristics. There's lots of stuff that you might want to do differently, but I think this is a good start. Anyway all the tools to set lengths and rotate editbones are now in place at least.

An import option was added to control the heuristic for bone directions (it doesn't affect bone lengths). The only two options currently are:

Blender (+Y)

This rotates bones so their tips lie on their local +Y axis (in glTF space). This option will round-trip bone directions when re-importing models exported from Blender.

(Also see #879)

Temperance (this is the default)

Name is provisional. It works well for many different models so I made it the default. It tries to place the bone tip on whichever axis is closest to the centroid of its children. Leaf bones are rotated the same way as their parents.

final

Generally it's not too bad for character models at least.

...instead of adding an extra node.
Since edit bones point along their local +Y axis, this will let us
pick the way they point.

As a demonstration this works, rotate all bones back to the way they
used to point before the vnode code was merged.
To demonstrate it works, set all lengths to 0.5
When we don't change coordinates, also don't change bone rotations.

This makes it easier to see what axis a bone points along in glTF:
import with debug_value == 100 and turn on display axes for the
armature.
@julienduroure julienduroure added enhancement New feature or request importer This involves or affects the import process Skinning_&_Rigging labels Feb 25, 2020
@fire
Copy link

fire commented Feb 25, 2020

Will this make the 2.83 release?

@julienduroure
Copy link
Collaborator

Yes, plan is to get it in time for 2.83, if everything get smooth

@fire
Copy link

fire commented Feb 28, 2020

Do you know what it'll take to have an option that'll force align it to https://github.com/pixiv/three-vrm/blob/v0.3.0/src/vrm/material/shaders/mtoon.frag standards? I'm not sure the exact details.

It's something like make all bone rotations line up with the global axes.

@scurest
Copy link
Contributor Author

scurest commented Feb 28, 2020

@fire Not sure what you're asking. What should I be looking for in that fragment shader?

@fire
Copy link

fire commented Feb 28, 2020

https://github.com/vrm-c/vrm-specification/blob/master/specification/0.0/README.md#vrm-rules

No local rotation for bone

My bad, I posted the relevant page. I'm not sure where to put this report, but here sounds like a good place for an option.

@julienduroure
Copy link
Collaborator

Every tests I did are OK :)
A very big thanks for this PR!
Any reason why this PR is still draft?

@julienduroure
Copy link
Collaborator

One more question @scurest : If I merge this PR, do I need to merge #945 first, or this PR #946 contains all modification from #945 too?

This is for forwards compatibility. A future heuristic might also
affect bone lengths, not just tips.
@scurest
Copy link
Contributor Author

scurest commented Mar 4, 2020

No problem.

If I merge this PR, do I need to merge #945 first, or this PR #946 contains all modification from #945 too?

This PR contains all the commits from #945 (branch for #945 is pointing at the second commit here).

Any reason why this PR is still draft?

Mostly because it depends on #945.

Do you want to change anything about the new option though? Since it'll be part of the UI/Python API. I just changed bone_tip_heuristic to bone_heuristic since a future heuristic might also affect bone lengths.

Eg. different name than "Temperance"? "strategy" instead of "heuristic"?

@fire
Copy link

fire commented Mar 5, 2020

"Temperance Strategy" sounds better. I don't see people use the word heuristic often.

@julienduroure
Copy link
Collaborator

Well, English is not my primary language, so not sure what is better

@scurest scurest marked this pull request as ready for review March 8, 2020 18:38
@scurest
Copy link
Contributor Author

scurest commented Mar 8, 2020

I'll mark it as ready since #945 looked good. Merging just this should automatically close #945 too.

@julienduroure julienduroure added this to the Blender 2.83 milestone Mar 9, 2020
@julienduroure julienduroure merged commit f3725f0 into KhronosGroup:master Mar 9, 2020
@scurest scurest deleted the prettify-bones branch March 9, 2020 15:23
@julienduroure
Copy link
Collaborator

Merged :)
Thanks again to @scurest for this big improvement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request importer This involves or affects the import process Skinning_&_Rigging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imported armature issues
3 participants