UI: Cleanup Dialog to Manage Orphaned Data #106653

Merged
Harley Acheson merged 22 commits from Harley/blender:ManageData into main 2024-01-17 01:09:21 +01:00
5 changed files with 273 additions and 33 deletions

View File

@ -237,35 +237,8 @@ class TOPBAR_MT_file_cleanup(Menu):
def draw(self, _context):
layout = self.layout
layout.separator()
props = layout.operator("outliner.orphans_purge", text="Unused Data-Blocks")
props.do_local_ids = True
props.do_linked_ids = True
props.do_recursive = False
props = layout.operator("outliner.orphans_purge", text="Recursive Unused Data-Blocks")
props.do_local_ids = True
props.do_linked_ids = True
props.do_recursive = True
layout.separator()
props = layout.operator("outliner.orphans_purge", text="Unused Linked Data-Blocks")
props.do_local_ids = False
props.do_linked_ids = True
props.do_recursive = False
props = layout.operator("outliner.orphans_purge", text="Recursive Unused Linked Data-Blocks")
props.do_local_ids = False
props.do_linked_ids = True
props.do_recursive = True
layout.separator()
props = layout.operator("outliner.orphans_purge", text="Unused Local Data-Blocks")
props.do_local_ids = True
props.do_linked_ids = False
props.do_recursive = False
props = layout.operator("outliner.orphans_purge", text="Recursive Unused Local Data-Blocks")
props.do_local_ids = True
props.do_linked_ids = False
props.do_recursive = True
layout.operator("outliner.orphans_cleanup")
layout.operator("outliner.orphans_manage")
class TOPBAR_MT_file(Menu):

View File

@ -12,6 +12,7 @@ set(INC
../../makesrna
../../sequencer
../../windowmanager
../../../../extern/fmtlib/include
# RNA_prototypes.h
${CMAKE_BINARY_DIR}/source/blender/makesrna
@ -137,6 +138,7 @@ set(LIB
bf_editor_undo
PRIVATE bf::intern::clog
PRIVATE bf::intern::guardedalloc
extern_fmtlib
)

View File

@ -65,6 +65,10 @@
#include "tree/tree_element_rna.hh"
#include "tree/tree_iterator.hh"
#include "wm_window.hh"
#include <fmt/format.h>
using namespace blender::ed::outliner;
namespace blender::ed::outliner {
@ -2140,7 +2144,7 @@ static int outliner_orphans_purge_invoke(bContext *C, wmOperator *op, const wmEv
RNA_int_set(op->ptr, "num_deleted", num_tagged[INDEX_ID_NULL]);
if (num_tagged[INDEX_ID_NULL] == 0) {
BKE_report(op->reports, RPT_INFO, "No orphaned data-blocks to purge");
BKE_report(op->reports, RPT_INFO, "No unused data-blocks to purge");
return OPERATOR_CANCELLED;
}
@ -2188,7 +2192,7 @@ static int outliner_orphans_purge_exec(bContext *C, wmOperator *op)
bmain, LIB_TAG_DOIT, do_local_ids, do_linked_ids, do_recursive_cleanup, num_tagged);
if (num_tagged[INDEX_ID_NULL] == 0) {
BKE_report(op->reports, RPT_INFO, "No orphaned data-blocks to purge");
BKE_report(op->reports, RPT_INFO, "No unused data-blocks to purge");
return OPERATOR_CANCELLED;
}
}
@ -2219,7 +2223,7 @@ void OUTLINER_OT_orphans_purge(wmOperatorType *ot)
/* identifiers */
ot->idname = "OUTLINER_OT_orphans_purge";
ot->name = "Purge All";
ot->description = "Clear all orphaned data-blocks without any users from the file";
ot->description = "Remove all unused data-blocks without any users from the file";
/* callbacks */
ot->invoke = outliner_orphans_purge_invoke;
@ -2248,10 +2252,267 @@ void OUTLINER_OT_orphans_purge(wmOperatorType *ot)
"do_recursive",
false,
"Recursive Delete",
"Recursively check for indirectly unused data-blocks, ensuring that no orphaned "
"Recursively check for indirectly unused data-blocks, ensuring that no unused "
"data-blocks remain after execution");
}
static void wm_block_orphans_cancel(bContext *C, void *arg_block, void * /*arg_data*/)
{
UI_popup_block_close(C, CTX_wm_window(C), (uiBlock *)arg_block);
}
static void wm_block_orphans_cancel_button(uiBlock *block)
{
uiBut *but = uiDefIconTextBut(
block, UI_BTYPE_BUT, 0, 0, IFACE_("Cancel"), 0, 0, 0, UI_UNIT_Y, 0, 0, 0, 0, 0, "");
UI_but_func_set(but, wm_block_orphans_cancel, block, nullptr);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
UI_but_flag_enable(but, UI_BUT_ACTIVE_DEFAULT);
}
Harley marked this conversation as resolved Outdated

