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

Implement BoneConstraint3D with CopyTransform/ConvertTransform/Aim Modifiers #100984

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Dec 31, 2024

BoneConstraint3D

A base class that allows two bones to be specified.

image

Note that interpolation by SkeletonModifier3D.influence for the entire skeleton is done for each SkeletonModifier3D.

However, constraint settings are often applied to multiple bones at the same time. Therefore, it prevents unnecessary interpolation by having the settings in an array so that the number of SkeletonModifier3Ds in the scene does not grow unnecessarily. For this reason, the BoneConstraint3D settings have the apply amount of modification to the bone for each element in the settings array.

AimModifier3D

This is a simple version of LookAtModifier3D that only allows bone to the target without advanced options such as angle limitation or time-based interpolation. The feature is simplified, but instead it is implemented with smooth tracking without euler,

image

CopyTransfromModifier3D

Apply the copied transform of the target bone to the apply bone with processing it with some masks and options.

image

There are 4 ways to apply the transform, depending on the combination of Relative and Additive options.

  • Relative + Additive
    • Extract target pose relative to the rest and add it to the apply bone's pose.
  • Relative + Not Additive
    • Extract target pose relative to the rest and add it to the apply bone's rest.
  • Not Relative + Additive
    • Extract target pose absolutely and add it to the apply bone's pose.
  • Not Relative + Not Additive
    • Extract target pose absolutely and the apply bone's pose is replaced with it.

ConvertTransfromModifier3D

Apply the copied transform of the target bone to the apply bone about the specific axis with remapping it with some options.

image


Compatibility with third-parties

I assume there is some compatibility, but cannot guarantee that the behavior is exactly the same in a particular situation (since Since it has not yet been tested/verified).

Blender

CopyXXX (for rotation with axis mask, rotation mode is roll only) -> CopyTransfromModifier3D
Transformation (for rotation, rotation mode is roll only) -> ConvertTransfromModifier3D
Track/LockedTrack -> AimModifier3D with using euler
DampedTrack ->AimModifier3D without using euler

VRMConstraint

https://github.com/vrm-c/vrm-specification/blob/master/specification/VRMC_node_constraint-1.0

RotationConstraint -> CopyTransfromModifier3D with only rotation with all axes without invert flags
RollConstraint -> ConvertTransfromModifier3D with rotation mode
AimConstraint -> AimModifier3D without using euler

@TokageItLab TokageItLab added this to the 4.x milestone Dec 31, 2024
@TokageItLab TokageItLab requested a review from a team December 31, 2024 18:27
@TokageItLab TokageItLab force-pushed the bone-constraint branch 2 times, most recently from b5d65a9 to 5490058 Compare December 31, 2024 18:32
@fire fire self-requested a review December 31, 2024 20:01
@fire
Copy link
Member

fire commented Jan 4, 2025

Do we have test scenes?

I'll look into those.

@TokageItLab
Copy link
Member Author

@fire I used this model in quick check to show axes where everything is a sibling of root:

image

constraint_demo.zip

@TokageItLab
Copy link
Member Author

TokageItLab commented Jan 5, 2025

@fire For now, since the feature freeze is nearing, I separated the PR only from the refactoring part, which will cause break compatibility; Since LookAtModifier is added in 4.4, if this PR is carried over to 4.5 it will occur.

@TokageItLab TokageItLab force-pushed the bone-constraint branch 3 times, most recently from d3c358e to 3aaf4e9 Compare January 8, 2025 20:29
@TokageItLab TokageItLab modified the milestones: 4.x, 4.5 Jan 13, 2025
@TokageItLab
Copy link
Member Author

I will write a CI test later, because this module does not depend on external factors such as time, but purely mathematical transformations, so it is possible to check for them.

@fire fire changed the title Implement BoneConstraint3D with CopyTransfrom/ConvertTransfrom/TrackBone Modifiers Implement BoneConstraint3D with CopyTransfrom/ConvertTransform/TrackBone Modifiers Jan 18, 2025
@fire fire changed the title Implement BoneConstraint3D with CopyTransfrom/ConvertTransform/TrackBone Modifiers Implement BoneConstraint3D with CopyTransform/ConvertTransform/TrackBone Modifiers Jan 18, 2025
@TokageItLab TokageItLab force-pushed the bone-constraint branch 3 times, most recently from 5724dec to e6c20fe Compare March 10, 2025 00:01
@aaronfranke
Copy link
Member

aaronfranke commented Mar 10, 2025

For reference, see also this GDScript implementation of VRM constraints I made for godot-vrm: https://github.com/V-Sekai/godot-vrm/blob/master/addons/vrm/node_constraint/bone_node_constraint.gd

