Page MenuHome

Fix T70805: Pose Bones rotate from wrong pivot when translated by Action Constraints.
Needs RevisionPublic

Authored by Sebastian Parborg (zeddb) on Oct 18 2019, 2:21 PM.

Details

Summary

This makes the action constraint behave the same way as other constraints when the user tries to apply transform to an affected bone.

I noticed that this leaves object action constraints still broken as I didn't touch the object code path. I'll look into this. But in the mean time I would be good to see if this approach is good to you guys.

Diff Detail

Event Timeline

Alexander Gavrilov (angavrilov) requested changes to this revision.Oct 18 2019, 2:54 PM

Completely wrong approach - changing constraints to fix a transform tool problem. The transform tools simply can't cope with any transformations applied by constraints via right side full matrix multiplication. This also is not backward compatible, which is unacceptable by itself.

This revision now requires changes to proceed.Oct 18 2019, 2:54 PM

This is how other constraints seem to work around the issue though. So I'm unsure how this would be solved.

I don't think breaking backwards compatibility is something we can't do in cases like these were the constraint didn't match the behavior of the other constraints.
We got this reported by our in house artists and I also showed this behavior to other developers at the studio. All of them agreed that this was a bug.

I don't think we should preserve buggy behavior just for the sake of it.

Constraints don't and shouldn't be 'working around' UI issues. This also changes how existing rigs would be evaluated by changing the transformation mixing mode instead of providing an option, which is not acceptable.

The proper way to do this is to implement a Mix Mode option, like I've already done in Copy Rotation, Copy Transforms and Transformation. The main issue there is actually naming the modes, as the current legacy mode of Action is slightly different from all other existing modes in other constraints, and it's best to keep naming consistent among all constraints.

Specifically, for full matrix transformation there are 3 conceivable modes, which need distinct and understandable names:

  1. Multiply matrices: Result = A * B.
  2. Separately handle just the scale: R = normalize(A) * B * A.scale; R.location = A * B.location
  3. Complete matrix separation: R_loc = A_loc + B_loc; R_rot = A_rot * B_rot; R_scale = A_scale * B_scale

Copy Transforms currently does 2, while Action does 1, and you implemented 3.

Perhaps we can just have two modes?

  1. Action Last (old matrix multiplication method, apply user transform first then action)
  2. Action First (new method implemented in this patch, apply action first then user transform)

I also found out why transformations of objects with constraints behaves weird. In source/blender/editors/transform/transform_convert_object.c:395, the evaluation of constraints is disabled so the pivot point will be in the wrong place. Just removing this seems to work for me.
But I am a bit afraid that this will break something else so I went back in the commit history and found that this has been around since the transform code was rewritten for 2.5:

https://developer.blender.org/rB63e80e271dd3f9f79fe3a85c10253ee6eea2ba4e (introduction of the disable constraints enum)
https://developer.blender.org/rBb6b61681efd3 (2.5 port)

So I guess the reason for this being there might be gone?
I guess this would be for an other pull request?

Not that this is not only breaking transform with the action constraint for object but also copy location and others too.

Possible names for the modes:

  1. After Original: Combined
  2. After Original: Split Scale
  3. After Original: Split All

Note that Before/After has to be in the modes only because constraints aren't nodes and you can't simply swap the order of the input connections to the mix operation.