"Fix" crash when deleting linked object which has indirect usages.
authorBastien Montagne <montagne29@wanadoo.fr>
Fri, 1 Jul 2016 15:51:08 +0000 (17:51 +0200)
committerBastien Montagne <montagne29@wanadoo.fr>
Fri, 1 Jul 2016 16:29:12 +0000 (18:29 +0200)
This is in fact very hairy situation here... Objects are only refcounted by scenes,
any other usage is 'free', which means once all object instanciations are gone Blender
considers it can delete it.

There is a trap here though: indirect usages. Typically, we should never modify linked data
(because it is essencially useless, changes would be ignored and ost on next reload or
even undo/redo). This means indirect usages are not affected by default 'safe' remapping/unlinking.

For unlinking preceeding deletion however, this is not acceptable - we are likely to end with
a zero-user ID (aka deletable one) which is still actually used by other linked data.

Solution choosen here is double:
I) From 'user-space' (i.e. outliner, operators...), we check for cases where deleting datablocks
should not be allowed (indirect data or indirectly used data), and abort (with report) if needed.
II) From 'lower' level (BKE_library_remap and RNA), we also unlink from linked data,
which makes actual deletion possible and safe.

Note that with previous behavior (2.77 one), linked object would be deleted, including from linked data -
but then, once file is saved and reloaded, indirect usage would link back the deleted object,
without any instanciation in scene, which made it somehow virtual and unreachable...

With new behavior, this is no more possible, but on the other hand it means that in situations of dependency cycles
(two linked objects using each other), linked objects become impossible to delete (from user space).

Not sure what's best here, behavior with those corner cases of library linking is very poorly defined... :(

source/blender/blenkernel/BKE_library_query.h
source/blender/blenkernel/BKE_library_remap.h
source/blender/blenkernel/intern/library_query.c
source/blender/blenkernel/intern/library_remap.c
source/blender/editors/object/object_add.c
source/blender/editors/object/object_group.c
source/blender/editors/space_outliner/outliner_edit.c
source/blender/editors/space_outliner/outliner_tools.c
source/blender/render/intern/source/pipeline.c

