[Bf-blender-cvs] [eb7218de8db] master: Fix T99386: Driven modifiers are always re-evaluated during animation

Sergey Sharybin noreply at git.blender.org
Thu Jul 7 15:22:30 CEST 2022


Commit: eb7218de8dba2a977fdcf8f2e75b16fcd8fc044a
Author: Sergey Sharybin
Date:   Mon Jul 4 13:02:24 2022 +0200
Branches: master
https://developer.blender.org/rBeb7218de8dba2a977fdcf8f2e75b16fcd8fc044a

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

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:
- Geometry: F13257368
- Grease Pencil: F13257369
- Shader FX: F13257370

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.

Differential Revision: https://developer.blender.org/D15358

===================================================================

M	source/blender/blenkernel/BKE_object.h
M	source/blender/blenkernel/intern/object.cc
M	source/blender/depsgraph/intern/builder/deg_builder_relations.cc
M	source/blender/depsgraph/intern/builder/deg_builder_rna.cc

===================================================================

diff --git a/source/blender/blenkernel/BKE_object.h b/source/blender/blenkernel/BKE_object.h
index 6ed05bc267a..96abea0e5ee 100644
--- a/source/blender/blenkernel/BKE_object.h
+++ b/source/blender/blenkernel/BKE_object.h
@@ -69,9 +69,6 @@ void BKE_object_free_caches(struct Object *object);
 void BKE_object_modifier_hook_reset(struct Object *ob, struct HookModifierData *hmd);
 void BKE_object_modifier_gpencil_hook_reset(struct Object *ob,
                                             struct HookGpencilModifierData *hmd);
-bool BKE_object_modifier_gpencil_use_time(struct Object *ob, struct GpencilModifierData *md);
-
-bool BKE_object_shaderfx_use_time(struct Object *ob, struct ShaderFxData *fx);
 
 /**
  * \return True if the object's type supports regular modifiers (not grease pencil modifiers).
@@ -633,8 +630,6 @@ void BKE_object_groups_clear(struct Main *bmain, struct Scene *scene, struct Obj
  */
 struct KDTree_3d *BKE_object_as_kdtree(struct Object *ob, int *r_tot);
 
-bool BKE_object_modifier_use_time(struct Scene *scene, struct Object *ob, struct ModifierData *md);
-
 /**
  * \note this function should eventually be replaced by depsgraph functionality.
  * Avoid calling this in new code unless there is a very good reason for it!
diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc
index 5ed1ac98ddf..f7436b6112c 100644
--- a/source/blender/blenkernel/intern/object.cc
+++ b/source/blender/blenkernel/intern/object.cc
@@ -5357,126 +5357,6 @@ KDTree_3d *BKE_object_as_kdtree(Object *ob, int *r_tot)
 /** \name Object Modifier Utilities
  * \{ */
 
