Page MenuHome

Fix T99386: Driven modifiers are always re-evaluated during animation
ClosedPublic

Authored by Sergey Sharybin (sergey) on Jul 4 2022, 1:10 PM.

Details

Summary

Even if the driver is not dependent on time the modifiers were always
re-evaluated during playback. This is due to the legacy nature of the
check whether modifier depends on time or not: it was simply checking
for sub-string match for modifier in the F-Curve and drivers RNA paths.

Nowadays such dependencies are created by the dependency graph builder,
which allows to have more granular control over what depends on what.

The code is now simplified to only check for "static" dependency of the
modifier form time: for example, Wave modifier which always depends on
time (even without explicit animation involved).

This change also fixes missing relation from the animation component to
the shader_fx modifiers, fixing race condition.

Additional files used to verify relations:

In these files different types of modifiers have an animated property,
and the purpose of the test is to verify that the modifiers do react
to the animation and that there is a relation between animation and
geometry components of the object. The latter one can only be checked
using the dependency graph relation visualization.

The drivers are not tested by these files. Those are not typically
depend on time, and if there were missing relation from driver to
the modifier we'd receive a bug report already. As well as if there
was a bug in missing time relation to a driver we'd also receive a
report.

Diff Detail

Repository
rB Blender

Event Timeline

Sergey Sharybin (sergey) requested review of this revision.Jul 4 2022, 1:10 PM
Sergey Sharybin (sergey) created this revision.
Sergey Sharybin (sergey) edited the summary of this revision. (Show Details)Jul 4 2022, 1:11 PM
Sergey Sharybin (sergey) edited the summary of this revision. (Show Details)Jul 7 2022, 1:00 PM

Conceptually I completely agree with the patch. Using the actual depsgraph dependency information is indeed better than the Old Ways.

Geometry File

With the geometry file (F13257368), I have these Time Source dependencies:

Pre-patch:

  • TIMESOURCEOBCube, geometry component.
  • TIMESOURCEACCubeAction, animation component: ANIMATION_EVAL()OBCube, animation component → OBCube, geometry component.

Post-patch:

  • TIMESOURCEACCubeAction, animation component: ANIMATION_EVAL()OBCube, animation component → OBCube, geometry component.

That LGTM, as the removed relation was already implied by the remaining chain of relations. I don't see how this would solve any race condition, though, but maybe that's clear from the other files.

Grease Pencil file

Pre-patch:

  • TIMESOURCEACSuzanneAction.001, animation component: ANIMATION_EVAL()OBSuzanne, animation component. Dead-ends there inside the animation component.
  • TIMESOURCEGDSuzanne, geometry component: GEOMETRY_EVAL()GEOMETRY_EVAL_DONE()OBSuzanne, geometry component: GEOMETRY_EVAL_INIT()GEOMETRY_EVAL().
  • TIMESOURCEOBSuzanne, geometry component: GEOMETRY_EVAL().

Post-patch:

  • TIMESOURCEACSuzanneAction.001, animation component: ANIMATION_EVAL()OBSuzanne, animation component.
  • TIMESOURCEGDSuzanne, geometry component: GEOMETRY_EVAL()GEOMETRY_EVAL_DONE()OBSuzanne, geometry component: GEOMETRY_EVAL_INIT()GEOMETRY_EVAL().

This also LGTM.

ShaderFX file

Pre-patch:

  • TIMESOURCEACSuzanneAction.001, animation component: ANIMATION_EVAL()OBSuzanne, animation component.
  • TIMESOURCEGDSuzanne, geometry component: GEOMETRY_EVAL()OBSuzanne, geometry component: GEOMETRY_EVAL_INIT()GEOMETRY_EVAL().
  • TIMESOURCEOBSuzanne, geometry component: GEOMETRY_EVAL()

Post-patch:

  • TIMESOURCEACSuzanneAction.001, animation component: ANIMATION_EVAL()OBSuzanne, animation component.
  • TIMESOURCEGDSuzanne, geometry component: GEOMETRY_EVAL_DONE()OBSuzanne, geometry component: GEOMETRY_EVAL_INIT()GEOMETRY_EVAL().

Conceptually this seems a little strange to me. How can GEOMETRY_EVAL_DONE() depend on the time source, when it indicates that the "evaluation of geometry is completely done" and the evaluation itself (i.e. GEOMETRY_EVAL) does *not* depend on the time source?

UPDATE: yes, it was strange, because I didn't look right. The arrow goes to GEOMETRY_EVAL(), not GEOMETRY_EVAL_DONE().

This revision is now accepted and ready to land.Jul 7 2022, 3:19 PM