[Bf-blender-cvs] [b4f92bf7bc5] temp-ui-button-type-refactor: UI Code Quality: Use derived structs for search buttons and decorators

Julian Eisel noreply at git.blender.org
Fri Jun 5 14:22:47 CEST 2020


Commit: b4f92bf7bc59aaacb0baefc8c0ad5c6914c23dad
Author: Julian Eisel
Date:   Sun May 3 21:03:22 2020 +0200
Branches: temp-ui-button-type-refactor
https://developer.blender.org/rBb4f92bf7bc59aaacb0baefc8c0ad5c6914c23dad

UI Code Quality: Use derived structs for search buttons and decorators

The current on-size-fits-all `uiBut` creates quite a mess, where it's hard to
reason about which members are free for use, under which conditions they are
used and how. This tries to untangle that mess.
A draw-back is that many casts have to be done although this seems reasonable.

(I don't expect an in-depth review, but would like to have the general design
change reviewed first.)

A complication was that we sometimes change the button type after it's created.
So I had to add logic to reallocate the button for use with the new, possibly
derived struct. Ideally that wouldn't be needed, but for now that's what we have.
This is also something I'd like to have reviewed.

Differential Revision: https://developer.blender.org/D7610

===================================================================

M	source/blender/editors/include/UI_interface.h
M	source/blender/editors/interface/interface.c
M	source/blender/editors/interface/interface_anim.c
M	source/blender/editors/interface/interface_handlers.c
M	source/blender/editors/interface/interface_intern.h
M	source/blender/editors/interface/interface_layout.c
M	source/blender/editors/interface/interface_query.c
M	source/blender/editors/interface/interface_widgets.c

===================================================================

diff --git a/source/blender/editors/include/UI_interface.h b/source/blender/editors/include/UI_interface.h
index 1222aedbb72..230a8a2fa3d 100644
--- a/source/blender/editors/include/UI_interface.h
+++ b/source/blender/editors/include/UI_interface.h
@@ -378,6 +378,7 @@ typedef enum {
   UI_BTYPE_SEPR_SPACER = 56 << 9,
   /** Resize handle (resize uilist). */
   UI_BTYPE_GRIP = 57 << 9,
+  UI_BTYPE_DECORATOR = 58 << 9,
 } eButType;
 
 #define BUTTYPE (63 << 9)
@@ -535,7 +536,7 @@ typedef bool (*uiMenuStepFunc)(struct bContext *C, int direction, void *arg1);
 bool UI_but_has_tooltip_label(const uiBut *but);
 bool UI_but_is_tool(const uiBut *but);
 bool UI_but_is_utf8(const uiBut *but);
-#define UI_but_is_decorator(but) ((but)->func == ui_but_anim_decorate_cb)
+#define UI_but_is_decorator(but) ((but)->type == UI_BTYPE_DECORATOR)
 
 bool UI_block_is_empty_ex(const uiBlock *block, const bool skip_title);
 bool UI_block_is_empty(const uiBlock *block);
diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c
index 48c3e25a5ad..45e474238ca 100644
--- a/source/blender/editors/interface/interface.c
+++ b/source/blender/editors/interface/interface.c
@@ -1773,7 +1773,7 @@ void UI_block_end_ex(const bContext *C, uiBlock *block, const int xy[2], int r_x
     ui_but_anim_flag(but, (scene) ? scene->r.cfra : 0.0f);
     ui_but_override_flag(but);
     if (UI_but_is_decorator(but)) {
-      ui_but_anim_decorate_update_from_flag(but);
+      ui_but_anim_decorate_update_from_flag((uiButDecorator *)but);
     }
     ui_but_predefined_extra_operator_icons_add(but);
   }
@@ -2021,6 +2021,7 @@ int ui_but_is_pushed_ex(uiBut *but, double *value)
       case UI_BTYPE_HOTKEY_EVENT:
       case UI_BTYPE_KEY_EVENT:
       case UI_BTYPE_COLOR:
+      case UI_BTYPE_DECORATOR:
         is_push = -1;
         break;
       case UI_BTYPE_BUT_TOGGLE:
@@ -3218,7 +3219,7 @@ static void ui_set_but_soft_range(uiBut *but)
 /**
  * Free data specific to a certain button type.
  * For now just do in a switch-case, we could instead have a callback stored in #uiBut and set that
- * in #ui_but_alloc().
+ * in #ui_but_alloc_info().
  */
 static void ui_but_free_type_specific(uiBut *but)
 {
@@ -3745,6 +3746,10 @@ static void ui_but_alloc_info(const eButType type,
   bool has_custom_type = true;
 
   switch (type) {
+    case UI_BTYPE_DECORATOR:
+      alloc_size = sizeof(uiButDecorator);
+      alloc_str = "uiButDecorator";
+      break;
     case UI_BTYPE_TAB:
       alloc_size = sizeof(uiButTab);
       alloc_str = "uiButTab";
@@ -3969,6 +3974,7 @@ static uiBut *ui_def_but(uiBlock *block,
   if (ELEM(but->type,
            UI_BTYPE_BLOCK,
            UI_BTYPE_BUT,
+           UI_BTYPE_DECORATOR,
            UI_BTYPE_LABEL,
            UI_BTYPE_PULLDOWN,
            UI_BTYPE_ROUNDBOX,
diff --git a/source/blender/editors/interface/interface_anim.c b/source/blender/editors/interface/interface_anim.c
index 8c9768f523d..9dc0b459988 100644
--- a/source/blender/editors/interface/interface_anim.c
+++ b/source/blender/editors/interface/interface_anim.c
@@ -116,35 +116,41 @@ void ui_but_anim_flag(uiBut *but, float cfra)
   }
 }
 
-static uiBut *ui_but_anim_decorate_find_attached_button(uiBut *but_decorate)
+static uiBut *ui_but_anim_decorate_find_attached_button(uiButDecorator *but_decorate)
 {
   uiBut *but_iter = NULL;
 
-  BLI_assert(UI_but_is_decorator(but_decorate));
-  BLI_assert(but_decorate->rnasearchpoin.data && but_decorate->rnasearchprop);
+  BLI_assert(UI_but_is_decorator(&but_decorate->but));
+  BLI_assert(but_decorate->rnapoin.data && but_decorate->rnaprop);
 
-  LISTBASE_CIRCULAR_BACKWARD_BEGIN (&but_decorate->block->buttons, but_iter, but_decorate->prev) {
-    if (but_iter != but_decorate &&
-        ui_but_rna_equals_ex(but_iter,
-                             &but_decorate->rnasearchpoin,
-                             but_decorate->rnasearchprop,
-                             POINTER_AS_INT(but_decorate->custom_data))) {
+  LISTBASE_CIRCULAR_BACKWARD_BEGIN (
+      &but_decorate->but.block->buttons, but_iter, but_decorate->but.prev) {
+    if (but_iter != (uiBut *)but_decorate &&
+        ui_but_rna_equals_ex(
+            but_iter, &but_decorate->rnapoin, but_decorate->rnaprop, but_decorate->rnaindex)) {
       return but_iter;
     }
   }
-  LISTBASE_CIRCULAR_BACKWARD_END(&but_decorate->block->buttons, but_iter, but_decorate->prev);
+  LISTBASE_CIRCULAR_BACKWARD_END(
+      &but_decorate->but.block->buttons, but_iter, but_decorate->but.prev);
 
   return NULL;
 }
 
-void ui_but_anim_decorate_update_from_flag(uiBut *but)
+void ui_but_anim_decorate_update_from_flag(uiButDecorator *decorator_but)
 {
-  const uiBut *but_anim = ui_but_anim_decorate_find_attached_button(but);
+  if (!decorator_but->rnapoin.data || !decorator_but->rnaprop) {
+    /* Nothing to do. */
+    return;
+  }
+
+  const uiBut *but_anim = ui_but_anim_decorate_find_attached_button(decorator_but);
+  uiBut *but = &decorator_but->but;
 
   if (!but_anim) {
     printf("Could not find button with matching property to decorate (%s.%s)\n",
-           RNA_struct_identifier(but->rnasearchpoin.type),
-           RNA_property_identifier(but->rnasearchprop));
+           RNA_struct_identifier(decorator_but->rnapoin.type),
+           RNA_property_identifier(decorator_but->rnaprop));
     return;
   }
 
@@ -321,7 +327,7 @@ void ui_but_anim_paste_driver(bContext *C)
 void ui_but_anim_decorate_cb(bContext *C, void *arg_but, void *UNUSED(arg_dummy))
 {
   wmWindowManager *wm = CTX_wm_manager(C);
-  uiBut *but_decorate = arg_but;
+  uiButDecorator *but_decorate = arg_but;
   uiBut *but_anim = ui_but_anim_decorate_find_attached_button(but_decorate);
 
   if (!but_anim) {
@@ -329,7 +335,7 @@ void ui_but_anim_decorate_cb(bContext *C, void *arg_but, void *UNUSED(arg_dummy)
   }
 
   /* FIXME(campbell), swapping active pointer is weak. */
-  SWAP(struct uiHandleButtonData *, but_anim->active, but_decorate->active);
+  SWAP(struct uiHandleButtonData *, but_anim->active, but_decorate->but.active);
   wm->op_undo_depth++;
 
   if (but_anim->flag & UI_BUT_DRIVEN) {
@@ -353,6 +359,6 @@ void ui_but_anim_decorate_cb(bContext *C, void *arg_but, void *UNUSED(arg_dummy)
     WM_operator_properties_free(&props_ptr);
   }
 
-  SWAP(struct uiHandleButtonData *, but_anim->active, but_decorate->active);
+  SWAP(struct uiHandleButtonData *, but_anim->active, but_decorate->but.active);
   wm->op_undo_depth--;
 }
diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c
index 8df41cc5b8f..7a9aff80ed7 100644
--- a/source/blender/editors/interface/interface_handlers.c
+++ b/source/blender/editors/interface/interface_handlers.c
@@ -2079,6 +2079,7 @@ static void ui_apply_but(
   /* handle different types */
   switch (but->type) {
     case UI_BTYPE_BUT:
+    case UI_BTYPE_DECORATOR:
       ui_apply_but_BUT(C, but, data);
       break;
     case UI_BTYPE_TEXT:
@@ -7449,6 +7450,7 @@ static int ui_do_button(bContext *C, uiBlock *block, uiBut *but, const wmEvent *
 
   switch (but->type) {
     case UI_BTYPE_BUT:
+    case UI_BTYPE_DECORATOR:
       retval = ui_do_but_BUT(C, but, data, event);
       break;
     case UI_BTYPE_KEY_EVENT:
@@ -8335,7 +8337,7 @@ void UI_context_update_anim_flag(const bContext *C)
         ui_but_anim_flag(but, (scene) ? scene->r.cfra : 0.0f);
         ui_but_override_flag(but);
         if (UI_but_is_decorator(but)) {
-          ui_but_anim_decorate_update_from_flag(but);
+          ui_but_anim_decorate_update_from_flag((uiButDecorator *)but);
         }
 
         ED_region_tag_redraw(region);
diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h
index 200edca76b7..cf57a6ff457 100644
--- a/source/blender/editors/interface/interface_intern.h
+++ b/source/blender/editors/interface/interface_intern.h
@@ -244,9 +244,6 @@ struct uiBut {
   struct PropertyRNA *rnaprop;
   int rnaindex;
 
-  struct PointerRNA rnasearchpoin;
-  struct PropertyRNA *rnasearchprop;
-
   /* Operator data */
   struct wmOperatorType *optype;
   struct PointerRNA *opptr;
@@ -304,6 +301,15 @@ typedef struct uiButSearch {
   struct PropertyRNA *rnasearchprop;
 } uiButSearch;
 
+/** Derived struct for #UI_BTYPE_DECORATOR */
+typedef struct uiButDecorator {
+  uiBut but;
+
+  struct PointerRNA rnapoin;
+  struct PropertyRNA *rnaprop;
+  int rnaindex;
+} uiButDecorator;
+
 /**
  * Additional, superimposed icon for a button, invoking an operator.
  */
@@ -936,7 +942,7 @@ bool ui_but_anim_expression_create(uiBut *but, const char *str);
 void ui_but_anim_autokey(struct bContext *C, uiBut *but, struct Scene *scene, float cfra);
 
 void ui_but_anim_decorate_cb(struct bContext *C, void *arg_but, void *arg_dummy);
-void ui_but_anim_decorate_update_from_flag(uiBut *but);
+void ui_but_anim_decorate_update_from_flag(uiButDecorator *but);
 
 /* interface_query.c */
 bool ui_but_is_editable(const uiBut *but) ATTR_WARN_UNUSED_RESULT;
diff --git a/source/blender/editors/interface/interface_layout.c b/source/blender/editors/interface/interface_layout.c
index b78eb9f67bc..5c556a6f84b 100644
--- a/source/blender/editors/interface/interface_layout.c
+++ b/source/blender/editors/interface/interface_layout.c
@@ -2948,29 +2948,28 @@ void uiItemMContents(uiLayout *layout, const char *menuname)
 void uiItemDecoratorR_prop(uiLayout *layout, PointerRNA *ptr, PropertyRNA *prop, int index)
 {
   uiBlock *block = layout->root->block;
-  uiBut *but = NULL;
-
   uiLayout *col;
+
   UI_block_layout_set_current(block, layout);
   col = uiLayoutColumn(layout, false);
   col->space = 0;
   col->emboss = UI_EMBOSS_NONE;
 
   if (ELEM(NULL, ptr, prop) || !RNA_property_animateable(ptr, prop)) {
-    but = uiDefIconBut(block,
-                       UI_BTYPE_BUT,
-                       0,
-                       ICON_BLANK1,
-                       0,
-                       0,
-                       UI_UNIT_X,
-                       UI_UNIT_Y,
-                       NULL,
-                       0.0,
-                       0.0,
-                       0.0,
-                       0.0,
-                       "");
+    

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list