Depsgraph: fix false positive time dependencies for simple drivers.

The dependency graph has to know whether a driver must be re-evaluated
every frame due to a dependency on the current frame number. For python
drivers it was using a heuristic based on searching for certain sub-
strings in the expression, notably including '('.

When the expression is actually evaluated using Python, this can't be
easily improved; however if the Simple Expression evaluator is used,
this check can be done precisely by accessing the parsed data.

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

5 files changed, 83 insertions, 34 deletions

diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h
index 426e0ed4b0e..405b052f477 100644
--- a/source/blender/blenkernel/BKE_fcurve.h
+++ b/source/blender/blenkernel/BKE_fcurve.h
@@ -107,6 +107,7 @@ bool driver_get_variable_property(struct ChannelDriver *driver,
                                   int *r_index);
 
 bool BKE_driver_has_simple_expression(struct ChannelDriver *driver);
+bool BKE_driver_expression_depends_on_time(struct ChannelDriver *driver);
 void BKE_driver_invalidate_expression(struct ChannelDriver *driver,
                                       bool expr_changed,
                                       bool varname_changed);
diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index 08687ef8cee..833b8409f7d 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -2142,20 +2142,34 @@ ChannelDriver *fcurve_copy_driver(const ChannelDriver *driver)
 
 /* Driver Expression Evaluation --------------- */
 
