Passing Dependency Graph to Drivers #77086

Closed
opened 2020-05-26 17:31:03 +02:00 by Sybren A. Stüvel · 15 comments

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.
Author
Member

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

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

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

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

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)` ?

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.
Author
Member

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.

Added subscriber: @StephenSwaney

Added subscriber: @StephenSwaney
Member

Added subscriber: @Mets

Added subscriber: @Mets

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.

Added subscriber: @Russ1642

Added subscriber: @Russ1642

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.

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.
Author
Member

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.

Added subscriber: @slumber

Added subscriber: @slumber

Added subscriber: @bblanimation

Added subscriber: @bblanimation
Author
Member

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'
Sybren A. Stüvel self-assigned this 2020-07-21 15:26:57 +02:00
Author
Member

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
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
8 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#77086
No description provided.