UI_block_funcN_set and UI_but_funcN_set exist to avoid global variables like these. You can pass them an allocated structure with these 3 booleans, and it should be freed for when the block or button is freed.

`UI_block_funcN_set` and `UI_but_funcN_set` exist to avoid global variables like these. You can pass them an allocated structure with these 3 booleans, and it should be freed for when the block or button is freed.
struct OrphansPurgeData {
wmOperator *op = nullptr;
char local = true;
char linked = false;
char recursive = false;
};
static void wm_block_orphans_purge(bContext *C, void *arg_block, void *arg_data)
{
OrphansPurgeData *purge_data = (OrphansPurgeData *)arg_data;
wmOperator *op = purge_data->op;
Main *bmain = CTX_data_main(C);
int num_tagged[INDEX_ID_MAX] = {0};
/* Tag all IDs to delete. */
BKE_lib_query_unused_ids_tag(bmain,
LIB_TAG_DOIT,
purge_data->local,
purge_data->linked,
purge_data->recursive,
num_tagged);
if (num_tagged[INDEX_ID_NULL] == 0) {
BKE_report(op->reports, RPT_INFO, "No unused data to remove");
}
else {
BKE_id_multi_tagged_delete(bmain);
BKE_reportf(op->reports, RPT_INFO, "Deleted %d data block(s)", num_tagged[INDEX_ID_NULL]);
DEG_relations_tag_update(bmain);
WM_event_add_notifier(C, NC_ID | NA_REMOVED, nullptr);
/* Force full redraw of the UI. */
WM_main_add_notifier(NC_WINDOW, nullptr);
}
UI_popup_block_close(C, CTX_wm_window(C), (uiBlock *)arg_block);
}
static void wm_block_orphans_purge_button(uiBlock *block, OrphansPurgeData *purge_data)
{
uiBut *but = uiDefIconTextBut(
block, UI_BTYPE_BUT, 0, 0, IFACE_("Delete"), 0, 0, 0, UI_UNIT_Y, 0, 0, 0, 0, 0, "");
UI_but_func_set(but, wm_block_orphans_purge, block, purge_data);
UI_but_drawflag_disable(but, UI_BUT_TEXT_LEFT);
}
static std::string orphan_desc(const bContext *C, const bool local, bool linked, bool recursive)
Harley marked this conversation as resolved Outdated

const bContext *, const bool