index c89dce99caae1a5ec4e31527b8916235ad6bb8bd..3a2b49f911c9ca20b94ffb1e99e5b5d26958389e 100644 (file)
@@ -53,7 +53,7 @@ enum {
 
 enum {
        IDWALK_RET_NOP            = 0,
-       IDWALK_RET_STOP_ITER      = 1 << 0,  /* Completly top iteration. */
+       IDWALK_RET_STOP_ITER      = 1 << 0,  /* Completly stop iteration. */
        IDWALK_RET_STOP_RECURSION = 1 << 1,  /* Stop recursion, that is, do not loop over ID used by current one. */
 };
 
@@ -76,4 +76,6 @@ void BKE_library_update_ID_link_user(struct ID *id_dst, struct ID *id_src, const
 
 int BKE_library_ID_use_ID(struct ID *id_user, struct ID *id_used);
 
+bool BKE_library_ID_is_indirectly_used(struct Main *bmain, void *idv);
+
 #endif  /* __BKE_LIBRARY_QUERY_H__ */
index 23d080e072cc6afd157ce41bedb1da76bb4efe19..754005276f055f1131295ffc95375ce88b458948 100644 (file)
@@ -56,7 +56,9 @@ void BKE_libblock_remap(
         struct Main *bmain, void *old_idv, void *new_idv,
         const short remap_flags) ATTR_NONNULL(1, 2);
 
-void BKE_libblock_unlink(struct Main *bmain, void *idv, const bool do_flag_never_null) ATTR_NONNULL();
+void BKE_libblock_unlink(
+        struct Main *bmain, void *idv,
+        const bool do_flag_never_null, const bool do_skip_indirect) ATTR_NONNULL();
 
 void BKE_libblock_relink_ex(void *idv, void *old_idv, void *new_idv, const bool us_min_never_null) ATTR_NONNULL(1);
 
index be3dde2753a9c857cf64826ce3854f1262eb0ae9..8373ad0123fca3ae18d1dc555a8c67fdf1be8ded 100644 (file)
@@ -865,3 +865,53 @@ int BKE_library_ID_use_ID(ID *id_user, ID *id_used)
 
        return iter.count;
 }
+
+
+static int foreach_libblock_check_indirect_usage_callback(
+        void *user_data, ID *UNUSED(id_self), ID **id_p, int UNUSED(cb_flag))
+{
+       IDUsersIter *iter = user_data;
+
+       if (*id_p && (*id_p == iter->id)) {
+               iter->count++;
+               return IDWALK_RET_STOP_ITER;
+       }
+
+       return IDWALK_RET_NOP;
+}
+
+/**
+ * Check wether given ID is used indirectly (i.e. by another linked ID).
+ */
+bool BKE_library_ID_is_indirectly_used(Main *bmain, void *idv)
+{
+       IDUsersIter iter;
+       ListBase *lb_array[MAX_LIBARRAY];
+       int i = set_listbasepointers(bmain, lb_array);
+
+       iter.id = idv;
+       iter.count = 0;
+       while (i--) {
+               ID *id_curr = lb_array[i]->first;
+
+               for (; id_curr; id_curr = id_curr->next) {
+                       if (!id_curr->lib) {
+                               continue;
+                       }
+
+                       iter.curr_id = id_curr;
+                       BKE_library_foreach_ID_link(
+                                   id_curr, foreach_libblock_check_indirect_usage_callback, &iter, IDWALK_NOP);
+
+                       if (iter.count) {
+                               break;
+                       }
+               }
+               if (iter.count) {
+                       break;
+               }
+       }
+
+       return (iter.count != 0);
+}
+
index 10dd123a121254e7292a34e2d64341c5ce68eb16..81543fad0fe4785038f632844e88091859926d45 100644 (file)
@@ -175,6 +175,11 @@ static int foreach_libblock_remap_callback(void *user_data, ID *UNUSED(id_self),
                                            (id_remap_data->flag & ID_REMAP_FORCE_NEVER_NULL_USAGE) == 0);
                const bool skip_never_null = (id_remap_data->flag & ID_REMAP_SKIP_NEVER_NULL_USAGE) != 0;
 
+#ifdef DEBUG_PRINT
+               printf("In %s: Remapping %s (%p) to %s (%p) (skip_indirect: %d)\n",
+                      id->name, old_id->name, old_id, new_id ? new_id->name : "<NONE>", new_id, skip_indirect);
+#endif
+
                if ((id_remap_data->flag & ID_REMAP_FLAG_NEVER_NULL_USAGE) && (cb_flag & IDWALK_NEVER_NULL)) {
                        id->tag |= LIB_TAG_DOIT;
                }
@@ -184,11 +189,14 @@ static int foreach_libblock_remap_callback(void *user_data, ID *UNUSED(id_self),
                    (is_obj_editmode && (((Object *)id)->data == *id_p)) ||
                    (skip_indirect && (is_proxy || is_indirect)))
                {
-                       if (!is_indirect && (is_never_null || is_proxy || is_obj_editmode)) {
+                       if (is_indirect) {
+                               id_remap_data->skipped_indirect++;
+                       }
+                       else if (is_never_null || is_proxy || is_obj_editmode) {
                                id_remap_data->skipped_direct++;
                        }
                        else {
-                               id_remap_data->skipped_indirect++;
+                               BLI_assert(0);
                        }
                        if (cb_flag & IDWALK_USER) {
                                id_remap_data->skipped_refcounted++;
@@ -461,9 +469,10 @@ void BKE_libblock_remap(Main *bmain, void *old_idv, void *new_idv, const short r
  * \param do_flag_never_null: If true, all IDs using \a idv in a 'non-NULL' way are flagged by \a LIB_TAG_DOIT flag
  * (quite obviously, 'non-NULL' usages can never be unlinked by this function...).
  */
-void BKE_libblock_unlink(Main *bmain, void *idv, const bool do_flag_never_null)
+void BKE_libblock_unlink(Main *bmain, void *idv, const bool do_flag_never_null, const bool do_skip_indirect)
 {
-       const short remap_flags = ID_REMAP_SKIP_INDIRECT_USAGE | (do_flag_never_null ? ID_REMAP_FLAG_NEVER_NULL_USAGE : 0);
+       const short remap_flags = (do_skip_indirect ? ID_REMAP_SKIP_INDIRECT_USAGE : 0) |
+                                 (do_flag_never_null ? ID_REMAP_FLAG_NEVER_NULL_USAGE : 0);
 
        BKE_main_lock(bmain);
 
@@ -709,7 +718,7 @@ void BKE_libblock_free_us(Main *bmain, void *idv)      /* test users */
        }
 
        if (id->us == 0) {
-               BKE_libblock_unlink(bmain, id, false);
+               BKE_libblock_unlink(bmain, id, false, false);
                
                BKE_libblock_free(bmain, id);
        }
index f85c76291cda6ca013890642e846bb1d79307f0b..0f1c72fcd55fc71f7e75f8510f5b78ea08a36b08 100644 (file)
@@ -73,6 +73,7 @@
 #include "BKE_lamp.h"
 #include "BKE_lattice.h"
 #include "BKE_library.h"
+#include "BKE_library_query.h"
 #include "BKE_key.h"
 #include "BKE_main.h"
 #include "BKE_material.h"
@@ -1108,11 +1109,18 @@ static void object_delete_check_glsl_update(Object *ob)
 /* note: now unlinks constraints as well */
 void ED_base_object_free_and_unlink(Main *bmain, Scene *scene, Base *base)
 {
-       DAG_id_type_tag(bmain, ID_OB);
+       if (BKE_library_ID_is_indirectly_used(bmain, base->object) && ID_REAL_USERS(base->object) <= 1) {
+               /* We cannot delete indirectly used object... */
+               printf("WARNING, undeletable object '%s', should have been catched before reaching this function!",
+                      base->object->id.name + 2);
+               return;
+       }
+
        BKE_scene_base_unlink(scene, base);
        object_delete_check_glsl_update(base->object);
        BKE_libblock_free_us(bmain, base->object);
        MEM_freeN(base);
+       DAG_id_type_tag(bmain, ID_OB);
 }
 
 static int object_delete_exec(bContext *C, wmOperator *op)
@@ -1129,6 +1137,19 @@ static int object_delete_exec(bContext *C, wmOperator *op)
 
        CTX_DATA_BEGIN (C, Base *, base, selected_bases)
        {
+               const bool is_indirectly_used = BKE_library_ID_is_indirectly_used(bmain, base->object);
+               if (base->object->id.tag & LIB_TAG_INDIRECT) {
+                       /* Can this case ever happen? */
+                       BKE_reportf(op->reports, RPT_WARNING, "Cannot delete indirectly linked object '%s'", base->object->id.name + 2);
+                       continue;
+               }
+               else if (is_indirectly_used && ID_REAL_USERS(base->object) <= 1) {
+                       BKE_reportf(op->reports, RPT_WARNING,
+                                   "Cannot delete object '%s' from scene '%s', indirectly used objects need at least one user",
+                                   base->object->id.name + 2, scene->id.name + 2);
+                       continue;
+               }
+
                /* deselect object -- it could be used in other scenes */
                base->object->flag &= ~SELECT;
 
@@ -1144,6 +1165,12 @@ static int object_delete_exec(bContext *C, wmOperator *op)
                                if (scene_iter != scene && !(scene_iter->id.lib)) {
                                        base_other = BKE_scene_base_find(scene_iter, base->object);
                                        if (base_other) {
+                                               if (is_indirectly_used && ID_REAL_USERS(base->object) <= 1) {
+                                                       BKE_reportf(op->reports, RPT_WARNING,
+                                                                   "Cannot delete object '%s' from scene '%s', indirectly used objects need at least one user",
+                                                                   base->object->id.name + 2, scene_iter->id.name + 2);
+                                                       break;
+                                               }
                                                ED_base_object_free_and_unlink(bmain, scene_iter, base_other);
                                        }
                                }
index 2b87a890f0f8b684ad59e5a1b2f7e231f6870a20..bcdd170c53c017e819781314de4454c84f5251a2 100644 (file)
@@ -528,7 +528,7 @@ static int group_unlink_exec(bContext *C, wmOperator *UNUSED(op))
        if (!group)
                return OPERATOR_CANCELLED;
 
-       BKE_libblock_unlink(bmain, group, false);
+       BKE_libblock_unlink(bmain, group, false, false);
        BKE_libblock_free(bmain, group);
 
        WM_event_add_notifier(C, NC_OBJECT | ND_DRAW, NULL);
index 45c45ffe39cb23c08a9825a7c97914c1063b31de..7990c169fcecf116aef3fb48739cddc870940720 100644 (file)
@@ -55,6 +55,7 @@
 #include "BKE_global.h"
 #include "BKE_idcode.h"
 #include "BKE_library.h"
+#include "BKE_library_query.h"
 #include "BKE_library_remap.h"
 #include "BKE_main.h"
 #include "BKE_outliner_treehash.h"
@@ -307,7 +308,7 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot)
 
 /* ID delete --------------------------------------------------- */
 
-static void id_delete(bContext *C, TreeElement *te, TreeStoreElem *tselem)
+static void id_delete(bContext *C, ReportList *reports, TreeElement *te, TreeStoreElem *tselem)
 {
        Main *bmain = CTX_data_main(C);
        ID *id = tselem->id;
@@ -316,16 +317,28 @@ static void id_delete(bContext *C, TreeElement *te, TreeStoreElem *tselem)
        BLI_assert(te->idcode != ID_LI || ((Library *)id)->parent == NULL);
        UNUSED_VARS_NDEBUG(te);
 
+       if (id->tag & LIB_TAG_INDIRECT) {
+               BKE_reportf(reports, RPT_WARNING, "Cannot delete indirectly linked id '%s'", id->name);
+               return;
+       }
+       else if (BKE_library_ID_is_indirectly_used(bmain, id) && ID_REAL_USERS(id) <= 1) {
+               BKE_reportf(reports, RPT_WARNING,
+                           "Cannot delete id '%s', indirectly used datablocks need at least one user",
+                           id->name);
+               return;
+       }
+
+
        BKE_libblock_delete(bmain, id);
 
        WM_event_add_notifier(C, NC_WINDOW, NULL);
 }
 
 void id_delete_cb(
-        bContext *C, ReportList *UNUSED(reports), Scene *UNUSED(scene),
+        bContext *C, ReportList *reports, Scene *UNUSED(scene),
         TreeElement *te, TreeStoreElem *UNUSED(tsep), TreeStoreElem *tselem, void *UNUSED(user_data))
 {
-       id_delete(C, te, tselem);
+       id_delete(C, reports, te, tselem);
 }
 
 static int outliner_id_delete_invoke_do(bContext *C, ReportList *reports, TreeElement *te, const float mval[2])
@@ -339,7 +352,7 @@ static int outliner_id_delete_invoke_do(bContext *C, ReportList *reports, TreeEl
                                            "Cannot delete indirectly linked library '%s'", ((Library *)tselem->id)->filepath);
                                return OPERATOR_CANCELLED;
                        }
-                       id_delete(C, te, tselem);
+                       id_delete(C, reports, te, tselem);
                        return OPERATOR_FINISHED;
                }
        }
index d92fec4c90df272b98d89f0e0a5d507eb5f042a5..bfec62997e14507578ee4688b343602709097a51 100644 (file)
@@ -57,6 +57,7 @@
 #include "BKE_fcurve.h"
 #include "BKE_group.h"
 #include "BKE_library.h"
+#include "BKE_library_query.h"
 #include "BKE_library_remap.h"
 #include "BKE_main.h"
 #include "BKE_report.h"
@@ -397,7 +398,7 @@ static void object_deselect_cb(
 }
 
 static void object_delete_cb(
-        bContext *C, ReportList *UNUSED(reports), Scene *scene, TreeElement *te,
+        bContext *C, ReportList *reports, Scene *scene, TreeElement *te,
         TreeStoreElem *UNUSED(tsep), TreeStoreElem *tselem, void *UNUSED(user_data))
 {
        Base *base = (Base *)te->directdata;
@@ -405,6 +406,18 @@ static void object_delete_cb(
        if (base == NULL)
                base = BKE_scene_base_find(scene, (Object *)tselem->id);
        if (base) {
+               Main *bmain = CTX_data_main(C);
+               if (base->object->id.tag & LIB_TAG_INDIRECT) {
+                       BKE_reportf(reports, RPT_WARNING, "Cannot delete indirectly linked object '%s'", base->object->id.name + 2);
+                       return;
+               }
+               else if (BKE_library_ID_is_indirectly_used(bmain, base->object) && ID_REAL_USERS(base->object) <= 1) {
+                       BKE_reportf(reports, RPT_WARNING,
+                                   "Cannot delete object '%s' from scene '%s', indirectly used objects need at least one user",
+                                   base->object->id.name + 2, scene->id.name + 2);
+                       return;
+               }
+
                // check also library later
                if (scene->obedit == base->object)
                        ED_object_editmode_exit(C, EM_FREEDATA | EM_FREEUNDO | EM_WAITCURSOR | EM_DO_UNDO);
@@ -809,7 +822,7 @@ static void outliner_do_data_operation(SpaceOops *soops, int type, int event, Li
        }
 }
 
-static Base *outline_delete_hierarchy(bContext *C, Scene *scene, Base *base)
+static Base *outline_delete_hierarchy(bContext *C, ReportList *reports, Scene *scene, Base *base)
 {
        Base *child_base, *base_next;
        Object *parent;
@@ -822,17 +835,29 @@ static Base *outline_delete_hierarchy(bContext *C, Scene *scene, Base *base)
                base_next = child_base->next;
                for (parent = child_base->object->parent; parent && (parent != base->object); parent = parent->parent);
                if (parent) {
-                       base_next = outline_delete_hierarchy(C, scene, child_base);
+                       base_next = outline_delete_hierarchy(C, reports, scene, child_base);
                }
        }
 
        base_next = base->next;
+
+       Main *bmain = CTX_data_main(C);
+       if (base->object->id.tag & LIB_TAG_INDIRECT) {
+               BKE_reportf(reports, RPT_WARNING, "Cannot delete indirectly linked object '%s'", base->object->id.name + 2);
+               return base_next;
+       }
+       else if (BKE_library_ID_is_indirectly_used(bmain, base->object) && ID_REAL_USERS(base->object) <= 1) {
+               BKE_reportf(reports, RPT_WARNING,
+                           "Cannot delete object '%s' from scene '%s', indirectly used objects need at least one user",
+                           base->object->id.name + 2, scene->id.name + 2);
+               return base_next;
+       }
        ED_base_object_free_and_unlink(CTX_data_main(C), scene, base);
        return base_next;
 }
 
 static void object_delete_hierarchy_cb(
-        bContext *C, ReportList *UNUSED(reports), Scene *scene,
+        bContext *C, ReportList *reports, Scene *scene,
         TreeElement *te, TreeStoreElem *UNUSED(tsep), TreeStoreElem *tselem, void *UNUSED(user_data))
 {
        Base *base = (Base *)te->directdata;
@@ -848,7 +873,7 @@ static void object_delete_hierarchy_cb(
                        ED_object_editmode_exit(C, EM_FREEDATA | EM_FREEUNDO | EM_WAITCURSOR | EM_DO_UNDO);
                }
 
-               outline_delete_hierarchy(C, scene, base);
+               outline_delete_hierarchy(C, reports, scene, base);
                /* leave for ED_outliner_id_unref to handle */
 #if 0
                te->directdata = NULL;
index 4d711f8664480cac3d35f2edcec52283832e25b6..7ae4687340ba3900725db1a6ca15002d2a46a035 100644 (file)
@@ -2268,7 +2268,7 @@ static void free_all_freestyle_renders(void)
                        if (freestyle_render) {
                                freestyle_scene = freestyle_render->scene;
                                RE_FreeRender(freestyle_render);
-                               BKE_libblock_unlink(re1->freestyle_bmain, freestyle_scene, false);
+                               BKE_libblock_unlink(re1->freestyle_bmain, freestyle_scene, false, false);
                                BKE_libblock_free(re1->freestyle_bmain, freestyle_scene);
                        }
                }