V-Sekai / godot-vrm

Importer/Exporter for VRM avatars and MToon shader. Available for Godot 4.1+ and 3.2+ in the Asset Library.
https://godotengine.org/asset-library/asset/2031
Other
287 stars 24 forks source link

VRMC_node_constraint malfunctions on some rigs #71

Open lyuma opened 1 year ago

lyuma commented 1 year ago

It seems to rotate the bone the wrong way relative to the origin. Likely one of the bone diff matrices is not being applied correctly.

image

Example VRM file showing the problem: VRM1_Constraint_Twist_Sample.vrm You must select LeftUpperArm in the inspector on GeneralSkeleton, and rotate it using the gizmo to observe the constraint rotation: the constraint does not update unless the bone is posed.

It seems to be a coordinate axis issue related to retarget: for example, if I rotate the arm to point straight up, then the sleeve sticks to the side as it would have been expected in the t-pose.

lyuma commented 1 year ago

@aaronfranke I'm worried this issue is caused by the roll axis being an enum rather than a Vector3, as in:

enum AimRollAxis {
    NONE = 0,
    POSITIVE_X = 1,
    POSITIVE_Y = 2,
    POSITIVE_Z = 3,
    NEGATIVE_X = 4,
    NEGATIVE_Y = 5,
    NEGATIVE_Z = 6,
}

Since we're rotating bones as part of retargeting, is it possible that the constraint can no longer be representable after the retargeting process...

Normally, I pass the quaternion or basis that represents the diff which happened during retarget and multiply it by vectors/axes or matrices. But in this case, I can't multiply this enum by that matrix, so I'm not sure what to do...

For example, let's say the arm's y axis is pointing 45 degrees down, as in an A-Pose, but the model is in a T-pose, so we re-create the bone axes using the SkeletonProfile. Now the constraint specifies a roll axis as +X or +Z.... how we do rotate that to the correct axis? Is this solvable by adding a child bone?

fire commented 1 year ago

A vector3 can be used if not we'll have to decompose to three axes as nodes.

aaronfranke commented 1 year ago

The enum is designed to represent the data in the VRM spec, which can be "X", "Y", or "Z" for the roll constraint. While we could convert this to Vector3, that would not be something we can re-export because it does not match VRM. Also, this logic is hard-coded for roll around just one of these axes, matching what VRM allows.

But anyway, I doubt changing the enum is the solution. I have not looked into this deeply but I would recommend trying to have the retargeter adjust the source_rest_transform and target_rest_transform values instead. These values are set during import and are what the constraint is relative to, so they are likely no longer correct after the retargeter does its thing. See the _get_posed_source_transform and _set_weighted_posed_target_rotation methods.

This is where I was setting it on my PR https://github.com/aaronfranke/godot-vrm/blob/node_constraint/addons/vrm/node_constraint/vrm_node_constraint_doc_ext.gd#L39 I don't know where you moved this code to. EDIT: Oh, it's in addons/vrm/1.0 now, I don't know why it was separated from the rest of the constraint code.

fire commented 1 year ago

@lyuma What's the approach to fixing this?