From 94dde6866cc24ef86e4cc198a05aed8ff2526cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 17 Apr 2023 11:35:18 +0200 Subject: [PATCH 1/2] Fix #106943: driver on inactive view layer doesn't work Animation data (including drivers) on inactive view layers now work. The removal of such view layers was too optimistic; they are now kept around. The bases are still removed, mostly for safety sake and to keep the changes to a minimum. `scene_remove_unused_view_layers()` has been renamed to `scene_minimize_unused_view_layers()` to reflect its new functionality. For compatibility with assumptions in other areas of the code, the function still ensures the input view layer is at index 0. --- .../intern/eval/deg_eval_copy_on_write.cc | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc index 604add1e933..bae10c21166 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc @@ -353,12 +353,23 @@ ViewLayer *get_original_view_layer(const Depsgraph *depsgraph, const IDNode *id_ return nullptr; } -/* Remove all view layers but the one which corresponds to an input one. */ -void scene_remove_unused_view_layers(const Depsgraph *depsgraph, - const IDNode *id_node, - Scene *scene_cow) +/** + * Clear all bases from the view layer. + * + * This is used when the view layer needs to stick around, for example when it's + * used in the compositor or when it has drivers, but the objects in that view + * layer are irrelevant. + */ +static void view_layer_unassign_bases(ViewLayer *view_layer) +{ + view_layer->basact = nullptr; +} + +/* Remove all bases from all view layers except the input one. */ +void scene_minimize_unused_view_layers(const Depsgraph *depsgraph, + const IDNode *id_node, + Scene *scene_cow) { - const ViewLayer *view_layer_input; if (depsgraph->is_render_pipeline_depsgraph) { /* If the dependency graph is used for post-processing (such as compositor) we do need to * have access to its view layer names so can not remove any view layers. @@ -370,18 +381,12 @@ void scene_remove_unused_view_layers(const Depsgraph *depsgraph, * NOTE: Need to keep view layers for all scenes, even indirect ones. This is because of * render layer node possibly pointing to another scene. */ LISTBASE_FOREACH (ViewLayer *, view_layer, &scene_cow->view_layers) { - view_layer->basact = nullptr; + view_layer_unassign_bases(view_layer); } return; } - if (id_node->linked_state == DEG_ID_LINKED_INDIRECTLY) { - /* Indirectly linked scenes means it's not an input scene and not a set scene, and is pulled - * via some driver. Such scenes should not have view layers after copy. */ - view_layer_input = nullptr; - } - else { - view_layer_input = get_original_view_layer(depsgraph, id_node); - } + + const ViewLayer *view_layer_input = get_original_view_layer(depsgraph, id_node); ViewLayer *view_layer_eval = nullptr; /* Find evaluated view layer. At the same time we free memory used by * all other of the view layers. */ @@ -394,15 +399,16 @@ void scene_remove_unused_view_layers(const Depsgraph *depsgraph, view_layer_eval = view_layer_cow; } else { - BKE_view_layer_free_ex(view_layer_cow, false); + view_layer_unassign_bases(view_layer_cow); } } - /* Make evaluated view layer the only one in the evaluated scene (if it exists). */ + + /* Make evaluated view layer the first one in the evaluated scene (if it exists). This is for + * legacy sake, as this used to remove all other view layers, automatically making the evaluated + * one the first. Some other code may still assume it is. */ if (view_layer_eval != nullptr) { - view_layer_eval->prev = view_layer_eval->next = nullptr; + BLI_listbase_swaplinks(&scene_cow->view_layers, scene_cow->view_layers.first, view_layer_eval); } - scene_cow->view_layers.first = view_layer_eval; - scene_cow->view_layers.last = view_layer_eval; } void scene_remove_all_bases(Scene *scene_cow) @@ -467,7 +473,7 @@ void scene_setup_view_layers_before_remap(const Depsgraph *depsgraph, const IDNode *id_node, Scene *scene_cow) { - scene_remove_unused_view_layers(depsgraph, id_node, scene_cow); + scene_minimize_unused_view_layers(depsgraph, id_node, scene_cow); /* If dependency graph is used for post-processing we don't need any bases and can free of them. * Do it before re-mapping to make that process faster. */ if (depsgraph->is_render_pipeline_depsgraph) { -- 2.30.2 From e5a52593718f916671b6786d825c516843f1acac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 17 Apr 2023 12:45:23 +0200 Subject: [PATCH 2/2] Feedback from Sergey --- source/blender/blenkernel/BKE_layer.h | 6 ++++ source/blender/blenkernel/intern/layer.cc | 31 +++++++++++-------- .../intern/eval/deg_eval_copy_on_write.cc | 16 ++-------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/source/blender/blenkernel/BKE_layer.h b/source/blender/blenkernel/BKE_layer.h index 27dbea51cda..27b454d4d75 100644 --- a/source/blender/blenkernel/BKE_layer.h +++ b/source/blender/blenkernel/BKE_layer.h @@ -75,6 +75,12 @@ void BKE_view_layer_free(struct ViewLayer *view_layer); */ void BKE_view_layer_free_ex(struct ViewLayer *view_layer, bool do_id_user); +/** + * Free the bases of this #ViewLayer, and what they reference. + * This includes baseact, object_bases, object_bases_hash, and layer_collections. + */ +void BKE_view_layer_free_object_content(struct ViewLayer *view_layer); + /** * Tag all the selected objects of a render-layer. */ diff --git a/source/blender/blenkernel/intern/layer.cc b/source/blender/blenkernel/intern/layer.cc index abd639c8213..69b4997c6e9 100644 --- a/source/blender/blenkernel/intern/layer.cc +++ b/source/blender/blenkernel/intern/layer.cc @@ -246,19 +246,7 @@ void BKE_view_layer_free(ViewLayer *view_layer) void BKE_view_layer_free_ex(ViewLayer *view_layer, const bool do_id_user) { - view_layer->basact = nullptr; - - BLI_freelistN(&view_layer->object_bases); - - if (view_layer->object_bases_hash) { - BLI_ghash_free(view_layer->object_bases_hash, nullptr, nullptr); - } - - LISTBASE_FOREACH_MUTABLE (LayerCollection *, lc, &view_layer->layer_collections) { - layer_collection_free(view_layer, lc); - MEM_freeN(lc); - } - BLI_listbase_clear(&view_layer->layer_collections); + BKE_view_layer_free_object_content(view_layer); LISTBASE_FOREACH (ViewLayerEngineData *, sled, &view_layer->drawdata) { if (sled->storage) { @@ -287,6 +275,23 @@ void BKE_view_layer_free_ex(ViewLayer *view_layer, const bool do_id_user) MEM_freeN(view_layer); } +void BKE_view_layer_free_object_content(ViewLayer *view_layer) +{ + view_layer->basact = nullptr; + + BLI_freelistN(&view_layer->object_bases); + + if (view_layer->object_bases_hash) { + BLI_ghash_free(view_layer->object_bases_hash, nullptr, nullptr); + } + + LISTBASE_FOREACH_MUTABLE (LayerCollection *, lc, &view_layer->layer_collections) { + layer_collection_free(view_layer, lc); + MEM_freeN(lc); + } + BLI_listbase_clear(&view_layer->layer_collections); +} + void BKE_view_layer_selected_objects_tag(const Scene *scene, ViewLayer *view_layer, const int tag) { BKE_view_layer_synced_ensure(scene, view_layer); diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc index bae10c21166..6675fd150d0 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc @@ -353,18 +353,6 @@ ViewLayer *get_original_view_layer(const Depsgraph *depsgraph, const IDNode *id_ return nullptr; } -/** - * Clear all bases from the view layer. - * - * This is used when the view layer needs to stick around, for example when it's - * used in the compositor or when it has drivers, but the objects in that view - * layer are irrelevant. - */ -static void view_layer_unassign_bases(ViewLayer *view_layer) -{ - view_layer->basact = nullptr; -} - /* Remove all bases from all view layers except the input one. */ void scene_minimize_unused_view_layers(const Depsgraph *depsgraph, const IDNode *id_node, @@ -381,7 +369,7 @@ void scene_minimize_unused_view_layers(const Depsgraph *depsgraph, * NOTE: Need to keep view layers for all scenes, even indirect ones. This is because of * render layer node possibly pointing to another scene. */ LISTBASE_FOREACH (ViewLayer *, view_layer, &scene_cow->view_layers) { - view_layer_unassign_bases(view_layer); + BKE_view_layer_free_object_content(view_layer); } return; } @@ -399,7 +387,7 @@ void scene_minimize_unused_view_layers(const Depsgraph *depsgraph, view_layer_eval = view_layer_cow; } else { - view_layer_unassign_bases(view_layer_cow); + BKE_view_layer_free_object_content(view_layer_cow); } } -- 2.30.2