griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Cleanup: Store asset shelf types in vector of unique_ptr

Browse Source 
This avoids the need for manual memory management/raw pointers,
improves const correctness, improves type safety, simplifies iteration,
and simplifies the process of registering a new asset shelf type.

Pull Request: https://projects.blender.org/blender/blender/pulls/117770
This commit is contained in:
Hans Goudey
2024-02-05 16:23:57 +01:00
committed by Hans Goudey
parent bb915ae9bd
commit cfb4e5a25d
4 changed files with 41 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
source/blender/blenkernel/BKE_screen.hh
View File

@@ -22,6 +22,7 @@ class AssetRepresentation;
}
struct ARegion;
struct AssetShelfType;
struct BlendDataReader;
struct BlendLibReader;
struct BlendWriter;
@@ -140,7 +141,7 @@ struct SpaceType {
ListBase regiontypes;
/** Asset shelf type definitions. */
ListBase asset_shelf_types; /* #AssetShelfType */
blender::Vector<std::unique_ptr<AssetShelfType>> asset_shelf_types;
/* read and write... */
@@ -518,8 +519,6 @@ enum AssetShelfTypeFlag {
ENUM_OPERATORS(AssetShelfTypeFlag, ASSET_SHELF_TYPE_FLAG_MAX);
struct AssetShelfType {
AssetShelfType *next, *prev;
char idname[BKE_ST_MAXNAME]; /* unique name */
int space_type;

1
source/blender/blenkernel/intern/screen.cc
View File

@@ -238,7 +238,6 @@ SpaceType::~SpaceType()
}
BLI_freelistN(&this->regiontypes);
BLI_freelistN(&this->asset_shelf_types);
}
void BKE_spacetypes_free()

28
source/blender/editors/asset/intern/asset_shelf.cc
View File

@@ -59,23 +59,27 @@ static bool asset_shelf_type_poll(const bContext &C,
return false;
}
BLI_assert_msg(BLI_findindex(&space_type.asset_shelf_types, shelf_type) != -1,
BLI_assert_msg(std::find_if(space_type.asset_shelf_types.begin(),
space_type.asset_shelf_types.end(),
[&](const std::unique_ptr<AssetShelfType> &type) {
return type.get() == shelf_type;
}) != space_type.asset_shelf_types.end(),
"Asset shelf type is not registered");
UNUSED_VARS_NDEBUG(space_type);
return !shelf_type->poll || shelf_type->poll(&C, shelf_type);
}
static AssetShelfType *asset_shelf_type_ensure(const SpaceType &space_type, AssetShelf &shelf)
static AssetShelfType *asset_shelf_type_ensure(SpaceType &space_type, AssetShelf &shelf)
{
if (shelf.type) {
return shelf.type;
}
LISTBASE_FOREACH (AssetShelfType *, shelf_type, &space_type.asset_shelf_types) {
for (std::unique_ptr<AssetShelfType> &shelf_type : space_type.asset_shelf_types) {
if (STREQ(shelf.idname, shelf_type->idname)) {
shelf.type = shelf_type;
return shelf_type;
shelf.type = shelf_type.get();
return shelf_type.get();
}
}
@@ -133,7 +137,7 @@ static void activate_shelf(RegionAssetShelf &shelf_regiondata, AssetShelf &shelf
* current context (all polls failed).
*/
static AssetShelf *update_active_shelf(const bContext &C,
const SpaceType &space_type,
SpaceType &space_type,
RegionAssetShelf &shelf_regiondata)
{
/* Note: Don't access #AssetShelf.type directly, use #asset_shelf_type_ensure(). */
@@ -163,8 +167,8 @@ static AssetShelf *update_active_shelf(const bContext &C,
}
/* Case 3: */
LISTBASE_FOREACH (AssetShelfType *, shelf_type, &space_type.asset_shelf_types) {
if (asset_shelf_type_poll(C, space_type, shelf_type)) {
for (std::unique_ptr<AssetShelfType> &shelf_type : space_type.asset_shelf_types) {
if (asset_shelf_type_poll(C, space_type, shelf_type.get())) {
AssetShelf *new_shelf = create_shelf_from_type(*shelf_type);
BLI_addhead(&shelf_regiondata.shelves, new_shelf);
/* Moves ownership to the regiondata. */
@@ -211,8 +215,8 @@ static bool asset_shelf_space_poll(const bContext *C, const SpaceLink *space_lin
const SpaceType *space_type = BKE_spacetype_from_id(space_link->spacetype);
/* Is there any asset shelf type registered that returns true for it's poll? */
LISTBASE_FOREACH (AssetShelfType *, shelf_type, &space_type->asset_shelf_types) {
if (asset_shelf_type_poll(*C, *space_type, shelf_type)) {
for (const std::unique_ptr<AssetShelfType> &shelf_type : space_type->asset_shelf_types) {
if (asset_shelf_type_poll(*C, *space_type, shelf_type.get())) {
return true;
}
}
@@ -410,7 +414,7 @@ int region_prefsizey()
void region_layout(const bContext *C, ARegion *region)
{
const SpaceLink *space = CTX_wm_space_data(C);
const SpaceType *space_type = BKE_spacetype_from_id(space->spacetype);
SpaceType *space_type = BKE_spacetype_from_id(space->spacetype);
RegionAssetShelf *shelf_regiondata = RegionAssetShelf::get_from_asset_shelf_region(*region);
if (!shelf_regiondata) {
@@ -486,7 +490,7 @@ void header_region_init(wmWindowManager * /*wm*/, ARegion *region)
void header_region(const bContext *C, ARegion *region)
{
const SpaceLink *space = CTX_wm_space_data(C);
const SpaceType *space_type = BKE_spacetype_from_id(space->spacetype);
SpaceType *space_type = BKE_spacetype_from_id(space->spacetype);
const ARegion *main_shelf_region = BKE_area_find_region_type(CTX_wm_area(C),
RGN_TYPE_ASSET_SHELF);

42
source/blender/makesrna/intern/rna_ui.cc
View File

@@ -1185,7 +1185,13 @@ static bool rna_AssetShelf_unregister(Main *bmain, StructRNA *type)
RNA_struct_free_extension(type, &shelf_type->rna_ext);
RNA_struct_free(&BLENDER_RNA, type);
BLI_freelinkN(&space_type->asset_shelf_types, shelf_type);
const auto it = std::find_if(
space_type->asset_shelf_types.begin(),
space_type->asset_shelf_types.end(),
[&](const std::unique_ptr<AssetShelfType> &type) { return type.get() == shelf_type; });
BLI_assert(it != space_type->asset_shelf_types.end());
space_type->asset_shelf_types.remove(it - space_type->asset_shelf_types.begin());
/* update while blender is running */
WM_main_add_notifier(NC_WINDOW, nullptr);
@@ -1201,11 +1207,11 @@ static StructRNA *rna_AssetShelf_register(Main *bmain,
StructCallbackFunc call,
StructFreeFunc free)
{
AssetShelfType dummy_shelf_type = {};
AssetShelf dummy_shelf = {};
std::unique_ptr<AssetShelfType> shelf_type = std::make_unique<AssetShelfType>();
/* setup dummy shelf & shelf type to store static properties in */
dummy_shelf.type = &dummy_shelf_type;
AssetShelf dummy_shelf = {};
dummy_shelf.type = shelf_type.get();
PointerRNA dummy_shelf_ptr = RNA_pointer_create(nullptr, &RNA_AssetShelf, &dummy_shelf);
bool have_function[3];
@@ -1215,64 +1221,62 @@ static StructRNA *rna_AssetShelf_register(Main *bmain,
return nullptr;
}
if (strlen(identifier) >= sizeof(dummy_shelf_type.idname)) {
if (strlen(identifier) >= sizeof(shelf_type->idname)) {
BKE_reportf(reports,
RPT_ERROR,
"Registering asset shelf class: '%s' is too long, maximum length is %d",
identifier,
(int)sizeof(dummy_shelf_type.idname));
(int)sizeof(shelf_type->idname));
return nullptr;
}
SpaceType *space_type = BKE_spacetype_from_id(dummy_shelf_type.space_type);
SpaceType *space_type = BKE_spacetype_from_id(shelf_type->space_type);
if (!space_type) {
BLI_assert_unreachable();
return nullptr;
}
/* Check if we have registered this asset shelf type before, and remove it. */
LISTBASE_FOREACH (AssetShelfType *, iter_shelf_type, &space_type->asset_shelf_types) {
if (STREQ(iter_shelf_type->idname, dummy_shelf_type.idname)) {
for (std::unique_ptr<AssetShelfType> &iter_shelf_type : space_type->asset_shelf_types) {
if (STREQ(iter_shelf_type->idname, shelf_type->idname)) {
if (iter_shelf_type->rna_ext.srna) {
BKE_reportf(reports,
RPT_INFO,
"Registering asset shelf class: '%s' has been registered before, "
"unregistering previous",
dummy_shelf_type.idname);
shelf_type->idname);
rna_AssetShelf_unregister(bmain, iter_shelf_type->rna_ext.srna);
}
break;
}
}
if (!RNA_struct_available_or_report(reports, dummy_shelf_type.idname)) {
if (!RNA_struct_available_or_report(reports, shelf_type->idname)) {
return nullptr;
}
if (!RNA_struct_bl_idname_ok_or_report(reports, dummy_shelf_type.idname, "_AST_")) {
if (!RNA_struct_bl_idname_ok_or_report(reports, shelf_type->idname, "_AST_")) {
return nullptr;
}
/* Create the new shelf type. */
AssetShelfType *shelf_type = static_cast<AssetShelfType *>(
MEM_mallocN(sizeof(*shelf_type), __func__));
memcpy(shelf_type, &dummy_shelf_type, sizeof(*shelf_type));
shelf_type->rna_ext.srna = RNA_def_struct_ptr(&BLENDER_RNA, shelf_type->idname, &RNA_AssetShelf);
shelf_type->rna_ext.data = data;
shelf_type->rna_ext.call = call;
shelf_type->rna_ext.free = free;
RNA_struct_blender_type_set(shelf_type->rna_ext.srna, shelf_type);
RNA_struct_blender_type_set(shelf_type->rna_ext.srna, shelf_type.get());
shelf_type->poll = have_function[0] ? asset_shelf_poll : nullptr;
shelf_type->asset_poll = have_function[1] ? asset_shelf_asset_poll : nullptr;
shelf_type->draw_context_menu = have_function[2] ? asset_shelf_draw_context_menu : nullptr;
BLI_addtail(&space_type->asset_shelf_types, shelf_type);
StructRNA *srna = shelf_type->rna_ext.srna;
space_type->asset_shelf_types.append(std::move(shelf_type));
/* update while blender is running */
WM_main_add_notifier(NC_WINDOW, nullptr);
return shelf_type->rna_ext.srna;
return srna;
}
static StructRNA *rna_AssetShelf_refine(PointerRNA *shelf_ptr)