Page MenuHome

Fix T70269: replace the Set Inverse operator with an eval-time update.
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on Oct 18 2019, 1:45 PM.

Details

Summary

It is rather complicated and hacky to try computing the correct
parent inverse matrix for Child Of outside of constraint evaluation.
To avoid problems, change the Set Inverse operator to simply set
a flag for the matrix to be recomputed during evaluation, which is
how it already works for some other constraints like Stretch To.

The downside of this approach obviously is that if the constraint
is disabled, Set Inverse will actually happen when it is re-enabled,
rather than immediately.

In addition, this changes the way how the inverse matrix works when
some of the channels of the constraint are disabled. Specifically,
before this the channel flags were used to filter both the parent
and the inverse matrix, which makes no sense, and means that it
is impossible to make an inverse matrix that would actually fully
neutralize the effect of the constraint. Now only the parent matrix
is filtered, while inverse is applied fully. This change is not
backward compatible, but it should be OK because the old way was
effectively unusable, so it's likely nobody relies on it.

Diff Detail

Repository
rB Blender

Event Timeline

Wouldn't it be better to stop working on adding more workarounds for the fact Blender doesn't have a node-based constraint system, and just spend time on creating a node-based constraint system?

Removed new features, reducing the patch to a Set Inverse refactor for T70269.

Alexander Gavrilov (angavrilov) retitled this revision from Child Of: make it easier to switch between multiple ad-hoc parents. to Fix T70269: replace the Set Inverse operator with an eval-time update..Dec 25 2019, 3:54 PM
Alexander Gavrilov (angavrilov) edited the summary of this revision. (Show Details)

Nice work, the new code is much clearer.

The downside of this approach obviously is that if the constraint is disabled, Set Inverse will actually happen when it is re-enabled, rather than immediately.

Do we want to allow this? I don't really like buttons that appear to work but don't. I see two alternatives:

  1. Disable the button (or allow clicking but show an error that it's not possible to use when the constraint is disabled).
  2. Enable the constraint, re-evaluate, disable, re-evaluate.

Personally I feel more for option 2; as Blender understands what the user wants, it should just do it. What do you think?

This patch also changes the Object Solver constraint code (for obvious reasons); probably a good idea to mention this in the patch description as well.

source/blender/blenkernel/intern/constraint.c
926–927

The space that should be between "matrixto" sneakily made its way to become a double space between "i.e. owner".

source/blender/makesrna/intern/rna_constraint.c
979

I think it would be a nice touch to mention that it's automatically set to false when the inverse matrix has been recomputed. I often see this in datasheets of microcontrollers, and it really helps me understand what's going on.

Im on the same boat as you sybren, if its something that must happen always, lets have it happen intelligently.

@Alexander Gavrilov (angavrilov) What's the status of this patch? Do you think we can target 2.82 for this?

Sybren A. Stüvel (sybren) commandeered this revision.EditedFeb 25 2020, 10:49 AM

As my question as to the status of this patch has been unanswered for over a month, I'm commandeering it to get it into master.

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Big update of the patch:

  • Updated the code for cleanups I did on master.
  • Added unit tests.
  • Added force evaluation when clicking 'Set Inverse' button on a disabled constraint.
This revision was not accepted when it landed; it landed in state Needs Review.Feb 27 2020, 10:38 AM
This revision was automatically updated to reflect the committed changes.