Page MenuHome

Stretch To: implement a mode similar to Damped Track for rotation.
ClosedPublic

Authored by Alexander Gavrilov (angavrilov) on Oct 26 2019, 4:22 PM.

Details

Summary

Most of the time Stretch To is used in actual rigs in combination
with Damped Track to handle rotation before the stretch, because
it produces rotations more appropriate for organic deformation.

The prevalence of this pattern suggests that Stretch To should
support that kind of rotation directly as an option.

Diff Detail

Repository
rB Blender
Branch
stretch-to-damped (branched from master)
Build Status
Buildable 5580
Build 5580: arc lint + arc unit

Event Timeline

Most of the time Stretch To is used in actual rigs in combination with Damped Track to handle rotation before the stretch because it produces rotations more appropriate for organic deformation.

Do you have any references to this? Where did you find this information?

Do you have an example blend file to test the changes?

source/blender/blenkernel/intern/constraint.c
3245

What "others"?

3348

Why are these arguments swapped all of a sudden? Was something wrong in the original code? In that case it should be fixed in a separate patch. Same goes for the other swapped arguments below.

Do you have any references to this? Where did you find this information?

Any rigger knows that, and rigs like BlenRig and Rigify are full of this pattern. Also, I really suggest you join blender-riggers.

I'd maybe even go as far as making the new option the default for newly created constraints.

source/blender/blenkernel/intern/constraint.c
3245

Other members of the enum obviously.

3348

Because the sign of vec changed to the proper direction that does not require negate_v3_v3. Negating the cross product means swapping args.

source/blender/blenkernel/intern/constraint.c
3245

Well, "obviously" not, because I had to ask for it.

3348

There still seems to be a mixup of two things: the addition of the Dampened Track mode, and the optimisation by which the negate_v3_v3(totmat[1], vec); call can be avoided. Probably it's better to do those things in different commits, so that each change is clear to whoever reads the commits.

Do you have any references to this? Where did you find this information?

Any rigger knows that

This patch isn't reviewed by riggers, but by coders. I don't know all the things riggers know.

and rigs like BlenRig and Rigify are full of this pattern.

This could have been mentioned in the patch description up front, it would give a better context in which to evaluate the changes.

I'd maybe even go as far as making the new option the default for newly created constraints.

I think it's better to do this as a separate step. First get people acquainted with the new functionality, maybe polish something, or see that it doesn't need polishing at all. Then consider changing the default, if it turns out that people constantly have to switch to the new mode.

Improved comment, split into two commits in my branch.

Stretch To: clean up the math code in the implementation.
    
Combine computing `size` and normalizing the matrix, invert the
direction of `vec` to avoid negating it later, use `rescale_m4`
instead of matrix multiplication to scale the final result.
    
Differential Revision: https://developer.blender.org/D6134
Stretch To: implement a mode similar to Damped Track for rotation.

Most of the time Stretch To is used in actual rigs, like BlenRig
or Rigify, in combination with Damped Track to handle rotation
before the stretch, because it produces rotations more appropriate
for organic deformation, and doesn't flip because of internal
gimbal lock.

The prevalence of this pattern suggests that Stretch To should
support that kind of rotation directly as an option.

Differential Revision: https://developer.blender.org/D6134
source/blender/blenkernel/intern/constraint.c
3245

"older modes" implies some knowledge of what is an "old mode" and what is a "new mode". Since this isn't directly clear from the code, I would avoid it. I think "other modes" is clear enough.

LGTM with the change to the comment I proposed.

This revision is now accepted and ready to land.Thu, Nov 7, 2:36 PM