Details

Reviewers

Sergey Sharybin (sergey)
Jacques Lucke (JacquesLucke)

Commits

rB8726354d465a: Fix Rigidbody depsgraph passive and constraint transform relations.

Summary

I noticed that I was mistaken about D8431

We need to have transforms from passive objects if they are animated or driven by parent relations.
This is not immediately obvious as the object transform matrix will still be available, it is just one frame behind in some cases.

I also removed dependency cycles if there is a constraint between two rigid bodies.
This is because bullet keeps track of its simulated bodies. So we do not need to supply objects transforms bullet should already have them.

I need combine these two fixes because otherwise we will get depsgraph warnings that nodes are missing that it expect to be there.

I checked that there were no regression so there is still no cycles after essentially reverting D8431:

rigidbodyconstaints.blend831 KBDownload

@Jacques Lucke (JacquesLucke) does this look good to you?

Diff Detail

Repository: rB Blender

Event Timeline

Sebastian Parborg (zeddb) requested review of this revision.Aug 27 2020, 4:08 PM

Sebastian Parborg (zeddb) created this revision.

Sebastian Parborg (zeddb) updated this revision to Diff 28237.Aug 27 2020, 4:10 PM

From quick reading seems fine, with some (pedantic?) suggestion: maybe have BKE_rigidbody_is_affected_by_simlation() instead of having in-lined check for RBO_TYPE_PASSIVE and RBO_FLAG_KINEMATIC? Will ease understanding of code for the depsgraph developers.

Sebastian Parborg (zeddb) edited the summary of this revision. (Show Details)Aug 27 2020, 4:53 PM

Updated with provided feedback

After thinking about it further, it seems to me that we actually do not need to add any relations between constraints at all.
For a valid constraint, both objects has to be rigid bodies, and if there is an empty, the transform is only used when the constraint is created, not during the animation.

It seems to work fine (no change in functionality in my test file).

RedMser (RedMser) added a subscriber: RedMser (RedMser).Aug 28 2020, 4:12 AM

From discussion here at the studio and reading the code seems fine.

@Jacques Lucke (JacquesLucke), I wouldn't mind if you give a second pair of eyes here.

This revision is now accepted and ready to land.Aug 28 2020, 9:30 AM

Jacques Lucke (JacquesLucke) added inline comments.Aug 28 2020, 12:48 PM

source/blender/blenkernel/BKE_rigidbody.h
146	typo, `simulation`
source/blender/depsgraph/intern/builder/deg_builder_relations.cc
1719	Can this relation be moved into the `if (rigidbody_object_depends_on_evaluated_geometry)` block below as well?