[Bf-blender-cvs] [67ee496c1b3] asset-uuid: Updates from code review.
Bastien Montagne
noreply at git.blender.org
Tue Nov 26 17:34:30 CET 2019
Commit: 67ee496c1b36963fd0c82708d843dbcd293807d9
Author: Bastien Montagne
Date: Tue Nov 26 17:28:02 2019 +0100
Branches: asset-uuid
https://developer.blender.org/rB67ee496c1b36963fd0c82708d843dbcd293807d9
Updates from code review.
Mostly various cleanups, and a few important fixes too (some potential
memleaks, missing writing of UUID struct for ID placeholders of linked
data-block in writefile.c).
===================================================================
M source/blender/blenkernel/BKE_asset_engine.h
M source/blender/blenkernel/intern/asset_engine.c
M source/blender/blenloader/BLO_readfile.h
M source/blender/blenloader/intern/readfile.c
M source/blender/blenloader/intern/writefile.c
M source/blender/editors/space_outliner/outliner_tools.c
M source/blender/makesdna/DNA_ID.h
M source/blender/python/intern/bpy_rna_id_collection.c
===================================================================
diff --git a/source/blender/blenkernel/BKE_asset_engine.h b/source/blender/blenkernel/BKE_asset_engine.h
index 4404aa773c1..6ad9de6918d 100644
--- a/source/blender/blenkernel/BKE_asset_engine.h
+++ b/source/blender/blenkernel/BKE_asset_engine.h
@@ -50,6 +50,8 @@ struct Main;
ASSETUUID_SUB_EQUAL(_uuida, _uuidb, uuid_view))
/* Various helpers */
+void BKE_asset_uuid_runtime_reset(struct AssetUUID *uuid);
+
unsigned int BKE_asset_uuid_hash(const void *key);
bool BKE_asset_uuid_cmp(const void *a, const void *b);
void BKE_asset_uuid_print(const struct AssetUUID *uuid);
diff --git a/source/blender/blenkernel/intern/asset_engine.c b/source/blender/blenkernel/intern/asset_engine.c
index 4e63d0baa99..60c27a44eff 100644
--- a/source/blender/blenkernel/intern/asset_engine.c
+++ b/source/blender/blenkernel/intern/asset_engine.c
@@ -49,6 +49,12 @@
#endif
/* Various helpers */
+void BKE_asset_uuid_runtime_reset(AssetUUID *uuid) {
+ uuid->ibuff = NULL;
+ uuid->width = uuid->height = 0;
+ uuid->tag = 0;
+}
+
unsigned int BKE_asset_uuid_hash(const void *key)
{
BLI_HashMurmur2A mm2a;
@@ -66,13 +72,17 @@ bool BKE_asset_uuid_cmp(const void *a, const void *b)
{
const AssetUUID *uuid1 = a;
const AssetUUID *uuid2 = b;
- return !ASSETUUID_EQUAL(uuid1, uuid2); /* Expects false when compared equal... */
+ return !ASSETUUID_EQUAL(uuid1, uuid2); /* Expects false when compared equal. */
}
void BKE_asset_uuid_print(const AssetUUID *uuid)
{
- /* TODO print nicer (as 128bit hexadecimal...). */
- printf("[%d,%d,%d,%d][%d,%d,%d,%d][%d,%d,%d,%d][%d,%d,%d,%d][%d,%d,%d,%d]\n",
+ /* TODO print nicer (as 128bit hexadecimal?). */
+ printf("[%.10d,%.10d,%.10d,%.10d]"
+ "[%.10d,%.10d,%.10d,%.10d]"
+ "[%.10d,%.10d,%.10d,%.10d]"
+ "[%.10d,%.10d,%.10d,%.10d]"
+ "[%.10d,%.10d,%.10d,%.10d]\n",
uuid->uuid_repository[0],
uuid->uuid_repository[1],
uuid->uuid_repository[2],
diff --git a/source/blender/blenloader/BLO_readfile.h b/source/blender/blenloader/BLO_readfile.h
index d1c6189ab9c..adf3bf00d48 100644
--- a/source/blender/blenloader/BLO_readfile.h
+++ b/source/blender/blenloader/BLO_readfile.h
@@ -28,7 +28,6 @@
extern "C" {
#endif
-struct AssetUUID;
struct BHead;
struct BlendThumbnail;
struct FileData;
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 1c86af638da..3dac6ed2160 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -2684,8 +2684,8 @@ static void direct_link_id(FileData *fd, ID *id)
if (id->uuid) {
id->uuid = newdataadr(fd, id->uuid);
- id->uuid->ibuff = NULL; /* Just in case... */
- id->uuid->width = id->uuid->height = 0;
+ /* Make sure runtime fields are always zeroed out. */
+ BKE_asset_uuid_runtime_reset(id->uuid);
}
}
@@ -9313,12 +9313,12 @@ static BHead *read_libblock(FileData *fd,
}
if (id->uuid) {
- /* read all data into fd->datamap */
+ /* Read all data into fd->datamap. */
bhead = read_data_into_oldnewmap(fd, bhead, __func__);
id->uuid = newdataadr(fd, id->uuid);
- id->uuid->ibuff = NULL; /* Just in case... */
- id->uuid->width = id->uuid->height = 0;
+ /* Make sure runtime fields are always zeroed out. */
+ BKE_asset_uuid_runtime_reset(id->uuid);
oldnewmap_free_unused(fd->datamap);
oldnewmap_clear(fd->datamap);
@@ -11890,8 +11890,18 @@ static void read_library_linked_ids(FileData *basefd,
/* BLI_assert(*realid != NULL); */
if (*realid && id->uuid) {
- /* we can give ownership of that pointer to new ID. */
+ /* it is important to keep the UUID we stored in that .blend file, not the (potentially
+ * different) one we get from the library, updating UUID should be handled by asset
+ * engine later - even though changing UUID is not recommended in any case. */
+ if ((*realid)->uuid != NULL) {
+ MEM_SAFE_FREE((*realid)->uuid);
+ }
+ /* We can give ownership of that pointer to new ID. */
(*realid)->uuid = id->uuid;
+ id->uuid = NULL;
+ }
+ else {
+ MEM_SAFE_FREE(id->uuid);
}
/* Now that we have a real ID, replace all pointers to placeholders in
@@ -11908,6 +11918,8 @@ static void read_library_linked_ids(FileData *basefd,
/* Clear GHash and free link placeholder IDs of the current type. */
BLI_ghash_clear(loaded_ids, NULL, NULL);
+ /* Note: this is currently just freeing ID struct itself, assuming there is no remaining
+ * allocated sub-data owned by this ID. */
BLI_freelistN(&pending_free_ids);
}
diff --git a/source/blender/blenloader/intern/writefile.c b/source/blender/blenloader/intern/writefile.c
index 077515c8b05..f094a106728 100644
--- a/source/blender/blenloader/intern/writefile.c
+++ b/source/blender/blenloader/intern/writefile.c
@@ -3698,6 +3698,11 @@ static void write_libraries(WriteData *wd, Main *main)
BLI_assert(0);
}
writestruct(wd, ID_LINK_PLACEHOLDER, ID, 1, id);
+ /* It is mandatory to write id's asset uuid reference for placeholders too, otherwise
+ * the whole asset info would be completely lost when reloading the linked data-block,
+ * especially in case it is not immediately found and needs to go through the whole
+ * 'asset engine update' process after main .blend read process is finished. */
+ write_iddata(wd, id);
}
}
}
diff --git a/source/blender/editors/space_outliner/outliner_tools.c b/source/blender/editors/space_outliner/outliner_tools.c
index 13545e01771..4855f943fb9 100644
--- a/source/blender/editors/space_outliner/outliner_tools.c
+++ b/source/blender/editors/space_outliner/outliner_tools.c
@@ -697,7 +697,8 @@ static void id_local_cb(bContext *C,
Main *bmain = CTX_data_main(C);
/* if the ID type has no special local function,
* just clear the lib. */
- /* XXX This is very, very, **very** suspicious - should not be handled that way at all!!! */
+ /* XXX This is very, very, **very** suspicious - should not be handled that way at all.
+ * Probably best thing to do here is to simply not do anything. */
if (id_make_local(bmain, tselem->id, false, false) == false) {
id_clear_lib_data(bmain, tselem->id);
}
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 43760df9517..3ea45168f45 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -239,7 +239,7 @@ enum eOverrideLibrary_Flag {
* Each engine is free to use it as it likes - it will be the only thing passed to it by blender to
* identify repository/asset/variant/version/view.
* Assumed to be 128bits (16 bytes) each, handled as four integers due to lack of real
- * bytes proptype in RNA :|.
+ * bytes proptype in RNA.
*/
#define ASSET_UUID_LENGTH 16
@@ -250,15 +250,18 @@ typedef struct AssetUUID {
int uuid_variant[4];
int uuid_revision[4];
int uuid_view[4];
- short flag; /* Saved. */
- short tag; /* Runtime. */
+ short flag;
+
+ /* Everything below this comment is runtime data. */
+ short tag;
/* Preview. */
short width;
short height;
char *ibuff; /* RGBA 8bits. */
- /* Used for load_post... */
+ /* Used for load_post and other temporary storage of the ID itself in its AssetUUID.
+ * Usage and scope are similar to the `ID.newid` pointer. */
struct ID *id;
} AssetUUID;
@@ -279,9 +282,9 @@ enum {
1 << 1, /* The asset engine was found but does not know about this asset (anymore). */
UUID_TAG_ASSET_RELOAD =
- 1 << 8, /* Set by the asset engine to indicates that that asset has to be reloaded. */
+ 1 << 8, /* Set by the asset engine to indicate that this asset has to be reloaded. */
UUID_TAG_ASSET_NOPREVIEW =
- 1 << 9, /* Set by the asset engine to indicates that that asset has no preview. */
+ 1 << 9, /* Set by the asset engine to indicate that this asset has no preview. */
};
/* watch it: Sequence has identical beginning. */
diff --git a/source/blender/python/intern/bpy_rna_id_collection.c b/source/blender/python/intern/bpy_rna_id_collection.c
index a30fa30cc49..5434503e491 100644
--- a/source/blender/python/intern/bpy_rna_id_collection.c
+++ b/source/blender/python/intern/bpy_rna_id_collection.c
@@ -398,7 +398,7 @@ PyDoc_STRVAR(
" :rtype: ID\n");
static PyObject *bpy_asset_uuid_search(PyObject *UNUSED(self), PyObject *args, PyObject *kwds)
{
-#if 0 /* If someone knows how to get a proper 'self' in that case... */
+#if 0 /* TODO: Find a proper way of getting bmain from `self ` in this case. */
BPy_StructRNA *pyrna = (BPy_StructRNA *)self;
Main *bmain = pyrna->ptr.data;
#else
More information about the Bf-blender-cvs
mailing list