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

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

Details

Reviewers

Sybren A. Stüvel (sybren)

Group Reviewers

Animation & Rigging

Commits

rB55f819ccc950: Stretch To: implement a mode similar to Damped Track for rotation.
rB21d6dd932c58: Stretch To: clean up the math code in the implementation.
rBaadc90d0fc67: Stretch To: implement a mode similar to Damped Track for rotation.
rB4e5768b2a786: Stretch To: clean up the math code in the implementation.

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

Alexander Gavrilov (angavrilov) created this revision.Oct 26 2019, 4:22 PM

Harbormaster completed remote builds in B5458: Diff 19201.Oct 26 2019, 4:22 PM

Alexander Gavrilov (angavrilov) added a reviewer: Animation & Rigging.Oct 26 2019, 4:23 PM

Alexander Gavrilov (angavrilov) added subscribers: Juan Pablo Bouza (jpbouza), Demeter Dzadik (Mets).

Daniel Calderón (xdanic) awarded a token.Oct 30 2019, 1:05 PM

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.

In D6134#143151, @Sybren A. Stüvel (sybren) wrote:

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.

Sybren A. Stüvel (sybren) added inline comments.Nov 6 2019, 2:43 PM

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.

In D6134#143156, @Alexander Gavrilov (angavrilov) wrote:

In D6134#143151, @Sybren A. Stüvel (sybren) wrote:

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

Harbormaster completed remote builds in B5580: Diff 19436.Nov 6 2019, 3:19 PM

Sybren A. Stüvel (sybren) added inline comments.Nov 6 2019, 5:23 PM

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.