+/* Index constants for the expression parameter array. */
+enum {
+  /* Index of the 'frame' variable. */
+  VAR_INDEX_FRAME = 0,
+  /* Index of the first user-defined driver variable. */
+  VAR_INDEX_CUSTOM
+};
+
 static ExprPyLike_Parsed *driver_compile_simple_expr_impl(ChannelDriver *driver)
 {
   /* Prepare parameter names. */
   int names_len = BLI_listbase_count(&driver->variables);
-  const char **names = BLI_array_alloca(names, names_len + 1);
-  int i = 0;
+  const char **names = BLI_array_alloca(names, names_len + VAR_INDEX_CUSTOM);
+  int i = VAR_INDEX_CUSTOM;
 
-  names[i++] = "frame";
+  names[VAR_INDEX_FRAME] = "frame";
 
   for (DriverVar *dvar = driver->variables.first; dvar; dvar = dvar->next) {
     names[i++] = dvar->name;
   }
 
-  return BLI_expr_pylike_parse(driver->expression, names, names_len + 1);
+  return BLI_expr_pylike_parse(driver->expression, names, names_len + VAR_INDEX_CUSTOM);
+}
+
+static bool driver_check_simple_expr_depends_on_time(ExprPyLike_Parsed *expr)
+{
+  /* Check if the 'frame' parameter is actually used. */
+  return BLI_expr_pylike_is_using_param(expr, VAR_INDEX_FRAME);
 }
 
 static bool driver_evaluate_simple_expr(ChannelDriver *driver,
@@ -2165,10 +2179,10 @@ static bool driver_evaluate_simple_expr(ChannelDriver *driver,
 {
   /* Prepare parameter values. */
   int vars_len = BLI_listbase_count(&driver->variables);
-  double *vars = BLI_array_alloca(vars, vars_len + 1);
-  int i = 0;
+  double *vars = BLI_array_alloca(vars, vars_len + VAR_INDEX_CUSTOM);
+  int i = VAR_INDEX_CUSTOM;
 
-  vars[i++] = time;
+  vars[VAR_INDEX_FRAME] = time;
 
   for (DriverVar *dvar = driver->variables.first; dvar; dvar = dvar->next) {
     vars[i++] = driver_get_variable_value(driver, dvar);
@@ -2176,7 +2190,8 @@ static bool driver_evaluate_simple_expr(ChannelDriver *driver,
 
   /* Evaluate expression. */
   double result_val;
-  eExprPyLike_EvalStatus status = BLI_expr_pylike_eval(expr, vars, vars_len + 1, &result_val);
+  eExprPyLike_EvalStatus status = BLI_expr_pylike_eval(
+      expr, vars, vars_len + VAR_INDEX_CUSTOM, &result_val);
   const char *message;
 
   switch (status) {
@@ -2245,6 +2260,44 @@ bool BKE_driver_has_simple_expression(ChannelDriver *driver)
   return driver_compile_simple_expr(driver) && BLI_expr_pylike_is_valid(driver->expr_simple);
 }
 
+/* TODO(sergey): This is somewhat weak, but we don't want neither false-positive
+ * time dependencies nor special exceptions in the depsgraph evaluation. */
+static bool python_driver_exression_depends_on_time(const char *expression)
+{
+  if (expression[0] == '\0') {
+    /* Empty expression depends on nothing. */
+    return false;
+  }
+  if (strchr(expression, '(') != NULL) {
+    /* Function calls are considered dependent on a time. */
+    return true;
+  }
+  if (strstr(expression, "frame") != NULL) {
+    /* Variable `frame` depends on time. */
+    /* TODO(sergey): This is a bit weak, but not sure about better way of handling this. */
+    return true;
+  }
+  /* Possible indirect time relation s should be handled via variable targets. */
+  return false;
+}
+
+/* Check if the expression in the driver may depend on the current frame. */
+bool BKE_driver_expression_depends_on_time(ChannelDriver *driver)
+{
+  if (driver->type != DRIVER_TYPE_PYTHON) {
+    return false;
+  }
+
+  if (BKE_driver_has_simple_expression(driver)) {
+    /* Simple expressions can be checked exactly. */
+    return driver_check_simple_expr_depends_on_time(driver->expr_simple);
+  }
+  else {
+    /* Otherwise, heuristically scan the expression string for certain patterns. */
+    return python_driver_exression_depends_on_time(driver->expression);
+  }
+}
+
 /* Reset cached compiled expression data */
 void BKE_driver_invalidate_expression(ChannelDriver *driver,
                                       bool expr_changed,
diff --git a/source/blender/blenlib/BLI_expr_pylike_eval.h b/source/blender/blenlib/BLI_expr_pylike_eval.h
index b8bf88dd85b..1db91ea4205 100644
--- a/source/blender/blenlib/BLI_expr_pylike_eval.h
+++ b/source/blender/blenlib/BLI_expr_pylike_eval.h
@@ -45,6 +45,7 @@ typedef enum eExprPyLike_EvalStatus {
 void BLI_expr_pylike_free(struct ExprPyLike_Parsed *expr);
 bool BLI_expr_pylike_is_valid(struct ExprPyLike_Parsed *expr);
 bool BLI_expr_pylike_is_constant(struct ExprPyLike_Parsed *expr);
+bool BLI_expr_pylike_is_using_param(struct ExprPyLike_Parsed *expr, int index);
 ExprPyLike_Parsed *BLI_expr_pylike_parse(const char *expression,
                                          const char **param_names,
                                          int param_names_len);
diff --git a/source/blender/blenlib/intern/expr_pylike_eval.c b/source/blender/blenlib/intern/expr_pylike_eval.c
index 43923ce8c98..6020dc41a62 100644
--- a/source/blender/blenlib/intern/expr_pylike_eval.c
+++ b/source/blender/blenlib/intern/expr_pylike_eval.c
@@ -140,6 +140,24 @@ bool BLI_expr_pylike_is_constant(ExprPyLike_Parsed *expr)
   return expr != NULL && expr->ops_count == 1 && expr->ops[0].opcode == OPCODE_CONST;
 }
 
+/** Check if the parsed expression uses the parameter with the given index. */
+bool BLI_expr_pylike_is_using_param(ExprPyLike_Parsed *expr, int index)
+{
+  int i;
+
+  if (expr == NULL) {
+    return false;
+  }
+
+  for (i = 0; i < expr->ops_count; i++) {
+    if (expr->ops[i].opcode == OPCODE_PARAMETER && expr->ops[i].arg.ival == index) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index 7d7a183ee75..fa7a63227f5 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -123,28 +123,6 @@ namespace DEG {
 
 namespace {
 
-/* TODO(sergey): This is somewhat weak, but we don't want neither false-positive
- * time dependencies nor special exceptions in the depsgraph evaluation. */
-
-bool python_driver_exression_depends_on_time(const char *expression)
-{
-  if (expression[0] == '\0') {
-    /* Empty expression depends on nothing. */
-    return false;
-  }
-  if (strchr(expression, '(') != NULL) {
-    /* Function calls are considered dependent on a time. */
-    return true;
-  }
-  if (strstr(expression, "frame") != NULL) {
-    /* Variable `frame` depends on time. */
-    /* TODO(sergey): This is a bit weak, but not sure about better way of handling this. */
-    return true;
-  }
-  /* Possible indirect time relation s should be handled via variable targets. */
-  return false;
-}
-
 bool driver_target_depends_on_time(const DriverTarget *target)
 {
   if (target->idtype == ID_SCE &&
@@ -176,10 +154,8 @@ bool driver_variables_depends_on_time(const ListBase *variables)
 
 bool driver_depends_on_time(ChannelDriver *driver)
 {
-  if (driver->type == DRIVER_TYPE_PYTHON) {
-    if (python_driver_exression_depends_on_time(driver->expression)) {
-      return true;
-    }
+  if (BKE_driver_expression_depends_on_time(driver)) {
+    return true;
   }
   if (driver_variables_depends_on_time(&driver->variables)) {
     return true;