Fix #105855: Crash with node add menu assets and keyboard navigation #106237
|
@ -53,8 +53,6 @@ struct LibraryAsset {
|
||||||
struct AssetItemTree {
|
struct AssetItemTree {
|
||||||
asset_system::AssetCatalogTree catalogs;
|
asset_system::AssetCatalogTree catalogs;
|
||||||
MultiValueMap<asset_system::AssetCatalogPath, LibraryAsset> assets_per_path;
|
MultiValueMap<asset_system::AssetCatalogPath, LibraryAsset> assets_per_path;
|
||||||
Map<const asset_system::AssetCatalogTreeItem *, asset_system::AssetCatalogPath>
|
|
||||||
full_catalog_per_tree_item;
|
|
||||||
};
|
};
|
||||||
|
|
||||||
static AssetLibraryReference all_library_reference()
|
static AssetLibraryReference all_library_reference()
|
||||||
|
@ -71,6 +69,34 @@ static bool all_loading_finished()
|
||||||
return ED_assetlist_is_loaded(&all_library_ref);
|
return ED_assetlist_is_loaded(&all_library_ref);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static asset_system::AssetLibrary *get_all_library_once_available()
|
||||||
|
{
|
||||||
|
const AssetLibraryReference all_library_ref = all_library_reference();
|
||||||
|
return ED_assetlist_library_get_once_available(all_library_ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The menus want to pass catalog paths to context and for this they need persistent pointers to
|
||||||
|
* the paths. Rather than keeping some own path storage, get a pointer into the asset system
|
||||||
HooglyBoogly marked this conversation as resolved
|
|||||||
|
* directly, which is persistent until the library is reloaded and can safely be held by context.
|
||||||
|
*/
|
||||||
|
static PointerRNA persistent_catalog_path_rna_pointer(
|
||||||
|
bScreen &owner_screen,
|
||||||
|
const asset_system::AssetLibrary &library,
|
||||||
|
const asset_system::AssetCatalogTreeItem &item)
|
||||||
|
{
|
||||||
|
asset_system::AssetCatalog *catalog = library.catalog_service->find_catalog_by_path(
|
||||||
HooglyBoogly marked this conversation as resolved
Hans Goudey
commented
I made this pointer const, and the one below, since there's already a const cast below, and keeping the non-const local to RNA makes most sense. I made this pointer const, and the one below, since there's already a const cast below, and keeping the non-const local to RNA makes most sense.
|
|||||||
|
item.catalog_path());
|
||||||
|
if (!catalog) {
|
||||||
|
return PointerRNA_NULL;
|
||||||
|
}
|
||||||
|
|
||||||
|
asset_system::AssetCatalogPath &path = catalog->path;
|
||||||
|
return {&owner_screen.id,
|
||||||
|
&RNA_AssetCatalogPath,
|
||||||
|
const_cast<asset_system::AssetCatalogPath *>(&path)};
|
||||||
|
}
|
||||||
|
|
||||||
static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node_tree)
|
static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node_tree)
|
||||||
{
|
{
|
||||||
if (!node_tree) {
|
if (!node_tree) {
|
||||||
|
@ -88,8 +114,7 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
|
||||||
ED_assetlist_storage_fetch(&all_library_ref, &C);
|
ED_assetlist_storage_fetch(&all_library_ref, &C);
|
||||||
ED_assetlist_ensure_previews_job(&all_library_ref, &C);
|
ED_assetlist_ensure_previews_job(&all_library_ref, &C);
|
||||||
|
|
||||||
asset_system::AssetLibrary *all_library = ED_assetlist_library_get_once_available(
|
asset_system::AssetLibrary *all_library = get_all_library_once_available();
|
||||||
all_library_ref);
|
|
||||||
if (!all_library) {
|
if (!all_library) {
|
||||||
return {};
|
return {};
|
||||||
}
|
}
|
||||||
|
@ -132,18 +157,7 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
|
||||||
catalogs_with_node_assets.insert_item(*catalog);
|
catalogs_with_node_assets.insert_item(*catalog);
|
||||||
});
|
});
|
||||||
|
|
||||||
/* Build another map storing full asset paths for each tree item, in order to have stable
|
return {std::move(catalogs_with_node_assets), std::move(assets_per_path)};
|
||||||
* pointers to asset catalog paths to use for context pointers. This is necessary because
|
|
||||||
* #asset_system::AssetCatalogTreeItem doesn't store its full path directly. */
|
|
||||||
Map<const asset_system::AssetCatalogTreeItem *, asset_system::AssetCatalogPath>
|
|
||||||
full_catalog_per_tree_item;
|
|
||||||
catalogs_with_node_assets.foreach_item([&](asset_system::AssetCatalogTreeItem &item) {
|
|
||||||
full_catalog_per_tree_item.add_new(&item, item.catalog_path());
|
|
||||||
});
|
|
||||||
|
|
||||||
return {std::move(catalogs_with_node_assets),
|
|
||||||
std::move(assets_per_path),
|
|
||||||
std::move(full_catalog_per_tree_item)};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void node_add_catalog_assets_draw(const bContext *C, Menu *menu)
|
static void node_add_catalog_assets_draw(const bContext *C, Menu *menu)
|
||||||
|
@ -193,14 +207,20 @@ static void node_add_catalog_assets_draw(const bContext *C, Menu *menu)
|
||||||
col, AS_asset_representation_name_get(&item.asset), ICON_NONE, "NODE_OT_add_group_asset");
|
col, AS_asset_representation_name_get(&item.asset), ICON_NONE, "NODE_OT_add_group_asset");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
asset_system::AssetLibrary *all_library = get_all_library_once_available();
|
||||||
|
if (!all_library) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
catalog_item->foreach_child([&](asset_system::AssetCatalogTreeItem &child_item) {
|
catalog_item->foreach_child([&](asset_system::AssetCatalogTreeItem &child_item) {
|
||||||
const asset_system::AssetCatalogPath &path = tree.full_catalog_per_tree_item.lookup(
|
PointerRNA path_ptr = persistent_catalog_path_rna_pointer(screen, *all_library, child_item);
|
||||||
&child_item);
|
if (path_ptr.data == nullptr) {
|
||||||
PointerRNA path_ptr{
|
return;
|
||||||
&screen.id, &RNA_AssetCatalogPath, const_cast<asset_system::AssetCatalogPath *>(&path)};
|
}
|
||||||
|
|
||||||
uiLayout *col = uiLayoutColumn(layout, false);
|
uiLayout *col = uiLayoutColumn(layout, false);
|
||||||
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
||||||
uiItemM(col, "NODE_MT_node_add_catalog_assets", path.name().c_str(), ICON_NONE);
|
uiItemM(col, "NODE_MT_node_add_catalog_assets", child_item.get_name().c_str(), ICON_NONE);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -261,16 +281,22 @@ static void add_root_catalogs_draw(const bContext *C, Menu *menu)
|
||||||
return menus;
|
return menus;
|
||||||
}();
|
}();
|
||||||
|
|
||||||
|
asset_system::AssetLibrary *all_library = get_all_library_once_available();
|
||||||
|
if (!all_library) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
tree.catalogs.foreach_root_item([&](asset_system::AssetCatalogTreeItem &item) {
|
tree.catalogs.foreach_root_item([&](asset_system::AssetCatalogTreeItem &item) {
|
||||||
if (all_builtin_menus.contains(item.get_name())) {
|
if (all_builtin_menus.contains(item.get_name())) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const asset_system::AssetCatalogPath &path = tree.full_catalog_per_tree_item.lookup(&item);
|
PointerRNA path_ptr = persistent_catalog_path_rna_pointer(screen, *all_library, item);
|
||||||
PointerRNA path_ptr{
|
if (path_ptr.data == nullptr) {
|
||||||
&screen.id, &RNA_AssetCatalogPath, const_cast<asset_system::AssetCatalogPath *>(&path)};
|
return;
|
||||||
|
}
|
||||||
uiLayout *col = uiLayoutColumn(layout, false);
|
uiLayout *col = uiLayoutColumn(layout, false);
|
||||||
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
||||||
uiItemM(col, "NODE_MT_node_add_catalog_assets", path.name().c_str(), ICON_NONE);
|
uiItemM(col, "NODE_MT_node_add_catalog_assets", item.get_name().c_str(), ICON_NONE);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -308,9 +334,17 @@ void uiTemplateNodeAssetMenuItems(uiLayout *layout, bContext *C, const char *cat
|
||||||
if (!item) {
|
if (!item) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const asset_system::AssetCatalogPath &path = tree.full_catalog_per_tree_item.lookup(item);
|
|
||||||
PointerRNA path_ptr{
|
asset_system::AssetLibrary *all_library = get_all_library_once_available();
|
||||||
&screen.id, &RNA_AssetCatalogPath, const_cast<asset_system::AssetCatalogPath *>(&path)};
|
if (!all_library) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
PointerRNA path_ptr = persistent_catalog_path_rna_pointer(screen, *all_library, *item);
|
||||||
|
if (path_ptr.data == nullptr) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
uiItemS(layout);
|
uiItemS(layout);
|
||||||
uiLayout *col = uiLayoutColumn(layout, false);
|
uiLayout *col = uiLayoutColumn(layout, false);
|
||||||
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
uiLayoutSetContextPointer(col, "asset_catalog_path", &path_ptr);
|
||||||
|
|
Loading…
Reference in New Issue
"own" in
some own path storage
doesn't quite make sense grammatically, I replaced it with "local".