`const bContext *`, `const bool`
{
Main *bmain = CTX_data_main(C);
int num_tagged[INDEX_ID_MAX] = {0};
std::string desc;
BKE_lib_query_unused_ids_tag(bmain, LIB_TAG_DOIT, local, linked, recursive, num_tagged);
bool is_first = true;
for (int i = 0; i < INDEX_ID_MAX - 2; i++) {
if (num_tagged[i] != 0) {
desc += fmt::format(
"{}{} {}",
(is_first) ? "" : ", ",
Harley marked this conversation as resolved Outdated

No reason to use append on strings afaik, just use += operator?

That kind of formatting could also benefit from using fmt::format from our extern/fmtlib library.

No reason to use `append` on strings afaik, just use `+=` operator? That kind of formatting could also benefit from using `fmt::format` from our `extern/fmtlib` library.
num_tagged[i],
(num_tagged[i] > 1) ?
TIP_(BKE_idtype_idcode_to_name_plural(BKE_idtype_idcode_from_index(i))) :
TIP_(BKE_idtype_idcode_to_name(BKE_idtype_idcode_from_index(i))));
is_first = false;
}
}
if (desc.empty()) {
desc = "Nothing";
}
return desc;
}
static uiBlock *wm_block_create_orphans_cleanup(bContext *C, ARegion *region, void *arg)
{
OrphansPurgeData *purge_data = static_cast<OrphansPurgeData *>(arg);
Harley marked this conversation as resolved Outdated

Use static_cast here instead of C style cast

Use `static_cast` here instead of C style cast
uiBlock *block = UI_block_begin(C, region, "orphans_remove_popup", UI_EMBOSS);
UI_block_flag_enable(
block, UI_BLOCK_KEEP_OPEN | UI_BLOCK_LOOP | UI_BLOCK_NO_WIN_CLIP | UI_BLOCK_NUMSELECT);
UI_block_theme_style_set(block, UI_BLOCK_THEME_STYLE_POPUP);
uiLayout *layout = uiItemsAlertBox(block, 40, ALERT_ICON_WARNING);
Harley marked this conversation as resolved Outdated

Not really important since it's already working, but I think simple std::string would be preferred over DynStr for simple cases like this where performance is not an issue.

Not really important since it's already working, but I think simple `std::string` would be preferred over `DynStr` for simple cases like this where performance is not an issue.

To be clear, I'm not expecting this to be addressed.

To be clear, I'm not expecting this to be addressed.
/* Title. */
uiItemL_ex(layout, TIP_("Purge Unused Data From This File"), 0, true, false);
uiItemS(layout);
std::string desc = "Local data: " + orphan_desc(C, true, false, false);
uiDefButBitC(block,
UI_BTYPE_CHECKBOX,
1,
0,
desc.c_str(),
0,
0,
0,
UI_UNIT_Y,
&purge_data->local,
0,
0,
0,
0,
"Delete unused local data");
desc = "Linked data: " + orphan_desc(C, false, true, false);
uiDefButBitC(block,
UI_BTYPE_CHECKBOX,
1,
0,
desc.c_str(),
0,
0,
0,
UI_UNIT_Y,
&purge_data->linked,
0,
0,
0,
0,
"Delete unused linked data");
uiDefButBitC(
block,
UI_BTYPE_CHECKBOX,
1,
0,
"Include indirect data",
Harley marked this conversation as resolved Outdated

Now getting into nitpicky territory...

This is using title case while "Local data" and "Linked data" are not. "unused" is implied for "Local data" and "Linked data", so why use it here? Clarifications like "(Recursive)" are not something we normally do in the UI, that seems like it should be in the tooltip.

I suggest "Include indirect data".

Now getting into nitpicky territory... This is using title case while "Local data" and "Linked data" are not. "unused" is implied for "Local data" and "Linked data", so why use it here? Clarifications like "(Recursive)" are not something we normally do in the UI, that seems like it should be in the tooltip. I suggest "Include indirect data".
0,
0,
0,
UI_UNIT_Y,
&purge_data->recursive,
0,
0,
0,
0,
"Recursively check for indirectly unused data, ensuring that no unused data remains");
uiItemS_ex(layout, 2.0f);
/* Buttons. */
#ifdef _WIN32
const bool windows_layout = true;
#else
const bool windows_layout = false;
#endif
uiLayout *split = uiLayoutSplit(layout, 0.0f, true);
uiLayoutSetScaleY(split, 1.2f);
if (windows_layout) {
/* Windows standard layout. */
uiLayoutColumn(split, false);
wm_block_orphans_purge_button(block, purge_data);
uiLayoutColumn(split, false);
wm_block_orphans_cancel_button(block);
}
else {
/* Non-Windows layout (macOS and Linux). */
uiLayoutColumn(split, false);
wm_block_orphans_cancel_button(block);
uiLayoutColumn(split, false);
wm_block_orphans_purge_button(block, purge_data);
}
UI_block_bounds_set_centered(block, 14 * UI_SCALE_FAC);
return block;
}
static int outliner_orphans_cleanup_exec(bContext *C, wmOperator *op)
{
OrphansPurgeData *purge_data = MEM_new<OrphansPurgeData>(__func__);

Do not invoke modal UI items from an exec function. This code should be in the invoke callback.

The exec callback should be basically what wm_block_orphans_purge is doing currently.

Further more, any reasons not to use some operator-aware popup system, like WM_operator_props_dialog_popup or so?

That way you can also define the settings (which types of data to purge) in the operator itself - or even better, can just re-use (and extend) existing OUTLINER_OT_orphans_purge, instead of creating a new one that does the same thing?

Do not invoke modal UI items from an `exec` function. This code should be in the `invoke` callback. The `exec` callback should be basically what `wm_block_orphans_purge` is doing currently. Further more, any reasons not to use some operator-aware popup system, like `WM_operator_props_dialog_popup` or so? That way you can also define the settings (which types of data to purge) in the operator itself - or even better, can just re-use (and extend) existing `OUTLINER_OT_orphans_purge`, instead of creating a new one that does the same thing?

Do not invoke modal UI items from an exec function. This code should be in the invoke callback

Thanks! done.

any reasons not to use some operator-aware popup system, like WM_operator_props_dialog_popup or so?

Just unfamiliarity with that, so will definitely will look into that. I'm also hoping to move toward dialogs that have a clear "cancel". I don't mind if some dialogs with complex behavior are more "bespoke".

> Do not invoke modal UI items from an exec function. This code should be in the invoke callback Thanks! done. > any reasons not to use some operator-aware popup system, like WM_operator_props_dialog_popup or so? Just unfamiliarity with that, so will definitely will look into that. I'm also hoping to move toward dialogs that have a clear "cancel". I don't mind if some dialogs with complex behavior are more "bespoke".
purge_data->op = op;
Harley marked this conversation as resolved Outdated

Use __func__ instead of manually choosing a string

Use `__func__` instead of manually choosing a string
UI_popup_block_invoke(C, wm_block_create_orphans_cleanup, purge_data, MEM_freeN);
return OPERATOR_FINISHED;
}
void OUTLINER_OT_orphans_cleanup(wmOperatorType *ot)
{
/* identifiers */
ot->idname = "OUTLINER_OT_orphans_cleanup";
ot->name = "Purge Unused Data...";
ot->description = "Remove unused data from this file";
/* callbacks */
Harley marked this conversation as resolved Outdated

Unused -> unused for the description

Unused -> unused for the description
ot->exec = outliner_orphans_cleanup_exec;
Harley marked this conversation as resolved
Review

Missing OPTYPE_UNDO I think? Otherwise, a comment explaining why it's not needed would be good to have.

Missing `OPTYPE_UNDO` I think? Otherwise, a comment explaining why it's not needed would be good to have.
/* flags */
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
}
static int outliner_orphans_manage_exec(bContext *C, wmOperator * /*op*/)
Harley marked this conversation as resolved Outdated

