From 8b7293eb4168908945d562fdd1c3f8cd4d4944e5 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 24 Aug 2022 17:07:43 +0200 Subject: LibOverride: Fix (unreported) crashes in some cases, preserve active object on Clear, general cleanup. Inconsistencies in update/tagging code between different code doing the same 'Clear. liboverride operation lead to crashes in some cases. Unify deg tagging and WM notifiers accross the three editor-level codepaths performing the common Make/Reset/Clear operations. Preserve if possible the active object accross Clear operation. Several cleanup/rename/re-arangement of code to make it more consistent. --- source/blender/editors/interface/interface_ops.c | 63 +++++++--- .../editors/interface/interface_templates.c | 14 +-- source/blender/editors/object/object_relations.c | 34 +++++- .../editors/space_outliner/outliner_tools.cc | 135 +++++++++++---------- 4 files changed, 155 insertions(+), 91 deletions(-) (limited to 'source/blender') diff --git a/source/blender/editors/interface/interface_ops.c b/source/blender/editors/interface/interface_ops.c index e170cb46925..3a5ba9acaac 100644 --- a/source/blender/editors/interface/interface_ops.c +++ b/source/blender/editors/interface/interface_ops.c @@ -820,19 +820,26 @@ static int override_idtemplate_make_exec(bContext *C, wmOperator *UNUSED(op)) return OPERATOR_CANCELLED; } - if (ID_IS_OVERRIDE_LIBRARY_REAL(owner_id)) { - PointerRNA idptr; - /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it - * to ensure remapping of the owner property from the linked data to the newly created - * liboverride (note that in theory this remapping has already been done by code above), but - * only in case owner ID was already an existing liboverride. - * - * Otherwise, owner ID will also have been overridden, and remapped already to use it's - * override of the data too. */ + PointerRNA idptr; + /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it + * to ensure remapping of the owner property from the linked data to the newly created + * liboverride (note that in theory this remapping has already been done by code above), but + * only in case owner ID was already local ID (override or pure local data). + * + * Otherwise, owner ID will also have been overridden, and remapped already to use it's + * override of the data too. */ + if (!ID_IS_LINKED(owner_id)) { RNA_id_pointer_create(id_override, &idptr); RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL); - RNA_property_update(C, &owner_ptr, prop); } + RNA_property_update(C, &owner_ptr, prop); + + /* 'Security' extra tagging, since this process may also affect the owner ID and not only the + * used ID, relying on the property update code only is not always enough. */ + DEG_id_tag_update(&CTX_data_scene(C)->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE); + WM_event_add_notifier(C, NC_WINDOW, NULL); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL); + WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL); return OPERATOR_FINISHED; } @@ -881,6 +888,9 @@ static int override_idtemplate_reset_exec(bContext *C, wmOperator *UNUSED(op)) RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL); RNA_property_update(C, &owner_ptr, prop); + /* No need for 'security' extra tagging here, since this process will never affect the owner ID. + */ + return OPERATOR_FINISHED; } @@ -919,24 +929,45 @@ static int override_idtemplate_clear_exec(bContext *C, wmOperator *UNUSED(op)) } Main *bmain = CTX_data_main(C); + ViewLayer *view_layer = CTX_data_view_layer(C); + Scene *scene = CTX_data_scene(C); ID *id_new = id; + if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) { id_new = id->override_library->reference; + bool do_remap_active = false; + if (OBACT(view_layer) == (Object *)id) { + BLI_assert(GS(id->name) == ID_OB); + BLI_assert(GS(id_new->name) == ID_OB); + do_remap_active = true; + } BKE_libblock_remap(bmain, id, id_new, ID_REMAP_SKIP_INDIRECT_USAGE); + if (do_remap_active) { + Object *ref_object = (Object *)id_new; + Base *basact = BKE_view_layer_base_find(view_layer, ref_object); + if (basact != NULL) { + view_layer->basact = basact; + } + DEG_id_tag_update(&scene->id, ID_RECALC_SELECT); + } BKE_id_delete(bmain, id); } else { BKE_lib_override_library_id_reset(bmain, id, true); } - PointerRNA idptr; - /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it to - * ensure remapping of the owner property from the linked data to the newly created liboverride - * (note that in theory this remapping has already been done by code above). */ - RNA_id_pointer_create(id_new, &idptr); - RNA_property_pointer_set(&owner_ptr, prop, idptr, NULL); + /* Here the affected ID may remain the same, or be replaced by its linked reference. In either + * case, the owner ID remains unchanged, and remapping is already handled by internal code, so + * calling `RNA_property_update` on it is enough to ensure proper notifiers are sent. */ RNA_property_update(C, &owner_ptr, prop); + /* 'Security' extra tagging, since this process may also affect the owner ID and not only the + * used ID, relying on the property update code only is not always enough. */ + DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE); + WM_event_add_notifier(C, NC_WINDOW, NULL); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL); + WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL); + return OPERATOR_FINISHED; } diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c index 95952b6e8c8..7d27af7220e 100644 --- a/source/blender/editors/interface/interface_templates.c +++ b/source/blender/editors/interface/interface_templates.c @@ -885,14 +885,14 @@ static void template_id_liboverride_hierarchy_make(bContext *C, C, bmain, owner_id, id, r_undo_push_label); if (id_override != NULL) { - /* Given `idptr` is re-assigned to owner property by caller to ensure proper updates etc. Here - * we also use it to ensure remapping of the owner property from the linked data to the newly - * created liboverride (note that in theory this remapping has already been done by code - * above), but only in case owner ID was already an existing liboverride. + /* `idptr` is re-assigned to owner property to ensure proper updates etc. Here we also use it + * to ensure remapping of the owner property from the linked data to the newly created + * liboverride (note that in theory this remapping has already been done by code above), but + * only in case owner ID was already local ID (override or pure local data). * - * Otherwise, owner ID will also have been overridden, and remapped already to use - * it's override of the data too. */ - if (ID_IS_OVERRIDE_LIBRARY_REAL(owner_id)) { + * Otherwise, owner ID will also have been overridden, and remapped already to use it's + * override of the data too. */ + if (!ID_IS_LINKED(owner_id)) { RNA_id_pointer_create(id_override, idptr); } } diff --git a/source/blender/editors/object/object_relations.c b/source/blender/editors/object/object_relations.c index 972912e8863..71dfaefe6a3 100644 --- a/source/blender/editors/object/object_relations.c +++ b/source/blender/editors/object/object_relations.c @@ -2420,6 +2420,8 @@ static int make_override_library_exec(bContext *C, wmOperator *op) DEG_id_tag_update(&CTX_data_scene(C)->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE); WM_event_add_notifier(C, NC_WINDOW, NULL); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL); + WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL); return success ? OPERATOR_FINISHED : OPERATOR_CANCELLED; } @@ -2546,7 +2548,8 @@ static int reset_override_library_exec(bContext *C, wmOperator *UNUSED(op)) } FOREACH_SELECTED_OBJECT_END; - WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, NULL); + WM_event_add_notifier(C, NC_WINDOW, NULL); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL); WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL); return OPERATOR_FINISHED; @@ -2570,26 +2573,49 @@ void OBJECT_OT_reset_override_library(wmOperatorType *ot) static int clear_override_library_exec(bContext *C, wmOperator *UNUSED(op)) { Main *bmain = CTX_data_main(C); + ViewLayer *view_layer = CTX_data_view_layer(C); + Scene *scene = CTX_data_scene(C); + LinkNode *todo_objects = NULL, *todo_object_iter; /* Make already existing selected liboverrides editable. */ - FOREACH_SELECTED_OBJECT_BEGIN (CTX_data_view_layer(C), CTX_wm_view3d(C), ob_iter) { + FOREACH_SELECTED_OBJECT_BEGIN (view_layer, CTX_wm_view3d(C), ob_iter) { if (ID_IS_LINKED(ob_iter)) { continue; } + BLI_linklist_prepend_alloca(&todo_objects, ob_iter); + } + FOREACH_SELECTED_OBJECT_END; + + for (todo_object_iter = todo_objects; todo_object_iter != NULL; + todo_object_iter = todo_object_iter->next) { + Object *ob_iter = todo_object_iter->link; if (BKE_lib_override_library_is_hierarchy_leaf(bmain, &ob_iter->id)) { + bool do_remap_active = false; + if (OBACT(view_layer) == ob_iter) { + do_remap_active = true; + } BKE_libblock_remap(bmain, &ob_iter->id, ob_iter->id.override_library->reference, ID_REMAP_SKIP_INDIRECT_USAGE); + if (do_remap_active) { + Object *ref_object = ob_iter->id.override_library->reference; + Base *basact = BKE_view_layer_base_find(view_layer, ref_object); + if (basact != NULL) { + view_layer->basact = basact; + } + DEG_id_tag_update(&scene->id, ID_RECALC_SELECT); + } BKE_id_delete(bmain, &ob_iter->id); } else { BKE_lib_override_library_id_reset(bmain, &ob_iter->id, true); } } - FOREACH_SELECTED_OBJECT_END; - WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, NULL); + DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE); + WM_event_add_notifier(C, NC_WINDOW, NULL); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, NULL); WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, NULL); return OPERATOR_FINISHED; diff --git a/source/blender/editors/space_outliner/outliner_tools.cc b/source/blender/editors/space_outliner/outliner_tools.cc index e5c417daa0d..d6305c836ff 100644 --- a/source/blender/editors/space_outliner/outliner_tools.cc +++ b/source/blender/editors/space_outliner/outliner_tools.cc @@ -1274,22 +1274,68 @@ static void id_override_library_reset_fn(bContext *C, OutlinerLibOverrideData *data = reinterpret_cast(user_data); const bool do_hierarchy = data->do_hierarchy; - if (ID_IS_OVERRIDE_LIBRARY_REAL(id_root)) { - Main *bmain = CTX_data_main(C); + if (!ID_IS_OVERRIDE_LIBRARY_REAL(id_root)) { + CLOG_WARN(&LOG, "Could not reset library override of data block '%s'", id_root->name); + return; + } - if (do_hierarchy) { - BKE_lib_override_library_id_hierarchy_reset(bmain, id_root, false); + Main *bmain = CTX_data_main(C); + + if (do_hierarchy) { + BKE_lib_override_library_id_hierarchy_reset(bmain, id_root, false); + } + else { + BKE_lib_override_library_id_reset(bmain, id_root, false); + } +} + +static void id_override_library_clear_single_fn(bContext *C, + ReportList *reports, + Scene *scene, + TreeElement *UNUSED(te), + TreeStoreElem *UNUSED(tsep), + TreeStoreElem *tselem, + void *UNUSED(user_data)) +{ + BLI_assert(TSE_IS_REAL_ID(tselem)); + Main *bmain = CTX_data_main(C); + ViewLayer *view_layer = CTX_data_view_layer(C); + ID *id = tselem->id; + + if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) { + BKE_reportf(reports, + RPT_WARNING, + "Cannot clear embedded library override id '%s', only overrides of real " + "data-blocks can be directly deleted", + id->name); + return; + } + + /* If given ID is not using any other override (it's a 'leaf' in the override hierarchy), + * delete it and remap its usages to its linked reference. Otherwise, keep it as a reset system + * override. */ + if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) { + bool do_remap_active = false; + if (OBACT(view_layer) == reinterpret_cast(id)) { + BLI_assert(GS(id->name) == ID_OB); + do_remap_active = true; } - else { - BKE_lib_override_library_id_reset(bmain, id_root, false); + BKE_libblock_remap(bmain, id, id->override_library->reference, ID_REMAP_SKIP_INDIRECT_USAGE); + if (do_remap_active) { + Object *ref_object = reinterpret_cast(id->override_library->reference); + Base *basact = BKE_view_layer_base_find(view_layer, ref_object); + if (basact != nullptr) { + view_layer->basact = basact; + } + DEG_id_tag_update(&scene->id, ID_RECALC_SELECT); } - - WM_event_add_notifier(C, NC_WM | ND_DATACHANGED, nullptr); - WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, nullptr); + BKE_id_delete(bmain, id); } else { - CLOG_WARN(&LOG, "Could not reset library override of data block '%s'", id_root->name); + BKE_lib_override_library_id_reset(bmain, id, true); } + + DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS | ID_RECALC_COPY_ON_WRITE); } static void id_override_library_resync_fn(bContext *UNUSED(C), @@ -1340,13 +1386,13 @@ static void id_override_library_resync_hierarchy_process(bContext *C, WM_event_add_notifier(C, NC_WINDOW, nullptr); } -static void id_override_library_clear_hierarchy_fn(bContext *UNUSED(C), - ReportList *UNUSED(reports), - Scene *UNUSED(scene), - TreeElement *UNUSED(te), - TreeStoreElem *UNUSED(tsep), - TreeStoreElem *tselem, - void *user_data) +static void id_override_library_delete_hierarchy_fn(bContext *UNUSED(C), + ReportList *UNUSED(reports), + Scene *UNUSED(scene), + TreeElement *UNUSED(te), + TreeStoreElem *UNUSED(tsep), + TreeStoreElem *tselem, + void *user_data) { OutlinerLibOverrideData *data = reinterpret_cast(user_data); @@ -1366,52 +1412,15 @@ static void id_override_library_clear_hierarchy_fn(bContext *UNUSED(C), } /* Clear (delete) a hierarchy of library overrides. */ -static void id_override_library_clear_hierarchy_process(bContext *C, - ReportList *UNUSED(reports), - OutlinerLibOverrideData &data) +static void id_override_library_delete_hierarchy_process(bContext *C, + ReportList *UNUSED(reports), + OutlinerLibOverrideData &data) { Main *bmain = CTX_data_main(C); for (auto &&id_hierarchy_root : data.id_hierarchy_roots.keys()) { BKE_lib_override_library_delete(bmain, id_hierarchy_root); } - - WM_event_add_notifier(C, NC_WINDOW, nullptr); -} - -static void id_override_library_clear_single_fn(bContext *C, - ReportList *reports, - Scene *UNUSED(scene), - TreeElement *UNUSED(te), - TreeStoreElem *UNUSED(tsep), - TreeStoreElem *tselem, - void *UNUSED(user_data)) -{ - BLI_assert(TSE_IS_REAL_ID(tselem)); - Main *bmain = CTX_data_main(C); - ID *id = tselem->id; - - if (!ID_IS_OVERRIDE_LIBRARY_REAL(id)) { - BKE_reportf(reports, - RPT_WARNING, - "Cannot clear embedded library override id '%s', only overrides of real " - "data-blocks can be directly deleted", - id->name); - return; - } - - /* If given ID is not using any other override (it's a 'leaf' in the override hierarchy), - * delete it and remap its usages to its linked reference. Otherwise, keep it as a reset system - * override. */ - if (BKE_lib_override_library_is_hierarchy_leaf(bmain, id)) { - BKE_libblock_remap(bmain, id, id->override_library->reference, ID_REMAP_SKIP_INDIRECT_USAGE); - BKE_id_delete(bmain, id); - } - else { - BKE_lib_override_library_id_reset(bmain, id, true); - } - - WM_event_add_notifier(C, NC_WINDOW, nullptr); } static void id_fake_user_set_fn(bContext *UNUSED(C), @@ -1785,11 +1794,11 @@ static int outliner_liboverride_operation_exec(bContext *C, wmOperator *op) op->reports, scene, space_outliner, - id_override_library_clear_hierarchy_fn, + id_override_library_delete_hierarchy_fn, OUTLINER_LIB_SELECTIONSET_SELECTED, nullptr); - id_override_library_clear_hierarchy_process(C, op->reports, override_data); + id_override_library_delete_hierarchy_process(C, op->reports, override_data); ED_undo_push(C, "Delete Overridden Data Hierarchy"); break; @@ -1799,11 +1808,9 @@ static int outliner_liboverride_operation_exec(bContext *C, wmOperator *op) break; } - /* wrong notifier still... */ - WM_event_add_notifier(C, NC_ID | NA_EDITED, nullptr); - - /* XXX: this is just so that outliner is always up to date. */ - WM_event_add_notifier(C, NC_SPACE | ND_SPACE_OUTLINER, nullptr); + WM_event_add_notifier(C, NC_WINDOW, nullptr); + WM_event_add_notifier(C, NC_WM | ND_LIB_OVERRIDE_CHANGED, nullptr); + WM_event_add_notifier(C, NC_SPACE | ND_SPACE_VIEW3D, nullptr); return OPERATOR_FINISHED; } -- cgit v1.2.3