[Bf-blender-cvs] [29887bbb329] asset-browser-poselib: Fix possible crash when blending poses

Julian Eisel noreply at git.blender.org
Fri Mar 26 17:42:20 CET 2021


Commit: 29887bbb329da4fe298a794521fa510258707217
Author: Julian Eisel
Date:   Fri Mar 26 17:25:03 2021 +0100
Branches: asset-browser-poselib
https://developer.blender.org/rB29887bbb329da4fe298a794521fa510258707217

Fix possible crash when blending poses

I did some casting hack to return an `AssetHandle` from a
`FileDirEntry`. But this doesn't actually work.

What I added now isn't something I'm happy with either, but it will have
to do for now. Basically we request an `AssetHandle`, which the Asset
Browser doesn't actually store, since it's just a wrapper around the
Asset Browser's `FileDirEntry`. So there is no consistent pointer the
editor could return to context queries, and the context needs a pointer
so that it can be passed around as `void *`.

Eventually once we have the planned asset storage design implemented,
the storage would include actual asset handles. So we can pass around
consistent pointers to them then.

===================================================================

M	source/blender/blenkernel/BKE_context.h
M	source/blender/blenkernel/intern/context.c
M	source/blender/editors/armature/pose_lib_2.c
M	source/blender/editors/space_file/space_file.c

===================================================================

diff --git a/source/blender/blenkernel/BKE_context.h b/source/blender/blenkernel/BKE_context.h
index 3bb4ad60007..a37fdfab3b3 100644
--- a/source/blender/blenkernel/BKE_context.h
+++ b/source/blender/blenkernel/BKE_context.h
@@ -341,6 +341,7 @@ int CTX_data_editable_gpencil_layers(const bContext *C, ListBase *list);
 int CTX_data_editable_gpencil_strokes(const bContext *C, ListBase *list);
 
 const struct AssetLibraryReference *CTX_wm_asset_library(const bContext *C);
+struct AssetHandle CTX_wm_asset_handle(const bContext *C, bool *r_is_valid);
 
 bool CTX_wm_interface_locked(const bContext *C);
 
diff --git a/source/blender/blenkernel/intern/context.c b/source/blender/blenkernel/intern/context.c
index 741db29d49b..f287a46a5cb 100644
--- a/source/blender/blenkernel/intern/context.c
+++ b/source/blender/blenkernel/intern/context.c
@@ -1403,6 +1403,32 @@ const AssetLibraryReference *CTX_wm_asset_library(const bContext *C)
   return ctx_data_pointer_get(C, "asset_library");
 }
 
+AssetHandle CTX_wm_asset_handle(const bContext *C, bool *r_is_valid)
+{
+  *r_is_valid = false;
+
+  AssetHandle *asset_handle_p =
+      (AssetHandle *)CTX_data_pointer_get_type(C, "asset_handle", &RNA_AssetHandle).data;
+  if (asset_handle_p) {
+    *r_is_valid = true;
+    return *asset_handle_p;
+  }
+
+  /* If the asset handle was not found in context directly, try if there's an active file with
+   * asset data there instead. Not nice to have this here, would be better to have this in
+   * `ED_asset.h`, but we can't include that in BKE. Even better would be not needing this at all
+   * and being able to have editors return this in the usual `context` callback. But that would
+   * require returning a non-owning pointer, which we don't have in the Asset Browser (yet). */
+  FileDirEntry *file =
+      (FileDirEntry *)CTX_data_pointer_get_type(C, "active_file", &RNA_FileSelectEntry).data;
+  if (file) {
+    *r_is_valid = true;
+    return (AssetHandle){.file_data = file};
+  }
+
+  return (AssetHandle){0};
+}
+
 Depsgraph *CTX_data_depsgraph_pointer(const bContext *C)
 {
   Main *bmain = CTX_data_main(C);
diff --git a/source/blender/editors/armature/pose_lib_2.c b/source/blender/editors/armature/pose_lib_2.c
index 1c62f119501..da8a5bbd175 100644
--- a/source/blender/editors/armature/pose_lib_2.c
+++ b/source/blender/editors/armature/pose_lib_2.c
@@ -393,15 +393,15 @@ static void poselib_tempload_exit(PoseBlendData *pbd)
 
 static bAction *poselib_blend_init_get_action(bContext *C, wmOperator *op)
 {
+  bool asset_handle_valid;
   const AssetLibraryReference *asset_library = CTX_wm_asset_library(C);
-  const AssetHandle *asset_handle =
-      CTX_data_pointer_get_type(C, "asset_handle", &RNA_AssetHandle).data;
+  const AssetHandle asset_handle = CTX_wm_asset_handle(C, &asset_handle_valid);
   /* Poll callback should check. */
-  BLI_assert((asset_library != NULL) && (asset_handle != NULL));
+  BLI_assert((asset_library != NULL) && asset_handle_valid);
 
   PoseBlendData *pbd = op->customdata;
 
-  pbd->temp_id_consumer = ED_asset_temp_id_consumer_create(asset_handle);
+  pbd->temp_id_consumer = ED_asset_temp_id_consumer_create(&asset_handle);
   return (bAction *)ED_asset_temp_id_consumer_ensure_local_id(
       pbd->temp_id_consumer, asset_library, ID_AC, CTX_data_main(C), op->reports);
 }
@@ -578,11 +578,12 @@ static int poselib_blend_exec(bContext *C, wmOperator *op)
 
 static bool poselib_asset_in_context(bContext *C)
 {
+  bool asset_handle_valid;
   /* Check whether the context provides the asset data needed to add a pose. */
   const AssetLibraryReference *asset_library = CTX_wm_asset_library(C);
-  const AssetHandle *asset_handle =
-      CTX_data_pointer_get_type(C, "asset_handle", &RNA_AssetHandle).data;
-  return (asset_library) != NULL && (asset_handle != NULL);
+  CTX_wm_asset_handle(C, &asset_handle_valid);
+
+  return (asset_library != NULL) && asset_handle_valid;
 }
 
 /* Poll callback for operators that require existing PoseLib data (with poses) to work. */
diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c
index b64fa0cdc37..6cf75a67432 100644
--- a/source/blender/editors/space_file/space_file.c
+++ b/source/blender/editors/space_file/space_file.c
@@ -884,20 +884,6 @@ static int /*eContextResult*/ file_context(const bContext *C,
     CTX_data_pointer_set(result, &screen->id, &RNA_FileSelectEntry, file);
     return CTX_RESULT_OK;
   }
-  /* TODO this kind of handle is very weak. We need something proper that doesn't depend on file
-   * data. */
-  if (CTX_data_equals(member, "asset_handle")) {
-    FileDirEntry *file = filelist_file(sfile->files, params->active_file);
-    if (file == NULL || !file->asset_data) {
-      return CTX_RESULT_NO_DATA;
-    }
-
-    BLI_STATIC_ASSERT(sizeof(AssetHandle) == sizeof(FileDirEntry *),
-                      "Expected AssetHandle to match FileDirEntry");
-
-    CTX_data_pointer_set(result, &screen->id, &RNA_AssetHandle, file);
-    return CTX_RESULT_OK;
-  }
   if (CTX_data_equals(member, "asset_library")) {
     FileAssetSelectParams *asset_params = ED_fileselect_get_asset_params(sfile);



More information about the Bf-blender-cvs mailing list