Would be better to adjust these default values to the available screen space? On some modern screens, 500x500 is a fairly small window...

Would be better to adjust these default values to the available screen space? On some modern screens, 500x500 is a fairly small window...
{
int width = 800;
int height = 500;
if (wm_get_screensize(&width, &height)) {
width /= 2;
height /= 2;
}
const rcti window_rect = {
/*xmin*/ 0,
/*xmax*/ width,
/*ymin*/ 0,
/*ymax*/ height,
};
if (WM_window_open(C,
IFACE_("Manage Unused Data"),
&window_rect,
SPACE_OUTLINER,
false,
false,
Harley marked this conversation as resolved
Review

Should return OPERATOR_CANCELLED if WM_window_open fails...

Should return `OPERATOR_CANCELLED` if `WM_window_open` fails...
true,
WIN_ALIGN_PARENT_CENTER,
nullptr,
nullptr) != nullptr)
{
SpaceOutliner *soutline = (SpaceOutliner *)CTX_wm_area(C)->spacedata.first;
soutline->outlinevis = SO_ID_ORPHANS;
return OPERATOR_FINISHED;
}
return OPERATOR_CANCELLED;
}
void OUTLINER_OT_orphans_manage(wmOperatorType *ot)
{
/* identifiers */
ot->idname = "OUTLINER_OT_orphans_manage";
ot->name = "Manage Unused Data...";
ot->description = "Open a window to manage unused data";
/* callbacks */
ot->exec = outliner_orphans_manage_exec;
/* flags */
ot->flag = OPTYPE_REGISTER;
}
/** \} */
} // namespace blender::ed::outliner

View File

@ -511,7 +511,9 @@ void OUTLINER_OT_keyingset_remove_selected(wmOperatorType *ot);
void OUTLINER_OT_drivers_add_selected(wmOperatorType *ot);
void OUTLINER_OT_drivers_delete_selected(wmOperatorType *ot);
void OUTLINER_OT_orphans_cleanup(wmOperatorType *ot);
void OUTLINER_OT_orphans_purge(wmOperatorType *ot);
void OUTLINER_OT_orphans_manage(wmOperatorType *ot);
/* `outliner_query.cc` */

View File

@ -60,6 +60,8 @@ void outliner_operatortypes()
WM_operatortype_append(OUTLINER_OT_drivers_delete_selected);
WM_operatortype_append(OUTLINER_OT_orphans_purge);
WM_operatortype_append(OUTLINER_OT_orphans_cleanup);
WM_operatortype_append(OUTLINER_OT_orphans_manage);
WM_operatortype_append(OUTLINER_OT_parent_drop);
WM_operatortype_append(OUTLINER_OT_parent_clear);