Passing Dependency Graph to Drivers #77086

New Issue

Sybren A. Stüvel · 2020-05-26T17:31:03+02:00

Sybren A. Stüvel commented

2020-05-26 17:31:03 +02:00

Problem Description

Driver functions written in Python have access to neither the context nor the dependency graph that is evaluating the driver. As a result, it is not possible to reliably obtain the current view layer. Accessing bpy.context.view_layer is not a valid option here (see #75553), as there could be multiple view layers being evaluated at once (one rendering and one being shown in the viewport, for example).

Proposed Solution

I think that conceptually the nicest solution for this is to make the currently evaluating depsgraph available to drivers. This can be done by passing it in the same way as we pass other driver variables, in BPY_driver_exec(). The driver "namespace" is not directly suitable for this, as it is a global object and thus has the same issues as accessing bpy.context.

To make BPY_driver_exec() aware of the current depsgraph, I see a few possible approaches:

Pass a Depsgraph * as parameter all the way down to the BPY_driver_exec() call. This is conceptually the simplest solution, but it does require changing a lot of functions (see the call tree below). This approach can be broken down into:
- Add a new Depsgraph * parameter to each function.
- Replace the float time parameters with a Depsgraph *, and use the depsgraph to obtain the time. This looks nice, but the evaluation time of the depsgraph and of the animation system can be different (for example when using the 'Animated Strip Time' of an NLA strip).
- Bundle the Depsgraph * and the float time into a new struct, and pass that instead of the time parameter.
Use a thread-local global variable to store the current Depsgraph *. This avoids making changes to a lot of functions. However, given that people have worked hard to remove as much G.main from Blender's codebase, I don't think adding new globals is the way forward. Furthermore, this would require very careful consideration when creating and destroying dependency graphs.
Convert the animation code to C++, so that a more object-oriented model can be used, and the Depsgraph * stored in a member variable rather than passed to each function.

Call tree of `BPY_driver_exec`

The BPY_driver_exec() function is called by evaluate_driver_python(), etc. Every indentation level means "is called by". Functions are only listed once.

BPY_driver_exec(time)
- evaluate_driver_python(time)
  - evaluate_driver(time)
    - evaluate_fcurve_driver(time)
    - calculate_fcurve(time)
      - animsys_evaluate_fcurves(time)
      - animsys_evaluate_action_ex(time)
        
        animsys_evaluate_action(time)
        
        BKE_animsys_evaluate_animdata(time)
        
        what_does_obaction(time)
        
        actcon_get_tarmat(depsgraph)
        
        BKE_animsys_evaluate_all_animation(depsgraph)
        
        BKE_animsys_eval_animdata(depsgraph)
      - nlastrip_evaluate_controls(time)
        
        nlastrips_ctime_get_strip(time)
        
        nlastrip_evaluate_meta() with time = strip-dependent
        
        nlastrip_evaluate()
        
        nlastrip_evaluate_transition()
        
        animsys_evaluate_nla(time)
        
        animsys_calculate_nla(time)
        
        BKE_animsys_get_nla_keyframing_context(time)
        
        achannel_setting_slider_cb(context)
        
        achannel_setting_slider_shapekey_cb(context)
        
        insert_keyframe(time)
        
        insert_key_button_exec(context)
        
        ED_autokeyframe_property(context)
        
        insert_action_keys(bAnimContext)
        
        insert_graph_keys(bAnimContext)
        
        autokeyframe_object(bAnimContext)
        
        autokeyframe_pose(bAnimContext)
        
        pyrna_struct_keyframe_insert(NOTHING) (no context available)
      - animsys_evaluate_drivers(time)
      - animsys_evaluate_action_group(time)
        
        poselib_apply_pose(time)
        
        poselib_preview_apply(context)
      - BKE_animsys_eval_driver(depsgraph)
      - fcurve_is_changed(time)
        
        ui_but_anim_flag(time)
        
        UI_context_update_anim_flag(context)
        
        UI_block_end_ex(context)
    - insert_keyframe_value(time)
      - insert_keyframe_direct(time)
      - insert_keyframe_fcurve_value(time)

bAnimContext objects are created by ANIM_animdata_get_context(context), so they could be extended with a Depsgraph * from the context.

### Problem Description Driver functions written in Python have access to neither the context nor the dependency graph that is evaluating the driver. As a result, it is not possible to reliably obtain the current view layer. Accessing `bpy.context.view_layer` is not a valid option here (see #75553), as there could be multiple view layers being evaluated at once (one rendering and one being shown in the viewport, for example). ### Proposed Solution I think that conceptually the nicest solution for this is to make the currently evaluating depsgraph available to drivers. This can be done by passing it in the same way as we pass other driver variables, in `BPY_driver_exec()`. The driver "namespace" is not directly suitable for this, as it is a global object and thus has the same issues as accessing `bpy.context`. To make `BPY_driver_exec()` aware of the current depsgraph, I see a few possible approaches: - Pass a `Depsgraph *` as parameter all the way down to the `BPY_driver_exec()` call. This is conceptually the simplest solution, but it does require changing a lot of functions (see the call tree below). This approach can be broken down into: - Add a new `Depsgraph *` parameter to each function. - Replace the `float time` parameters with a `Depsgraph *`, and use the depsgraph to obtain the time. This looks nice, but the evaluation time of the depsgraph and of the animation system can be different (for example when using the 'Animated Strip Time' of an NLA strip). - Bundle the `Depsgraph *` and the `float time` into a new struct, and pass that instead of the `time` parameter. - Use a thread-local global variable to store the current `Depsgraph *`. This avoids making changes to a lot of functions. However, given that people have worked hard to remove as much `G.main` from Blender's codebase, I don't think adding new globals is the way forward. Furthermore, this would require very careful consideration when creating and destroying dependency graphs. - Convert the animation code to C++, so that a more object-oriented model can be used, and the `Depsgraph *` stored in a member variable rather than passed to each function. ### Call tree of `BPY_driver_exec` The `BPY_driver_exec()` function is called by `evaluate_driver_python()`, etc. Every indentation level means "is called by". Functions are only listed once. - `BPY_driver_exec(time)` - `evaluate_driver_python(time)` - `evaluate_driver(time)` - `evaluate_fcurve_driver(time)` - `calculate_fcurve(time)` - `animsys_evaluate_fcurves(time)` - `animsys_evaluate_action_ex(time)` - `animsys_evaluate_action(time)` - `BKE_animsys_evaluate_animdata(time)` - `what_does_obaction(time)` - `actcon_get_tarmat(depsgraph)` - `BKE_animsys_evaluate_all_animation(depsgraph)` - `BKE_animsys_eval_animdata(depsgraph)` - `nlastrip_evaluate_controls(time)` - `nlastrips_ctime_get_strip(time)` - `nlastrip_evaluate_meta()` with time = strip-dependent - `nlastrip_evaluate()` - `nlastrip_evaluate_transition()` - `animsys_evaluate_nla(time)` - `animsys_calculate_nla(time)` - `BKE_animsys_get_nla_keyframing_context(time)` - `achannel_setting_slider_cb(context)` - `achannel_setting_slider_shapekey_cb(context)` - `insert_keyframe(time)` - `insert_key_button_exec(context)` - `ED_autokeyframe_property(context)` - `insert_action_keys(bAnimContext)` - `insert_graph_keys(bAnimContext)` - `autokeyframe_object(bAnimContext)` - `autokeyframe_pose(bAnimContext)` - `pyrna_struct_keyframe_insert(NOTHING)` (no context available) - `animsys_evaluate_drivers(time)` - `animsys_evaluate_action_group(time)` - `poselib_apply_pose(time)` - `poselib_preview_apply(context)` - `BKE_animsys_eval_driver(depsgraph)` - `fcurve_is_changed(time)` - `ui_but_anim_flag(time)` - `UI_context_update_anim_flag(context)` - `UI_block_end_ex(context)` - `insert_keyframe_value(time)` - `insert_keyframe_direct(time)` - `insert_keyframe_fcurve_value(time)` `bAnimContext` objects are created by `ANIM_animdata_get_context(context)`, so they could be extended with a `Depsgraph *` from the context.

Sybren A. Stüvel commented

2020-05-26 17:31:03 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Sybren A. Stüvel commented

2020-05-26 17:31:03 +02:00

Added subscribers: @dr.sybren, @Sergey, @brecht, @ideasman42

Sergey Sharybin commented

2020-05-26 17:46:09 +02:00

I am all up for using more OOP. But afraid it is a way bigger project than any other approach. And it might still need passing a context of some sort, maybe.

From future-proof point of view I think is best to refactor the code so that BPY_driver_exec receives AnimationEvalContext, even if it consists of a single Depsgraph* field. For the time -- i think we should not pass it explicitly, "just" get it from depsgraph.

TLS I do not like at all. Makes following the code more difficult, makes troubleshooting more difficult. And having a pool of worker threads which can evaluate different dependency graphs makes things sound fragile.

That's the "technical" aspect of the proposal. From the user level if will be "just" expression like my_driver_evaluaiton_function(depsgraph) ?

I am all up for using more OOP. But afraid it is a way bigger project than any other approach. And it might still need passing a context of some sort, maybe. From future-proof point of view I think is best to refactor the code so that `BPY_driver_exec` receives `AnimationEvalContext`, even if it consists of a single `Depsgraph*` field. For the time -- i think we should not pass it explicitly, "just" get it from `depsgraph`. TLS I do not like at all. Makes following the code more difficult, makes troubleshooting more difficult. And having a pool of worker threads which can evaluate different dependency graphs makes things sound fragile. That's the "technical" aspect of the proposal. From the user level if will be "just" expression like `my_driver_evaluaiton_function(depsgraph)` ?

Brecht Van Lommel commented

2020-05-26 18:14:30 +02:00

I think passing down both a Depsgraph* and float time where needed through these functions is fine.

The fact that the time may be different than the depsgraph time is weak, but I'm not sure how to avoid that without deeper NLA changes. I would just leave that behavior as is.

It would be good if we could actually make it so bpy.context is valid for access from Python drivers and app handlers. For Python drivers it would contain only data like scene, view layer and depsgraph. No UI, no active object, selected objects, etc. But I'm not sure how hard that is to implement. Either way, for that we'd also need to pass the depsgraph down to the driver function.

I think passing down both a `Depsgraph*` and `float time` where needed through these functions is fine. The fact that the time may be different than the depsgraph time is weak, but I'm not sure how to avoid that without deeper NLA changes. I would just leave that behavior as is. It would be good if we could actually make it so `bpy.context` is valid for access from Python drivers and app handlers. For Python drivers it would contain only data like scene, view layer and depsgraph. No UI, no active object, selected objects, etc. But I'm not sure how hard that is to implement. Either way, for that we'd also need to pass the depsgraph down to the driver function.

Sybren A. Stüvel commented

2020-05-26 18:33:51 +02:00

In #77086#939052, @Sergey wrote:
From future-proof point of view I think is best to refactor the code so that BPY_driver_exec receives AnimationEvalContext, even if it consists of a single Depsgraph* field. For the time -- i think we should not pass it explicitly, "just" get it from depsgraph.

I agree.

That's the "technical" aspect of the proposal. From the user level if will be "just" expression like my_driver_evaluaiton_function(depsgraph) ?

Yes. By putting depsgraph in the local scope we can treat it like any other Driver variable. I don't see another practical way to do this.

In #77086#939058, @brecht wrote:
I think passing down both a Depsgraph* and float time where needed through these functions is fine.

I'm not a fan of having sets of parameters that are always passed to other functions in tandem. I think it's okay-ish to have a distinction between "fcurve/driver evaluation time" and "current depsgraph time", but if those are always passed together I'm more in favour of Sergey's AnimationEvalContext.

The fact that the time may be different than the depsgraph time is weak, but I'm not sure how to avoid that without deeper NLA changes. I would just leave that behavior as is.

I'll dive more into how exactly the Animated Strip Time in the NLA editor works, and whether that's really impacting drivers. Regardless, IMO it should be clear which functions operate on depsgraph time and which ones can receive an arbitrary time.

It would be good if we could actually make it so bpy.context is valid for access from Python drivers and app handlers. For Python drivers it would contain only data like scene, view layer and depsgraph. No UI, no active object, selected objects, etc. But I'm not sure how hard that is to implement. Either way, for that we'd also need to pass the depsgraph down to the driver function.

There is already a _RestrictContext Python class and an accompanying RestrictBlend context manager that do something similar while Blender is starting. However, this manipulates the global bpy.context, which is not something we want in this case. Instead of having bpy.context behave differently in different threads, we could document bpy.context access for drivers/handlers as "not allowed", and pass a suitable context to the driver in its local variables.

> In #77086#939052, @Sergey wrote: > From future-proof point of view I think is best to refactor the code so that `BPY_driver_exec` receives `AnimationEvalContext`, even if it consists of a single `Depsgraph*` field. For the time -- i think we should not pass it explicitly, "just" get it from `depsgraph`. I agree. > That's the "technical" aspect of the proposal. From the user level if will be "just" expression like `my_driver_evaluaiton_function(depsgraph)` ? Yes. By putting `depsgraph` in the local scope we can treat it like any other Driver variable. I don't see another practical way to do this. > In #77086#939058, @brecht wrote: > I think passing down both a `Depsgraph*` and `float time` where needed through these functions is fine. I'm not a fan of having sets of parameters that are always passed to other functions in tandem. I think it's okay-ish to have a distinction between "fcurve/driver evaluation time" and "current depsgraph time", but if those are always passed together I'm more in favour of Sergey's `AnimationEvalContext`. > The fact that the time may be different than the depsgraph time is weak, but I'm not sure how to avoid that without deeper NLA changes. I would just leave that behavior as is. I'll dive more into how exactly the Animated Strip Time in the NLA editor works, and whether that's really impacting drivers. Regardless, IMO it should be clear which functions operate on depsgraph time and which ones can receive an arbitrary time. > It would be good if we could actually make it so `bpy.context` is valid for access from Python drivers and app handlers. For Python drivers it would contain only data like scene, view layer and depsgraph. No UI, no active object, selected objects, etc. But I'm not sure how hard that is to implement. Either way, for that we'd also need to pass the depsgraph down to the driver function. There is already a `_RestrictContext` Python class and an accompanying `RestrictBlend` context manager that do something similar while Blender is starting. However, this manipulates the global `bpy.context`, which is not something we want in this case. Instead of having `bpy.context` behave differently in different threads, we could document `bpy.context` access for drivers/handlers as "not allowed", and pass a suitable `context` to the driver in its local variables.

Stephen Swaney commented

2020-05-27 03:02:02 +02:00

Added subscriber: @StephenSwaney

Demeter Dzadik commented

2020-05-27 18:09:33 +02:00

Added subscriber: @Mets

Brecht Van Lommel commented

2020-05-27 19:52:12 +02:00

In #77086#939069, @dr.sybren wrote:
There is already a _RestrictContext Python class and an accompanying RestrictBlend context manager that do something similar while Blender is starting. However, this manipulates the global bpy.context, which is not something we want in this case. Instead of having bpy.context behave differently in different threads, we could document bpy.context access for drivers/handlers as "not allowed", and pass a suitable context to the driver in its local variables.

I don't think a context would providing anything that isn't already in depsgraph, so I wouldn't do that.

Documenting isn't really effective for this case because users are unlikely to be looking at the relevant docs when writing the wrong code. The original design for Context was definitely that it should have a different value in different threads, but instead it has become more of a global variable in both the C and Python code. Multiple threads using the same context is fundamentally broken, there's no way to use it reliably then.

I'm not saying we have to solve this problem now though, just to me it seems like a more complete solution to this problem.

> In #77086#939069, @dr.sybren wrote: > There is already a `_RestrictContext` Python class and an accompanying `RestrictBlend` context manager that do something similar while Blender is starting. However, this manipulates the global `bpy.context`, which is not something we want in this case. Instead of having `bpy.context` behave differently in different threads, we could document `bpy.context` access for drivers/handlers as "not allowed", and pass a suitable `context` to the driver in its local variables. I don't think a `context` would providing anything that isn't already in `depsgraph`, so I wouldn't do that. Documenting isn't really effective for this case because users are unlikely to be looking at the relevant docs when writing the wrong code. The original design for `Context` was definitely that it should have a different value in different threads, but instead it has become more of a global variable in both the C and Python code. Multiple threads using the same context is fundamentally broken, there's no way to use it reliably then. I'm not saying we have to solve this problem now though, just to me it seems like a more complete solution to this problem.

Russell commented

2020-05-28 17:12:22 +02:00

Added subscriber: @Russ1642

Russell commented

2020-05-28 17:12:22 +02:00

My experience is that passing around depsgraph objects is painful, prone to error, and difficult to debug. If there's a way to handle this in a more automated fashion then I'm all for that. It would be great if code didn't have to be completely rewritten just because you're rendering an animation rather than a still frame.

Sybren A. Stüvel commented

2020-05-29 17:16:36 +02:00

In #77086#940237, @Russ1642 wrote:
My experience is that passing around depsgraph objects is painful, prone to error, and difficult to debug.

Why? It's the same as passing around the context parameter passed operators, and that's done all over the place.

If there's a way to handle this in a more automated fashion then I'm all for that. It would be great if code didn't have to be completely rewritten just because you're rendering an animation rather than a still frame.

This is about drivers in Python, and all they have to do is replace some_call() with some_call(depsgraph), and then use that instead of bpy.context.view_layer.depsgraph. I think calling this a complete rewrite is hyperbole.

> In #77086#940237, @Russ1642 wrote: > My experience is that passing around depsgraph objects is painful, prone to error, and difficult to debug. Why? It's the same as passing around the `context` parameter passed operators, and that's done all over the place. > If there's a way to handle this in a more automated fashion then I'm all for that. It would be great if code didn't have to be completely rewritten just because you're rendering an animation rather than a still frame. This is about drivers in Python, and all they have to do is replace `some_call()` with `some_call(depsgraph)`, and then use that instead of `bpy.context.view_layer.depsgraph`. I think calling this a complete rewrite is hyperbole.

Swann Martinez commented

2020-06-08 11:06:32 +02:00

Added subscriber: @slumber

Christopher Gearhart commented

2020-06-08 20:08:35 +02:00

Added subscriber: @bblanimation

Sybren A. Stüvel commented

2020-07-21 15:26:57 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Sybren A. Stüvel closed this issue

2020-07-21 15:26:57 +02:00

Sybren A. Stüvel self-assigned this 2020-07-21 15:26:57 +02:00

Sybren A. Stüvel commented

2020-07-21 15:26:57 +02:00

This was resolved in 686ab4c940.

This was resolved in 686ab4c940.

Thomas Dinges added this to the 2.90 milestone 2023-02-08 16:27:16 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Passing Dependency Graph to Drivers #77086

Problem Description

Proposed Solution

Call tree of BPY_driver_exec

Call tree of `BPY_driver_exec`