Also, I made an example model here: vrm-c/vrm-specification#444 Before merging this PR, we should write a draft PR to godot-vrm that uses this PR and test that it works correctly.

This PR looks much more comprehensive than what I wrote and seems to handle everything in VRM and more, awesome! Feel free to request a review from me once this PR is ready to go.

@aaronfranke aaronfranke self-requested a review March 10, 2025 00:49
@TokageItLab
Copy link
Member Author

TokageItLab commented Mar 14, 2025

I removed the use euler option from Copy/ConvertTransformModifier.

Since the bone pose only stores the quaternion, there is no guarantee that the euler will be stored in the next frame even if the SkeletonModifier sets the euler. Then I concluded that it would be difficult to accomplish converting euler rotation without a more fundamental implementation in the Skeleton3D class.

BTW, this is not a problem in cases where processing is performed on all three axes. Where this becomes a problem is when using ConvertTransformModifier to handle Rotation, or CopyTransformModifier to mask or invert on one or two axes. But it should still cover VRM compatibility. It lacks the compatibility of the constrains that Blender has that use Euler.

For now, to ensure mathematical correctness, they behave as if they were copying/cancelling a roll in a particular axis or inverting an element in a Quaternion. Perhaps similar to the roll+swing rotation mode in Blender.

It may seem somewhat less intuitive, but I have explained conceptually in the documentation how they work. Also, their mathematical validation is ensured by the added unit tests.

@TokageItLab TokageItLab marked this pull request as ready for review March 14, 2025 15:51
@TokageItLab TokageItLab requested review from a team as code owners March 14, 2025 15:51
@TokageItLab TokageItLab force-pushed the bone-constraint branch 2 times, most recently from 9b9da6c to 5673975 Compare March 14, 2025 16:09
@AThousandShips

This comment was marked as resolved.

@fire
Copy link
Member

fire commented Mar 14, 2025

Should we store in swing twist decomposed quaternion/basis?

I mentioned wanting to support cone, twist and polygon (kusudama)

@TokageItLab

This comment was marked as resolved.

@TokageItLab
Copy link
Member Author

TokageItLab commented Mar 14, 2025

Should we store in swing twist decomposed qusternions/basis?

@fire I don't think Skeleton3D needs to retain that. However, it could be discussed whether or not these functions like get_roll_angle() should be made public.

@AThousandShips

This comment was marked as resolved.

@fire

This comment was marked as resolved.

@SaracenOne SaracenOne self-assigned this Mar 14, 2025
@TokageItLab TokageItLab force-pushed the bone-constraint branch 2 times, most recently from 64b60f1 to dc9ffe2 Compare March 15, 2025 06:35
@TokageItLab
Copy link
Member Author

TokageItLab commented Mar 15, 2025

For consistency (Copy/ConvertModifier does not contain the word "Bone") and to account for the possibility of setting non-bone targets in the future, I renamed TrackBoneModifier3D to AimModifier3D as a more generic name. TrackModifier3D is another idea, but the word "track" has many homonyms, so the word "aim" was chosen to match with VRMConstraint.

There is a possibility of a little confusion with LookAtModifier3D, but AimModifier3D has BoneConstraint3D as its base class, so I assume that this is not a problem.

@TokageItLab TokageItLab changed the title Implement BoneConstraint3D with CopyTransform/ConvertTransform/TrackBone Modifiers Implement BoneConstraint3D with CopyTransform/ConvertTransform/Aim Modifiers Mar 15, 2025
@TokageItLab TokageItLab marked this pull request as draft March 16, 2025 00:57
@TokageItLab TokageItLab marked this pull request as ready for review March 16, 2025 11:55
@TokageItLab
Copy link
Member Author

Fixed several bugs discovered while applying Blender's constrained settings for actual use:

  • Fixed a problem with inverse_lerp causing division by zero when target's range in ConvertTransformModifier is 0
  • Fixed a problem with incorrect rest space in AimModifier (this PR includes Fix rest translation space in LookAtModifier3D #104217)
  • Fixed an issue with incorrect conversion of AimModifier's without-euler mode

Twist without constraint:
image

Twist with constraint:
image

@TokageItLab
Copy link
Member Author

Improved handling of zero division in ConvertTransformModifier. Also, "target bone" has been renamed to "reference bone".

While "target" in Aim is clear, the word "target" in Convert and Copy is a bit confusing (although Blender consistently calls it "target"). This rename would make Aim "target" to "reference", but I think it's still understandable.

If anyone has any other better names, suggestions are welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for review
Development

Successfully merging this pull request may close these issues.

5 participants