[Bf-blender-cvs] [5f02998] id-remap: Hopefully fix the 'real user' nightmare.
Bastien Montagne
noreply at git.blender.org
Tue Nov 10 20:15:23 CET 2015
Commit: 5f0299898dd7556553bb5be0f2c2279b71cdef86
Author: Bastien Montagne
Date: Tue Nov 10 16:56:54 2015 +0100
Branches: id-remap
https://developer.blender.org/rB5f0299898dd7556553bb5be0f2c2279b71cdef86
Hopefully fix the 'real user' nightmare.
Idea is to add two new flags, one saying 'we need to ensure a real user exists',
the other 'we had to increment user count to ensure we have a real user'.
This allows us to easily control the extra user in release/delete/remap cases,
and also fixes the infamous 'add new image to texture, open image in ImageEditor,
delete image from texture, have a zero-user red image in Image Editor' issue.
There is still much to be done here, more places where we can use those flags,
also clear them when we force usercount to zero, etc.
All this allows us to fix unsolvable issues (like Group being ensure_user'ed
in loading code, but only if they do have objects in them), and to avoid returning
ugly bool from editors' callbacks (this is still to be cleaned up in the branch too).
Bad news - this means we cannot use short ID->flag anymore (not enough flags), for now
added a new int ID->flag2 (replacing pad int), not sure how to best manage change here,
maybe for 2.8 we can totally wipe ID->flag? But this would totally break forward compat.
===================================================================
M source/blender/blenkernel/intern/library.c
M source/blender/blenloader/intern/readfile.c
M source/blender/makesdna/DNA_ID.h
===================================================================
diff --git a/source/blender/blenkernel/intern/library.c b/source/blender/blenkernel/intern/library.c
index ce78084..846add2 100644
--- a/source/blender/blenkernel/intern/library.c
+++ b/source/blender/blenkernel/intern/library.c
@@ -164,12 +164,14 @@ void id_us_ensure_real(ID *id)
{
if (id) {
const int limit = ID_FAKE_USERS(id);
+ id->flag2 |= LIB_EXTRAUSER;
if (id->us <= limit) {
- if (id->us < limit) {
+ if (id->us < limit || ((id->us == limit) && (id->flag2 & LIB_EXTRAUSER_SET))) {
printf("ID user count error: %s (from '%s')\n", id->name, id->lib ? id->lib->filepath : "[Main]");
BLI_assert(0);
}
id->us = limit + 1;
+ id->flag2 |= LIB_EXTRAUSER_SET;
}
}
}
@@ -190,7 +192,13 @@ void id_us_plus(ID *id)
void id_us_min(ID *id)
{
if (id) {
- const int limit = ID_FAKE_USERS(id);
+ const int limit = ID_FAKE_USERS(id);
+
+ if ((id->us == limit) && (id->flag2 & LIB_EXTRAUSER) && !(id->flag2 & LIB_EXTRAUSER_SET)) {
+ /* We need an extra user here, but never actually incremented user count for it so far, do it now. */
+ id_us_ensure_real(id);
+ }
+
if (id->us <= limit) {
printf("ID user decrement error: %s (from '%s')\n", id->name, id->lib ? id->lib->filepath : "[Main]");
BLI_assert(0);
@@ -1028,8 +1036,7 @@ enum {
/* Set by callback. */
ID_REMAP_IS_LINKED_DIRECT = 1 << 16, /* new_id is directly linked in current .blend. */
- ID_REMAP_IS_USER_ONE = 1 << 17, /* old_id was used by some nasty 'user_one' stuff (like image editor). */
- ID_REMAP_IS_USER_ONE_SKIPPED = 1 << 18, /* there was some skipped 'user_one' usages of old_id. */
+ ID_REMAP_IS_USER_ONE_SKIPPED = 1 << 17, /* there was some skipped 'user_one' usages of old_id. */
};
static bool foreach_libblock_remap_callback(void *user_data, ID **id_p, int cb_flag)
@@ -1081,12 +1088,14 @@ static bool foreach_libblock_remap_callback(void *user_data, ID **id_p, int cb_f
*id_p = new_id;
if (cb_flag & IDWALK_USER) {
id_us_min(old_id);
- id_us_plus(new_id); /* XXX Check, do we really want to handle LIB_INDIRECT/LIB_EXTERN here? */
+ /* We do not want to handle LIB_INDIRECT/LIB_EXTERN here. */
+ if (new_id)
+ new_id->us++;
}
else if (cb_flag & IDWALK_USER_ONE) {
id_us_ensure_real(new_id);
- /* We cannot affect old_id->us directly, flag it as such for final handling... */
- id_remap_data->flag |= ID_REMAP_IS_USER_ONE;
+ /* We cannot affect old_id->us directly, LIB_EXTRAUSER(_SET) are assumed to be set as needed,
+ * that extra user is processed in final handling... */
}
if (!is_indirect) {
id_remap_data->flag |= ID_REMAP_IS_LINKED_DIRECT;
@@ -1117,7 +1126,7 @@ static bool foreach_libblock_remap_callback(void *user_data, ID **id_p, int cb_f
* \param r_id_remap_data if non-NULL, the IDRemap struct to use (uselful to retrieve info about remapping process).
* \return true is there was some 'user_one' users of \a old_id (needed to handle correctly #old_id->us count).
*/
-static bool libblock_remap_data(
+static void libblock_remap_data(
Main *bmain, ID *id, ID *old_id, ID *new_id, const bool skip_indirect_usage, IDRemap *r_id_remap_data)
{
IDRemap id_remap_data;
@@ -1163,9 +1172,11 @@ static bool libblock_remap_data(
}
}
+ /* XXX We may not want to always 'transfer' fakeuser from old to new id... Think for now it's desired behavior
+ * though, we can always add an option (flag) to control this later if needed. */
if (old_id && (old_id->flag & LIB_FAKEUSER)) {
- r_id_remap_data->skipped_direct++;
- r_id_remap_data->skipped_refcounted++;
+ id_fake_user_clear(old_id);
+ id_fake_user_set(new_id);
}
if (new_id && (new_id->flag & LIB_INDIRECT) && (r_id_remap_data->flag & ID_REMAP_IS_LINKED_DIRECT)) {
@@ -1176,8 +1187,6 @@ static bool libblock_remap_data(
printf("%s: %d occurences skipped (%d direct and %d indirect ones)\n", __func__,
r_id_remap_data->skipped_direct + r_id_remap_data->skipped_indirect,
r_id_remap_data->skipped_direct, r_id_remap_data->skipped_indirect);
-
- return (r_id_remap_data->flag & ID_REMAP_IS_USER_ONE) != 0;
}
/** Replace all references in .blend file to \a old_id by \a new_id (if \a new_id is NULL, it unlinks \a old_id). */
@@ -1187,7 +1196,6 @@ void BKE_libblock_remap_locked(Main *bmain, void *old_idv, void *new_idv, const
ID *old_id = old_idv;
ID *new_id = new_idv;
int skipped_direct, skipped_refcounted;
- bool is_user_one;
BLI_assert(old_id != NULL);
BLI_assert((new_id == NULL) || GS(old_id->name) == GS(new_id->name));
@@ -1214,7 +1222,7 @@ void BKE_libblock_remap_locked(Main *bmain, void *old_idv, void *new_idv, const
}
}
- is_user_one = libblock_remap_data(bmain, NULL, old_id, new_id, skip_indirect_usage, &id_remap_data);
+ libblock_remap_data(bmain, NULL, old_id, new_id, skip_indirect_usage, &id_remap_data);
if (free_notifier_reference_cb) {
free_notifier_reference_cb(old_id);
@@ -1223,21 +1231,23 @@ void BKE_libblock_remap_locked(Main *bmain, void *old_idv, void *new_idv, const
/* We assume editors do not hold references to their IDs... This is false in some cases
* (Image is especially tricky here), editors' code is to handle refcount (id->us) itself then. */
if (remap_editor_id_reference_cb) {
- is_user_one = is_user_one || remap_editor_id_reference_cb(old_id, new_id);
+ remap_editor_id_reference_cb(old_id, new_id);
}
skipped_direct = id_remap_data.skipped_direct;
skipped_refcounted = id_remap_data.skipped_refcounted;
- BLI_assert(old_id->us - skipped_refcounted >= 0);
-
- /* If old_id was used by some ugly 'user_one' stuff (like Image or Clip editors...), we have the right to
- * decrease once more its user count... unless we had to skip some 'user_one' cases. I hate that stuff!!! */
- if (is_user_one && !(id_remap_data.flag & ID_REMAP_IS_USER_ONE_SKIPPED) && (old_id->us - skipped_refcounted > 0)) {
- BLI_assert(old_id->us - skipped_refcounted == 1);
+ /* If old_id was used by some ugly 'user_one' stuff (like Image or Clip editors...), and user count has actually
+ * been incremented for that, we have to decrease once more its user count... unless we had to skip
+ * some 'user_one' cases. */
+ if ((old_id->flag2 & LIB_EXTRAUSER_SET) && !(id_remap_data.flag & ID_REMAP_IS_USER_ONE_SKIPPED)) {
id_us_min(old_id);
+ old_id->flag2 &= ~LIB_EXTRAUSER_SET;
}
+ BLI_assert(old_id->us - skipped_refcounted >= 0);
+ UNUSED_VARS_NDEBUG(skipped_refcounted);
+
if (skipped_direct == 0) {
/* old_id is assumed to not be used directly anymore... */
if (old_id->lib && (old_id->flag & LIB_EXTERN)) {
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 209159f..6996133 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -7927,6 +7927,8 @@ static BHead *read_libblock(FileData *fd, Main *main, BHead *bhead, int flag, ID
/* clear first 8 bits */
id->flag = (id->flag & 0xFF00) | flag | LIB_NEED_LINK;
+ /* clear first 16 bits */
+ id->flag2 = (id->flag2 & 0xFFFF0000);
id->lib = main->curlib;
id->us = (id->flag & LIB_FAKEUSER) ? 1 : 0;
id->icon_id = 0;
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 1d140f9..de15219 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -127,8 +127,9 @@ typedef struct ID {
* to.
*/
short flag;
+ int flag2;
int us;
- int icon_id, pad2;
+ int icon_id;
IDProperty *properties;
} ID;
@@ -299,6 +300,14 @@ enum {
LIB_ID_RECALC_ALL = (LIB_ID_RECALC | LIB_ID_RECALC_DATA),
};
+/* id->flag2: set first 16 bits always at zero while reading */
+enum {
+ /* tag datablock has having an extra user. */
+ LIB_EXTRAUSER = 1 << 0,
+ /* tag datablock has having actually increased usercount for the extra virtual user. */
+ LIB_EXTRAUSER_SET = 1 << 1,
+};
+
/* To filter ID types (filter_id) */
/* XXX We cannot put all needed IDs inside an enum...
* We'll have to see whether we can fit all needed ones inside 32 values,
More information about the Bf-blender-cvs
mailing list