-bool BKE_object_modifier_use_time(Scene *scene, Object *ob, ModifierData *md)
-{
-  if (BKE_modifier_depends_ontime(scene, md)) {
-    return true;
-  }
-
-  /* Check whether modifier is animated. */
-  /* TODO: this should be handled as part of build_animdata() -- Aligorith */
-  if (ob->adt) {
-    AnimData *adt = ob->adt;
-    FCurve *fcu;
-
-    char md_name_esc[sizeof(md->name) * 2];
-    BLI_str_escape(md_name_esc, md->name, sizeof(md_name_esc));
-
-    char pattern[sizeof(md_name_esc) + 16];
-    BLI_snprintf(pattern, sizeof(pattern), "modifiers[\"%s\"]", md_name_esc);
-
-    /* action - check for F-Curves with paths containing 'modifiers[' */
-    if (adt->action) {
-      for (fcu = (FCurve *)adt->action->curves.first; fcu != nullptr; fcu = (FCurve *)fcu->next) {
-        if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-          return true;
-        }
-      }
-    }
-
-    /* This here allows modifier properties to get driven and still update properly
-     *
-     * Workaround to get T26764 (e.g. subsurf levels not updating when animated/driven)
-     * working, without the updating problems (T28525 T28690 T28774 T28777) caused
-     * by the RNA updates cache introduced in r.38649
-     */
-    for (fcu = (FCurve *)adt->drivers.first; fcu != nullptr; fcu = (FCurve *)fcu->next) {
-      if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-        return true;
-      }
-    }
-
-    /* XXX: also, should check NLA strips, though for now assume that nobody uses
-     * that and we can omit that for performance reasons... */
-  }
-
-  return false;
-}
-
-bool BKE_object_modifier_gpencil_use_time(Object *ob, GpencilModifierData *md)
-{
-  if (BKE_gpencil_modifier_depends_ontime(md)) {
-    return true;
-  }
-
-  /* Check whether modifier is animated. */
-  /* TODO(Aligorith): this should be handled as part of build_animdata() */
-  if (ob->adt) {
-    AnimData *adt = ob->adt;
-
-    char md_name_esc[sizeof(md->name) * 2];
-    BLI_str_escape(md_name_esc, md->name, sizeof(md_name_esc));
-
-    char pattern[sizeof(md_name_esc) + 32];
-    BLI_snprintf(pattern, sizeof(pattern), "grease_pencil_modifiers[\"%s\"]", md_name_esc);
-
-    /* action - check for F-Curves with paths containing 'grease_pencil_modifiers[' */
-    if (adt->action) {
-      LISTBASE_FOREACH (FCurve *, fcu, &adt->action->curves) {
-        if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-          return true;
-        }
-      }
-    }
-
-    /* This here allows modifier properties to get driven and still update properly */
-    LISTBASE_FOREACH (FCurve *, fcu, &adt->drivers) {
-      if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
-bool BKE_object_shaderfx_use_time(Object *ob, ShaderFxData *fx)
-{
-  if (BKE_shaderfx_depends_ontime(fx)) {
-    return true;
-  }
-
-  /* Check whether effect is animated. */
-  /* TODO(Aligorith): this should be handled as part of build_animdata() */
-  if (ob->adt) {
-    AnimData *adt = ob->adt;
-
-    char fx_name_esc[sizeof(fx->name) * 2];
-    BLI_str_escape(fx_name_esc, fx->name, sizeof(fx_name_esc));
-
-    char pattern[sizeof(fx_name_esc) + 32];
-    BLI_snprintf(pattern, sizeof(pattern), "shader_effects[\"%s\"]", fx_name_esc);
-
-    /* action - check for F-Curves with paths containing string[' */
-    if (adt->action) {
-      LISTBASE_FOREACH (FCurve *, fcu, &adt->action->curves) {
-        if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-          return true;
-        }
-      }
-    }
-
-    /* This here allows properties to get driven and still update properly */
-    LISTBASE_FOREACH (FCurve *, fcu, &adt->drivers) {
-      if (fcu->rna_path && strstr(fcu->rna_path, pattern)) {
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
 /**
  * Set "ignore cache" flag for all caches on this object.
  */
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index afcf3eaacb7..31d5308e825 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -2189,7 +2189,7 @@ void DepsgraphRelationBuilder::build_object_data_geometry(Object *object)
         ctx.node = reinterpret_cast<::DepsNodeHandle *>(&handle);
         mti->updateDepsgraph(md, &ctx);
       }
-      if (BKE_object_modifier_use_time(scene_, object, md)) {
+      if (BKE_modifier_depends_ontime(scene_, md)) {
         TimeSourceKey time_src_key;
         add_relation(time_src_key, obdata_ubereval_key, "Time Source -> Modifier");
       }
@@ -2208,7 +2208,7 @@ void DepsgraphRelationBuilder::build_object_data_geometry(Object *object)
         ctx.node = reinterpret_cast<::DepsNodeHandle *>(&handle);
         mti->updateDepsgraph(md, &ctx, graph_->mode);
       }
-      if (BKE_object_modifier_gpencil_use_time(object, md)) {
+      if (BKE_gpencil_modifier_depends_ontime(md)) {
         TimeSourceKey time_src_key;
         add_relation(time_src_key, obdata_ubereval_key, "Time Source");
       }
@@ -2226,7 +2226,7 @@ void DepsgraphRelationBuilder::build_object_data_geometry(Object *object)
         ctx.node = reinterpret_cast<::DepsNodeHandle *>(&handle);
         fxi->updateDepsgraph(fx, &ctx);
       }
-      if (BKE_object_shaderfx_use_time(object, fx)) {
+      if (BKE_shaderfx_depends_ontime(fx)) {
         TimeSourceKey time_src_key;
         add_relation(time_src_key, obdata_ubereval_key, "Time Source");
       }
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_rna.cc b/source/blender/depsgraph/intern/builder/deg_builder_rna.cc
index ac7a5bc2f30..5202ada5408 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_rna.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_rna.cc
@@ -271,7 +271,8 @@ RNANodeIdentifier RNANodeQuery::construct_node_identifier(const PointerRNA *ptr,
            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)) {
+           RNA_struct_is_a(ptr->type, &RNA_VertexGroupElement) ||
+           RNA_struct_is_a(ptr->type, &RNA_ShaderFx)) {
     /* 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. */



More information about the Bf-blender-cvs mailing list