From fc1f5bded46afbb9b16fffe9e4c7f7c212566255 Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Sun, 19 Jan 2020 12:44:34 +0300 Subject: 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 --- source/blender/blenkernel/BKE_fcurve.h | 1 + source/blender/blenkernel/intern/fcurve.c | 69 +++++++++++++++++++--- source/blender/blenlib/BLI_expr_pylike_eval.h | 1 + source/blender/blenlib/intern/expr_pylike_eval.c | 18 ++++++ .../intern/builder/deg_builder_relations.cc | 28 +-------- 5 files changed, 83 insertions(+), 34 deletions(-) (limited to 'source/blender') 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; -- cgit v1.2.3