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

Temperance setting does not fully correct for rotation in VRM models #967

Closed
lyuma opened this issue Mar 9, 2020 · 11 comments · Fixed by #995
Closed

Temperance setting does not fully correct for rotation in VRM models #967

lyuma opened this issue Mar 9, 2020 · 11 comments · Fixed by #995
Milestone

Comments

@lyuma
Copy link

lyuma commented Mar 9, 2020

Describe the bug
Temperance import Bone Dir setting appears to align all bones to cardinal axes on VRM files, leading to disconnected bones.

This is using the new Bone Dir import feature introduced in the pull request by @scurest and @julienduroure at #946

Temperance is already a huge improvement over the previous behavior, and it seems pretty close to what I want: most bones are aligned correctly. however, all bones appear to be aligned to cardinal axes. For example you can see even thumb bones are pointing straight outward, rather than connected. Hair bones point straight down, even if the hair is angled. Leg bones do not quite connect. On another VRM model by a different author, arm bones are also slightly disconnected.

VRM models are interesting, because they are really the worst-case scenario for import: the spec requires them to have bones aligned with the global +Y axis on export!

To Reproduce
Steps to reproduce the behavior:

  1. Download the attached Alicia model.
  2. Blender: Import -> glTF 2.0 -> Select Alicia.vrm / bone dir = Temperance (default)
  3. Join meshes, mesh -> viewport display -> display as wire
  4. Notice thumb bones and hair bones are disconnected. Leg bones are slightly disconnected at the knee.

Expected behavior
I expect bones to appear connected to each other: the parent's tail should be in the same spot as the child's head. Leaf bones can follow the algorithm presented in the original PR.

Screenshots
Screen Shot 2020-03-09 at 10 26 06 AM

.blend file/ .gltf
AliciaSolid_vrm-0.51.glb.zip

Originally available from https://github.com/vrm-c/UniVRM/tree/master/Tests/Models/Alicia_vrm-0.51 , renamed from .vrm to .glb

Version

  • OS: [e.g. macOS, linux]
    macOS 64 bit
  • Blender Version [e.g. 2.82, 2.83, or build date]
    Tested on both 2.83 Alpha [March 09, 16:22:23 - 07bdbeda8462] and 2.81

Additional context
It seems to happen in other VRM files too. Maybe I misunderstand what this Bone Dir setting is supposed to do, but the behavior appears very different from the screenshots in the PR linked above.

@scurest
Copy link
Contributor

scurest commented Mar 9, 2020

however, all bones appear to be aligned to cardinal axes

Yeah. This is the line that snaps to the local axes.

rot = nearby_signed_perm_matrix(rot).to_quaternion()

Comment it out to get this

alice

The reason it's there is to handle non-uniform scalings. I explain how the math works out in #945, but basically non-uniform scalings are not coordinate independent like translations or rotations; they "privilege" certain axes, so we can't change what those axes are, just rearrange them.

I wrote the code to be robust if you do use a non-aligned rotation though. If the rotation is not aligned to the local axes, as long as the scaling on that bone is always uniform, it will be fine. If there is a non-uniform scaling, the scaling will be transformed as if the rotation were aligned to the local axes. I have no idea what the error will actually look like in that case though.

So what to do...

  • attempt to detect when no non-uniform scaling is happening and allow non-aligned rotations?
  • add a UI option (tickbox? new heuristic?) or something to allow non-aligned rotations? (I've been calling this strategy "Fortune" instead of "Temperance" in my head :p)

@fire
Copy link

fire commented Mar 9, 2020

Non-uniform scale can be used for height adjustments and character skinny - fat adjustments. I'll try to find an example for testing.

Edited:

Not able to find one on quick notice.

@scurest
Copy link
Contributor

scurest commented Mar 9, 2020

We can test it pretty easy I guess. Grab one of her twintail bones and scale it along the Y axis.

axa1

Export, then re-import both with that line commented out and not. You get this

axa2

The one that looks wrong is with that line commented out. Non-uniform scalings always go along a local axis, so since we changed the direction of the local axes, we can't scale in the correct direction anymore.

@scurest
Copy link
Contributor

scurest commented Mar 9, 2020

@julienduroure julienduroure added this to the Blender 2.83 milestone Mar 10, 2020
@julienduroure
Copy link
Collaborator

@lyuma Can you please test the scurest branch ?

@lyuma
Copy link
Author

lyuma commented Mar 12, 2020

Hi @julienduroure and @scurest Sorry for taking so long to get back.

For me, Fortune is working as well as can be expected, and it seems to act correctly in the vast majority of cases.

I did find one common case where this approach does not produce the optimal bone rotation (It's still much better than with Temperance or Blender settings, mind you!) The case is when there is bone as a child of a body bone which is not really part of the body. For example accessories, skirt attachments or a tail. And in these cases, the bone scale is also set wrong, regardless of Bone Dir.

One such model is the CC-BY Possum model by 1029chris, which is available both in FBX and VRM. You can see the Fortune importer appears to prefer the Tail bone as the child of Hips, rather than the Spine.

Here is the armature of the Possum VRM file (lost its bone rotations) with glTF Fortune setting.
possum_hips_bone_vrm

For reference, here is an image of the "original" Possum FBX.
possum_hips_bone_fbx

For reference, here are some other screenshots I have showing this bone scaling and orientation error. You can see how the bones are not symmetric between left and right because the left side has an accessory attached.
Screen Shot 2020-03-12 at 8 50 44 AM
Screen Shot 2020-03-12 at 10 23 55 AM
Screen Shot 2020-03-12 at 9 59 07 AM

Anyway, I'm merely documenting my observations. In every case, the orientation chosen is consistent with the scale chosen by the glTF importer. It's also unclear if this is even a problem, or if any correct method can exist to solve this. If we do need to fix this, it would have to start with choosing the correct bone scale, so it is not a problem with the rotation code.

TLDR: As far as I can tell, Fortune is working as well as can be expected, and leads to quite pleasant looking armatures.

@scurest
Copy link
Contributor

scurest commented Mar 13, 2020

@lyuma Thanks for checking.

I know the Collada importer uses a heuristic that goes something like "put the bone tip at the head of the child that is the root of the deepest (highest) sub-tree of the bone tree", which looks like it would help in your examples, although this also has pathological cases.

Another useful idea is "rotate leaf bones so they point away from their parent". I tested this before picking the current method ("rotate leaves with the same rotation the parent used") for Temperance.

I'll try to throw something together to test these.

@scurest
Copy link
Contributor

scurest commented Mar 13, 2020

Branch with options for those if anyone wants to try it.

To me it looks like both of these cause basically as many problems as they fix though. So I think I'll probably PR just the "Fortune" option.

@julienduroure What do you think?

@fire
Copy link

fire commented Mar 18, 2020

I would like fortune to be merged, @julienduroure thoughts?

@julienduroure
Copy link
Collaborator

Let's go with fortune option.
@scurest , can you please create the PR?

@scurest
Copy link
Contributor

scurest commented Mar 25, 2020

Yeah, sorry. Opened #995.

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

Successfully merging a pull request may close this issue.

4 participants