Page MenuHome

T76121 Driver depending on animated modifier property does not update
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on May 4 2020, 1:52 PM.

Details

Summary

This patch contains two possible fixes for T76121. The root cause is this code from deg_builder_rna.c, function RNANodeQuery::construct_node_identifier():

else if (RNA_struct_is_a(ptr->type, &RNA_Mesh) || RNA_struct_is_a(ptr->type, &RNA_Modifier) ||
         RNA_struct_is_a(ptr->type, &RNA_GpencilModifier) ||
         RNA_struct_is_a(ptr->type, &RNA_Spline) || RNA_struct_is_a(ptr->type, &RNA_TextBox) ||
         RNA_struct_is_a(ptr->type, &RNA_GPencilLayer) ||
         RNA_struct_is_a(ptr->type, &RNA_LatticePoint) ||
         RNA_struct_is_a(ptr->type, &RNA_MeshUVLoop) ||
         RNA_struct_is_a(ptr->type, &RNA_MeshLoopColor) ||
         RNA_struct_is_a(ptr->type, &RNA_VertexGroupElement)) {
  /* When modifier is used as FROM operation this is likely referencing to
   * the property (for example, modifier's influence).
   * But when it's used as TO operation, this is geometry component. */
  switch (source) {
    case RNAPointerSource::ENTRY:
      node_identifier.type = NodeType::GEOMETRY;
      break;
    case RNAPointerSource::EXIT:
      node_identifier.type = NodeType::PARAMETERS;
      node_identifier.operation_code = OperationCode::PARAMETERS_EVAL;
      break;
  }
  return node_identifier;
}

The example file in T76121 has an animated modifier property that's used as variable in a driver.

Building the relations for the driver target creates the relation PARAMETERS_EVALDRIVER(variable) (blue in the image below).

Building the relations for the FCurve targeting the modifier property creates the relation ANIMATION_EXITGEOMETRY_EVAL_INIT (green in the image below).

This means that there is NOT a relation ANIMATION_EXITPARAMETERS_EVAL, and as a result, the driver is not properly updated when its variable reads animated data. This can be resolved by adding the missing relation (red in the image below).

This missing relation can be added in two different spots, namely when building the driver or the animation relations. Both feel a bit like a hack, though, as either is then tightly coupled with the behaviour of RNANodeQuery::construct_node_identifier(). To me the root cause seems to be that that particular function can only deal with a single relation, while in this case multiple relations would be required. Resolving this, however, would be quite an undertaking.

@Sergey Sharybin (sergey) Do you find this solution acceptable? And if so, which of the two locations for adding the missing relation would have your preference?

Diff Detail

Repository
rB Blender

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.May 4 2020, 1:52 PM

Why not to add relation from aimation to parameters component in a generic manner:

  • Do it in build_animdata().
  • Do it regardless of the modifiers check.

Why not to add relation from aimation to parameters component in a generic manner:

Because I was being conservative. This approach is better indeed. I also removed the comment, as it's no longer a driver-and-modifier situation but something generic.

  • Only add ANIMATION → PARAMETERS relation if there is an animation component

See the notes, things are a bit more annoying :\

Also, worth checking that object with drivers and animation behaves unexpectedly (at least, that there is no obvious temptation to create cyclic dependencies :)

source/blender/depsgraph/intern/builder/deg_builder_relations.cc
1238–1240

Think you'll have a lot of warning messages in the terminal for objects which does not have animation, or objects which have adt but no action.

After deeper look seems that we should either:

  • Put this deeper into build_animdata_curves()
  • Surround with check_id_has_anim_component()
  • Or, since surrounding with check_id_has_anim_component() is an option, do it in build_parameters().

Eeeek. You've figured the feedback before I've typed it!

Well, still give it a quick test for drivers+animation on the same datablock. Otherwise LGTM.

This revision is now accepted and ready to land.May 4 2020, 2:37 PM