Commit: d204830107ef7f6f5802543cbb4cad7489f5507c Author: Julian Eisel Date: Fri Feb 3 16:12:14 2023 +0100 Branches: master https://developer.blender.org/rBd204830107ef7f6f5802543cbb4cad7489f5507c
UI: Make uiBut safe for non-trivial construction No user-visible changes expected. Essentially, this makes it possible to use C++ types like `std::function` inside `uiBut`. This has plenty of benefits, for example this should help significantly reducing unsafe `void *` use (since a `std::function` can hold arbitrary data while preserving types). ---- I wanted to use a non-trivially-constructible C++ type (`std::function`) inside `uiBut`. But this would mean we can't use `MEM_cnew()` like allocation anymore. Rather than writing worse code, allow non-trivial construction for `uiBut`. Member-initializing all members is annoying since there are so many, but rather safe than sorry. As we use more C++ types (e.g. convert callbacks to use `std::function`), this should become less since they initialize properly on default construction. Also use proper C++ inheritance for `uiBut` subtypes, the old way to allocate based on size isn't working anymore. Differential Revision: https://developer.blender.org/D17164 Reviewed by: Hans Goudey =================================================================== M source/blender/editors/include/UI_interface.h M source/blender/editors/interface/interface.cc M source/blender/editors/interface/interface_anim.cc M source/blender/editors/interface/interface_context_menu.cc M source/blender/editors/interface/interface_handlers.cc M source/blender/editors/interface/interface_intern.hh M source/blender/editors/interface/interface_layout.cc M source/blender/editors/interface/interface_ops.cc M source/blender/editors/interface/interface_region_color_picker.cc M source/blender/editors/interface/interface_region_search.cc M source/blender/editors/interface/interface_templates.cc M source/blender/editors/interface/interface_widgets.cc M source/blender/editors/interface/views/grid_view.cc M source/blender/editors/interface/views/tree_view.cc =================================================================== diff --git a/source/blender/editors/include/UI_interface.h b/source/blender/editors/include/UI_interface.h index db3f0dc41fb..cf2c0ea2ebb 100644 --- a/source/blender/editors/include/UI_interface.h +++ b/source/blender/editors/include/UI_interface.h @@ -322,6 +322,8 @@ enum { * - bit 9-15: button type (now 6 bits, 64 types) */ typedef enum { + UI_BUT_POIN_NONE = 0, + UI_BUT_POIN_CHAR = 32, UI_BUT_POIN_SHORT = 64, UI_BUT_POIN_INT = 96, diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc index 2a21356b9aa..bf1f276d702 100644 --- a/source/blender/editors/interface/interface.cc +++ b/source/blender/editors/interface/interface.cc @@ -3474,7 +3474,7 @@ static void ui_but_free(const bContext *C, uiBut *but) BLI_assert(UI_butstore_is_registered(but->block, but) == false); - MEM_freeN(but); + MEM_delete(but); } void UI_block_free(const bContext *C, uiBlock *block) @@ -3975,89 +3975,57 @@ void ui_block_cm_to_display_space_v3(uiBlock *block, float pixel[3]) IMB_colormanagement_scene_linear_to_display_v3(pixel, display); } -static void ui_but_alloc_info(const eButType type, - size_t *r_alloc_size, - const char **r_alloc_str, - bool *r_has_custom_type) +/** + * Factory function: Allocate button and set #uiBut.type. + */ +static uiBut *ui_but_new(const eButType type) { - size_t alloc_size; - const char *alloc_str; - bool has_custom_type = true; + uiBut *but = nullptr; switch (type) { case UI_BTYPE_NUM: - alloc_size = sizeof(uiButNumber); - alloc_str = "uiButNumber"; + but = MEM_new<uiButNumber>("uiButNumber"); break; case UI_BTYPE_COLOR: - alloc_size = sizeof(uiButColor); - alloc_str = "uiButColor"; + but = MEM_new<uiButColor>("uiButColor"); break; case UI_BTYPE_DECORATOR: - alloc_size = sizeof(uiButDecorator); - alloc_str = "uiButDecorator"; + but = MEM_new<uiButDecorator>("uiButDecorator"); break; case UI_BTYPE_TAB: - alloc_size = sizeof(uiButTab); - alloc_str = "uiButTab"; + but = MEM_new<uiButTab>("uiButTab"); break; case UI_BTYPE_SEARCH_MENU: - alloc_size = sizeof(uiButSearch); - alloc_str = "uiButSearch"; + but = MEM_new<uiButSearch>("uiButSearch"); break; case UI_BTYPE_PROGRESS_BAR: - alloc_size = sizeof(uiButProgressbar); - alloc_str = "uiButProgressbar"; + but = MEM_new<uiButProgressbar>("uiButProgressbar"); break; case UI_BTYPE_HSVCUBE: - alloc_size = sizeof(uiButHSVCube); - alloc_str = "uiButHSVCube"; + but = MEM_new<uiButHSVCube>("uiButHSVCube"); break; case UI_BTYPE_COLORBAND: - alloc_size = sizeof(uiButColorBand); - alloc_str = "uiButColorBand"; + but = MEM_new<uiButColorBand>("uiButColorBand"); break; case UI_BTYPE_CURVE: - alloc_size = sizeof(uiButCurveMapping); - alloc_str = "uiButCurveMapping"; + but = MEM_new<uiButCurveMapping>("uiButCurveMapping"); break; case UI_BTYPE_CURVEPROFILE: - alloc_size = sizeof(uiButCurveProfile); - alloc_str = "uiButCurveProfile"; + but = MEM_new<uiButCurveProfile>("uiButCurveProfile"); break; case UI_BTYPE_HOTKEY_EVENT: - alloc_size = sizeof(uiButHotkeyEvent); - alloc_str = "uiButHotkeyEvent"; + but = MEM_new<uiButHotkeyEvent>("uiButHotkeyEvent"); break; case UI_BTYPE_VIEW_ITEM: - alloc_size = sizeof(uiButViewItem); - alloc_str = "uiButViewItem"; + but = MEM_new<uiButViewItem>("uiButViewItem"); break; default: - alloc_size = sizeof(uiBut); - alloc_str = "uiBut"; - has_custom_type = false; + but = MEM_new<uiBut>("uiBut"); break; } - if (r_alloc_size) { - *r_alloc_size = alloc_size; - } - if (r_alloc_str) { - *r_alloc_str = alloc_str; - } - if (r_has_custom_type) { - *r_has_custom_type = has_custom_type; - } -} - -static uiBut *ui_but_alloc(const eButType type) -{ - size_t alloc_size; - const char *alloc_str; - ui_but_alloc_info(type, &alloc_size, &alloc_str, nullptr); - - return static_cast<uiBut *>(MEM_callocN(alloc_size, alloc_str)); + but->type = type; + return but; } uiBut *ui_but_change_type(uiBut *but, eButType new_type) @@ -4067,46 +4035,41 @@ uiBut *ui_but_change_type(uiBut *but, eButType new_type) return but; } - size_t alloc_size; - const char *alloc_str; uiBut *insert_after_but = but->prev; - bool new_has_custom_type, old_has_custom_type; /* Remove old button address */ BLI_remlink(&but->block->buttons, but); - ui_but_alloc_info(but->type, nullptr, nullptr, &old_has_custom_type); - ui_but_alloc_info(new_type, &alloc_size, &alloc_str, &new_has_custom_type); + const uiBut *old_but_ptr = but; + /* Button may have pointer to a member within itself, this will have to be updated. */ + const bool has_str_ptr_to_self = but->str == but->strdata; + const bool has_poin_ptr_to_self = but->poin == (char *)but; - if (new_has_custom_type || old_has_custom_type) { - const uiBut *old_but_ptr = but; - /* Button may have pointer to a member within itself, this will have to be updated. */ - const bool has_str_ptr_to_self = but->str == but->strdata; - const bool has_poin_ptr_to_self = but->poin == (char *)but; - - but = static_cast<uiBut *>(MEM_recallocN_id(but, alloc_size, alloc_str)); - but->type = new_type; - if (has_str_ptr_to_self) { - but->str = but->strdata; - } - if (has_poin_ptr_to_self) { - but->poin = (char *)but; - } + /* Copy construct button with the new type. */ + but = ui_but_new(new_type); + *but = *old_but_ptr; + if (has_str_ptr_to_self) { + but->str = but->strdata; + } + if (has_poin_ptr_to_self) { + but->poin = (char *)but; + } - BLI_insertlinkafter(&but->block->buttons, insert_after_but, but); + BLI_insertlinkafter(&but->block->buttons, insert_after_but, but); - if (but->layout) { - const bool found_layout = ui_layout_replace_but_ptr(but->layout, old_but_ptr, but); - BLI_assert(found_layout); - UNUSED_VARS_NDEBUG(found_layout); - ui_button_group_replace_but_ptr(uiLayoutGetBlock(but->layout), old_but_ptr, but); - } + if (but->layout) { + const bool found_layout = ui_layout_replace_but_ptr(but->layout, old_but_ptr, but); + BLI_assert(found_layout); + UNUSED_VARS_NDEBUG(found_layout); + ui_button_group_replace_but_ptr(uiLayoutGetBlock(but->layout), old_but_ptr, but); + } #ifdef WITH_PYTHON - if (UI_editsource_enable_check()) { - UI_editsource_but_replace(old_but_ptr, but); - } -#endif + if (UI_editsource_enable_check()) { + UI_editsource_but_replace(old_but_ptr, but); } +#endif + + MEM_delete(old_but_ptr); return but; } @@ -4152,14 +4115,11 @@ static uiBut *ui_def_but(uiBlock *block, } } - uiBut *but = ui_but_alloc((eButType)(type & BUTTYPE)); + uiBut *but = ui_but_new((eButType)(type & BUTTYPE)); - but->type = (eButType)(type & BUTTYPE); but->pointype = (eButPointerType)(type & UI_BUT_POIN_TYPES); but->bit = type & UI_BUT_POIN_BIT; but->bitnr = type & 31; - but->icon = ICON_NONE; - but->iconadd = 0; but->retval = retval; @@ -4180,7 +4140,6 @@ static uiBut *ui_def_but(uiBlock *block, but->disabled_info = block->lockstr; but->emboss = block->emboss; - but->pie_dir = UI_RADIAL_NONE; but->block = block; /* pointer back, used for front-buffer status, and picker. */ @@ -6310,7 +6269,7 @@ void UI_but_func_search_set(uiBut *but, if (search_exec_fn) { #ifdef DEBUG - if (search_but->but.func) { + if (but->func) { /* watch this, can be cause of much confusion, see: T47691 */ printf("%s: warning, overwriting button callback with search function callback!\n", __func__); diff --git a/source/blender/editors/interface/interface_anim.cc b/source/blender/editors/interface/interface_anim.cc index a8a4faf622f..7a1c46640d2 100644 --- a/source/blender/editors/interface/interface_anim.cc +++ b/source/blender/editors/interface/interface_anim.cc @@ -113,41 +113,38 @@ void ui_but_anim_flag(uiBut *but, const AnimationEvalContext *anim_eval_context) } } -static uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate) +static uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but) { uiBut *but_iter = nullptr; - BLI_assert(UI_but_is_decorator(&but_decorate->but)); - BLI_assert(but_decorate->rnapoin.data && but_decorate->rnaprop); + BLI_assert(UI_but_is_decorator(but)); + BLI_assert(but->decorated_rnapoin.data && but->decorated_rnaprop); - LISTBASE_CIRCULAR_BACKWARD_BEGIN ( - uiBut *, &but_decorate->but.block->buttons, but_iter, but_decorate->but.prev) { - if (but_iter != (uiBut *)but_decorate && + LISTBASE_CIRCULAR_BACKWARD_BEGIN (uiBut *, &but->block->buttons, but_iter, but->prev) { + if (but_iter != but && ui_but_rna_equals_ex( - but_iter, &but_decorate->rnapoin, but_decorate->rnaprop, but_decorate->rnaindex)) { + but_iter, &but->decorated_rnapoin, but->decorated_rnaprop, but->decorated_rnaindex)) { return but_iter; } } - LISTBASE_CIRCULAR_BACKWARD_END( - uiBut *, &but_decorate->but.block->buttons, but_iter, but_decorate->but.prev); + LISTBASE_CIRCULAR_BACKWARD_END(uiBut *, &but->block->buttons, but_iter, but->prev); return nullptr; } -void ui_but_anim_decorate_update_from_flag(uiButDecorator *decorator_but) +void ui_but_anim_decorate_update_from_flag(uiButDecorator *but) { - if (!decorator_but->rnapoin.data || !decorator_but->rnaprop) { + if (!but->decorated_rnapoin.data || !but->decorated_rnaprop) { /* Nothing to do. */ return; } - const uiBut *but_anim = ui_but_anim_decorate_find_attached_button(decorator_but); - uiBut *but = &decorator_but->but; + const uiBut *but_anim = ui_but_anim_decorate_find_attached_button(but); if (!but_anim) { printf("Could not find button with matching property to decorate (%s.%s)\n", - RNA_struct_identifier(decorator_but->rnap @@ Diff output truncated at 10240 characters. @@ _______________________________________________ Bf-blender-cvs mailing list Bf-blender-cvs@blender.org List details, subscription details or unsubscribe: https://lists.blender.org/mailman/listinfo/bf-blender-cvs
