Refactor: Solve invalid icon-id enum values, use type alias

Some code attempted to use `BIFIconID` instead of `int` to pass around
icon-ids. Problem is, that this is just a subset of the allowed ids,
more icons may be created at runtime and extend the range of valid
icon-ids. Such icons could give runtime warning prints.

Idea is to use a `using BIFIconID = int;` instead. This way there is
still a descriptive type name, while the whole dynamic range of possible
icon-ids is supported.

Additionally multiple `using BIFIconID = int;` declarations are valid,
so we can place these in multiple headers and use the type name in APIs
instead of just `int`, whithout having to include a single header
defining them. A type mismatch (one instance differs from the others)
will result in a compiler error.

Pull Request: https://projects.blender.org/blender/blender/pulls/111052
This commit is contained in:
Julian Eisel 2023-08-14 12:06:35 +02:00 committed by Julian Eisel
parent a3a0a33978
commit 5dcf704324
9 changed files with 34 additions and 25 deletions

View File

@ -9,6 +9,7 @@
#pragma once
#include "BLI_sys_types.h"
#include "BLI_utildefines.h"
struct bTheme;
@ -18,12 +19,21 @@ struct bTheme;
#define DEF_ICON_COLOR(name) ICON_##name,
#define DEF_ICON_BLANK(name) ICON_BLANK_##name,
enum BIFIconID {
/**
* Builtin icons with a compile-time icon-id. Dynamically created icons such as preview image
* icons get a dynamic icon-id <= #BIFICONID_LAST_STATIC, #BIFIconID can hold both.
*/
enum BIFIconID_Static {
/* ui */
#include "UI_icons.hh"
BIFICONID_LAST,
BIFICONID_LAST_STATIC,
};
/** Type that fits all compile time and dynamic icon-ids. */
using BIFIconID = int;
BLI_STATIC_ASSERT(sizeof(BIFIconID_Static) <= sizeof(BIFIconID),
"Expected all builtin icon IDs to fit into `BIFIconID`");
#define BIFICONID_FIRST (ICON_NONE)
/* use to denote intentionally unset theme color */

View File

@ -1691,7 +1691,7 @@ static PointerRNA *ui_but_extra_operator_icon_add_ptr(uiBut *but,
{
uiButExtraOpIcon *extra_op_icon = MEM_new<uiButExtraOpIcon>(__func__);
extra_op_icon->icon = (BIFIconID)icon;
extra_op_icon->icon = icon;
extra_op_icon->optype_params = MEM_cnew<wmOperatorCallParams>(__func__);
extra_op_icon->optype_params->optype = optype;
extra_op_icon->optype_params->opptr = MEM_cnew<PointerRNA>(__func__);
@ -1847,7 +1847,7 @@ static void ui_but_predefined_extra_operator_icons_add(uiBut *but)
return;
}
}
ui_but_extra_operator_icon_add_ptr(but, optype, WM_OP_INVOKE_DEFAULT, int(icon));
ui_but_extra_operator_icon_add_ptr(but, optype, WM_OP_INVOKE_DEFAULT, icon);
}
}
@ -3857,7 +3857,7 @@ static void ui_but_update_ex(uiBut *but, const bool validate)
const size_t slen = strlen(item.name);
ui_but_string_free_internal(but);
ui_but_string_set_internal(but, item.name, slen);
but->icon = (BIFIconID)item.icon;
but->icon = item.icon;
}
}
}
@ -4270,7 +4270,7 @@ void ui_def_but_icon(uiBut *but, const int icon, const int flag)
icon,
(flag & UI_BUT_ICON_PREVIEW) != 0);
}
but->icon = (BIFIconID)icon;
but->icon = icon;
but->flag |= flag;
if (but->str && but->str[0]) {

View File

@ -39,16 +39,15 @@ void context_path_add_generic(Vector<ContextPathItem> &path,
RNA_struct_name_get_alloc(&rna_ptr, name, sizeof(name), nullptr);
/* Use a blank icon by default to check whether to retrieve it automatically from the type. */
const BIFIconID icon = icon_override == ICON_NONE ?
static_cast<BIFIconID>(RNA_struct_ui_icon(rna_ptr.type)) :
icon_override;
const BIFIconID icon = icon_override == ICON_NONE ? RNA_struct_ui_icon(rna_ptr.type) :
icon_override;
if (&rna_type == &RNA_NodeTree) {
ID *id = (ID *)ptr;
path.append({name, int(icon), ID_REAL_USERS(id)});
path.append({name, icon, ID_REAL_USERS(id)});
}
else {
path.append({name, int(icon), 1});
path.append({name, icon, 1});
}
}

View File

@ -299,7 +299,7 @@ static bool menu_items_to_ui_button(MenuSearch_Item *item, uiBut *but)
*drawstr_sep = '\0';
}
but->icon = (BIFIconID)item->icon;
but->icon = item->icon;
but->str = but->strdata;
}

View File

