Page MenuHome

Fix bad sync between rigid bodies and modifiers
Needs ReviewPublic

Authored by Germano Cavalcante (mano-wii) on Thu, Feb 13, 1:49 PM.

Details

Summary

This fixes T68128, T73000, T63238, T58044 and maybe T71283

Rigid Bodies are calculated after the modifiers. So the obmat that the
modifiers use may be out of date.

Currently the obmat is reset according to the loc, rot and scale of
the object at the beginning of each frame, causing the modifiers to use
a "reseted" obmat.

The patch solution was to move the sync of the rigid body transform to
BKE_object_eval_local_transform and remove all reference to the old
BKE_rigidbody_object_sync_transforms eval function.

By moving to BKE_object_eval_local_transform we can know when or not
to "reset" obmat.

Additionally, the order to calculate the rigid body was changed to
before the modifiers, so they no longer have a 1 frame delay in the
transformation (but the rigid body now has a 1 frame delay in geometry
changing).

Diff Detail

Repository
rB Blender
Branch
arcpatch-D6834 (branched from master)
Build Status
Buildable 6627
Build 6627: arc lint + arc unit

Event Timeline

Nice find :)

Additionally, the order to calculate the rigid body was changed to before the modifiers, so they no longer have a 1 frame delay in the transformation (but the rigid body now has a 1 frame delay in geometry changing).

Are those mutually exclusive? Or would there be a way to solve both at the same time?
I feel that the reordered code should have a comment that explains this. If it's important to have the RBO calculation before the modifiers, this should be in a comment. If the order causes a known issue with a delay, which could be solved by reordering things, that should also be a comment.

source/blender/blenkernel/BKE_rigidbody.h
141–142

Document what the return value means. It probably means "didn't sync", but there is quite a difference between "wasn't necessary" or "wasn't possible".

source/blender/blenkernel/intern/object_update.c
85

A comment here that explains why this is happening is IMO vital.

I'm also not convinced all this code needs to be here. I would think this is hypothetical situation would be a bit nicer:

if (BKE_rigidbody_sync_transforms(depsgraph, ob)) {
  /* Do not calculate local matrix. */
  return;
}

The check whether ob->rigidbody_object is not NULL and getting the stuff from the depsgraph can then be done inside that function.

I have no opinion on whether this is done in BKE_rigidbody_sync_transforms() or in another (new) function.

source/blender/depsgraph/intern/builder/deg_builder_relations.cc
1705

Shouldn't the description here change SyncSim Eval?

Germano Cavalcante (mano-wii) marked 3 inline comments as done.

Additionally, the order to calculate the rigid body was changed to before the modifiers, so they no longer have a 1 frame delay in the transformation (but the rigid body now has a 1 frame delay in geometry changing).

Are those mutually exclusive? Or would there be a way to solve both at the same time?

They are mutually exclusive.
Some modifiers depend on the object's matrix and the object's matrix depends on the geometry created.
The solution would be if the rigid body simulation were treated as a modifier and could be organized within the modifiers stack.
But that would be a very complex change.

Updates:

  • Simplify BKE_rigidbody_sync_transforms and create a BKE_rigidbody_sync_transforms_ex.
  • Document the return of BKE_rigidbody_sync_transforms_ex.
  • Retype "Rigidbody Sync -> Transform Final" as "Sync -> Sim Eval".
source/blender/depsgraph/intern/builder/deg_builder_relations.cc
1705

I am not familiar with this area, so if you think so then you must be right.