Page MenuHome

Child-Of now sets inverse matrix automatically upon creation
ClosedPublic

Authored by Joseph Brandenburg (TheAngerSpecialist) on Sep 9 2020, 5:50 PM.

Details

Summary

This matches the behavior of other applications and improves UX,
especially for new users.

The former behaviour was an especially annoying idiosyncracy for
Blender. Maya in particular makes heavy use of the Parent
constraint in its workflow, and sets the inverse automatically.
This commit will improve the workflow for Maya artists who are
transitioning to Blender.

I don't know if my approach here is "correct". Some constraints
are initialized with correct settings already, such as the
Stretch-To constraint (original length) or the Limit Location
constraint (distance). I didn't check how those constraints
get or set this information... instead, I added a few lines to
constraint_add_exec that sets the CHILDOF_SET_INVERSE flag
if the new constraint is a Child-Of. This seems safe, since
the user is probably adding the constraint in pose-mode
already. However, I don't know if this is the correct approach.

Diff Detail

Repository
rB Blender
Branch
add-constraint-child-of-with-inverse (branched from master)
Build Status
Buildable 10083
Build 10083: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) requested changes to this revision.Sep 10 2020, 10:17 AM

Thanks for the patch. Could you maybe update it with a description of the current and proposed flow when adding a ChildOf constraint, so that it's clear from a user's perspective what's changing?

source/blender/editors/object/object_constraint.c
2081–2082

Why the NULL checks? I don't see any reason why con would be NULL, or why a ChildOf constraint would be created without any data.

This revision now requires changes to proceed.Sep 10 2020, 10:17 AM
  • Remove unnecesary null check and unnecesary variable.
Joseph Brandenburg (TheAngerSpecialist) marked an inline comment as done.EditedSep 10 2020, 3:12 PM

The NULL check was there simply because I copied the code from another function that had it. I wanted to be careful.

Here's a video demonstrating the behavior of the Child-Of constraint's creation in 2.90 and in my patch.

When creating a Child-Of constraint before, the bone you constrain jumps away from its position, because it is using an identity matrix until you set it in the properties panel. furthermore, because the bones' axes may be pointing in any direction, and rarely point in the direction of the world axes, this is a completely unpredictable behavior. This is especially confusing to new users, who may be following a tutorial and HEY! *Where did my bone go?*.
The new behavior is better for users - the child-of constraint is now set to recalculate the inverse matrix upon creation. So instead of jumping away, it stays in place! This also saves a few clicks for the sake of speed. And it doesn't disable or remove any of the advantages of being able to set the inverse at any time.

Thanks for having a look :)

  • Remove unnecesary null check and unnecesary variable.
  • Revert "Remove unnecesary null check and unnecesary variable."
  • Remove unnecesary null check

The variable *data seems unncecesary to me, but when I tried to do (bChildOfConstraint *)con->data->flag |= CHILDOF_SET_INVERSE; I would get a compilation error. This works now, but I think it can be better.

Thanks for the more elaborate description & video, it really shows the difference this makes!

The variable *data seems unncecesary to me, but when I tried to do (bChildOfConstraint *)con->data->flag |= CHILDOF_SET_INVERSE; I would get a compilation error. This works now, but I think it can be better.

Having an extra variable is fine. The reason that the compilation fails is that flag shouldn't be converted to bChildOfConstraint *. ((bChildOfConstraint *)con->data)->flag |= CHILDOF_SET_INVERSE; would work, but I think your current approach (with an extra variable) is easier to read & understand.

For future patches, either set up your IDE to use Clang-Format, or run make format (the { should be on the same line as case CONSTRAINT_TYPE_CHILDOF:).

One more question: have you tried setting the CHILDOF_SET_INVERSE flag in childof_new_data() instead? That might be a nicer change, as then all the default flags are available in one place.

  • Move Child-Of Set Inverse flag to constraint.c

Thank you Dr. I didn't know about childof_new_data(). That's exactly what I was looking for when I wrote above:

I don't know if my approach here is "correct". Some constraints
are initialized with correct settings already, such as the
Stretch-To constraint (original length)...

So this seems like the best way, Very simple change for a big improvement to the workflow.

I did a make format before committing this time. That's another new thing I've learned!

Thanks again. I'm looking forward to learning more and finding ways to contribute.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 11 2020, 3:19 PM
This revision was automatically updated to reflect the committed changes.

Congrats, your fix will be part of Blender 2.91 :)

This sets the inverse matrix after selecting the target but not after selecting a vgroup in the target, which makes it do the strange offset that the patch was trying to avoid

The problem also happens when you change the OB target from one to another, in that case the automatic set inverse will also not work

Add a Child Of to the monkey
Add the right plane as target: Works
Select the other plane as target: Fails
Select the vgroup: Fails

The problem is the "Fix" being done on initial evaluation. In order to work correctly it would need to update on target changes.