@ -2313,7 +2313,7 @@ static void widget_draw_text_icon(const uiFontStyle *fstyle,
/* Big previews with optional text label below */
if (but->flag & UI_BUT_ICON_PREVIEW && ui_block_is_menu(but->block)) {
const BIFIconID icon = BIFIconID(ui_but_icon(but));
const BIFIconID icon = ui_but_icon(but);
int icon_size = BLI_rcti_size_y(rect);
int text_size = 0;
@ -2350,7 +2350,7 @@ static void widget_draw_text_icon(const uiFontStyle *fstyle,
}
#endif
const BIFIconID icon = BIFIconID(ui_but_icon(but));
const BIFIconID icon = ui_but_icon(but);
const int icon_size_init = is_tool ? ICON_DEFAULT_HEIGHT_TOOLBAR : ICON_DEFAULT_HEIGHT;
const float icon_size = icon_size_init / (but->block->aspect * UI_INV_SCALE_FAC);
const float icon_padding = 2 * UI_SCALE_FAC;
@ -5632,10 +5632,10 @@ void ui_draw_preview_item_stateless(const uiFontStyle *fstyle,
}
GPU_blend(GPU_BLEND_ALPHA);
if (draw_as_icon) {
widget_draw_icon_centered(BIFIconID(iconid), 1.0f, alpha, rect, text_col);
widget_draw_icon_centered(iconid, 1.0f, alpha, rect, text_col);
}
else {
widget_draw_preview(BIFIconID(iconid), alpha, rect);
widget_draw_preview(iconid, alpha, rect);
}
GPU_blend(GPU_BLEND_NONE);

View File

@ -2446,7 +2446,7 @@ static BIFIconID tree_element_get_icon_from_id(const ID *id)
return ICON_FILE_TEXT;
}
/* Helps distinguish text-based formats like the file-browser does. */
return (BIFIconID)ED_file_extension_icon(text->filepath);
return ED_file_extension_icon(text->filepath);
}
case ID_GR:
return ICON_OUTLINER_COLLECTION;
@ -2493,7 +2493,7 @@ static BIFIconID tree_element_get_icon_from_id(const ID *id)
case ID_NT: {
const bNodeTree *ntree = (bNodeTree *)id;
const bNodeTreeType *ntreetype = ntree->typeinfo;
return (BIFIconID)ntreetype->ui_icon;
return ntreetype->ui_icon;
}
case ID_MC:
return ICON_SEQUENCE;
@ -3366,7 +3366,7 @@ static void outliner_draw_tree_element(bContext *C,
if (ELEM(tselem->type, TSE_SOME_ID, TSE_LAYER_COLLECTION) ||
(te_rna_struct && RNA_struct_is_ID(te_rna_struct->getPointerRNA().type)))
{
const BIFIconID lib_icon = (BIFIconID)UI_icon_from_library(tselem->id);
const BIFIconID lib_icon = UI_icon_from_library(tselem->id);
if (lib_icon != ICON_NONE) {
UI_icon_draw_alpha(
float(startx) + offsx + 2 * ufac, float(*starty) + 2 * ufac, lib_icon, alpha_fac);

View File

@ -256,7 +256,7 @@ StringRefNull TreeElementOverridesPropertyOperation::getOverrideOperationLabel()
std::optional<BIFIconID> TreeElementOverridesPropertyOperation::getIcon() const
{
if (const std::optional<PointerRNA> col_item_ptr = get_collection_ptr()) {
return (BIFIconID)RNA_struct_ui_icon(col_item_ptr->type);
return RNA_struct_ui_icon(col_item_ptr->type);
}
return {};
@ -451,7 +451,7 @@ void OverrideRNAPathTreeBuilder::ensure_entire_collection(
static BIFIconID get_property_icon(PointerRNA &ptr, PropertyRNA &prop)
{
BIFIconID icon = (BIFIconID)RNA_property_ui_icon(&prop);
BIFIconID icon = RNA_property_ui_icon(&prop);
if (icon) {
return icon;
}
@ -460,7 +460,7 @@ static BIFIconID get_property_icon(PointerRNA &ptr, PropertyRNA &prop)
* #Object.modifiers property). */
if (RNA_property_type(&prop) == PROP_COLLECTION) {
const StructRNA *coll_ptr_type = RNA_property_pointer_type(&ptr, &prop);
icon = (BIFIconID)RNA_struct_ui_icon(coll_ptr_type);
icon = RNA_struct_ui_icon(coll_ptr_type);
if (icon != ICON_DOT) {
return icon;
}
@ -503,7 +503,7 @@ TreeElement &OverrideRNAPathTreeBuilder::ensure_label_element_for_ptr(TreeElemen
TSE_GENERIC_LABEL,
index++);
TreeElementLabel *te_label = tree_element_cast<TreeElementLabel>(new_te);
te_label->setIcon((BIFIconID)RNA_struct_ui_icon(ptr.type));
te_label->setIcon(RNA_struct_ui_icon(ptr.type));
MEM_delete(dyn_name);

View File

@ -759,7 +759,7 @@ static void rna_Brush_reset_icon(Brush *br)
return;
}
if (id->icon_id >= BIFICONID_LAST) {
if (id->icon_id >= BIFICONID_LAST_STATIC) {
BKE_icon_id_delete(id);
BKE_previewimg_id_free(id);
}

View File

@ -257,7 +257,7 @@ void WM_init(bContext *C, int argc, const char **argv)
/* Init icons before reading .blend files for preview icons, which can
* get triggered by the depsgraph. This is also done in background mode
* for scripts that do background processing with preview icons. */
BKE_icons_init(BIFICONID_LAST);
BKE_icons_init(BIFICONID_LAST_STATIC);
/* Reports can't be initialized before the window-manager,
* but keep before file reading, since that may report errors */