blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 779 Projects 19 Wiki Activity

UI: uiBut Indeterminate State #108210

Merged
Julian Eisel merged 15 commits from Harley/blender:Indeterminate into main 2023-07-17 19:37:24 +02:00
Conversation 5 Commits 15 Files Changed 4 +13 -12
1 changed files with 13 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1fe0b4456c - Show all commits

25
source/blender/editors/interface/interface.cc
View File

@ -2304,7 +2304,8 @@ int ui_but_is_pushed(uiBut *but)
static void ui_but_update_select_flag(uiBut *but, double *value)
{
switch (ui_but_is_pushed_ex(but, value)) {
switch (ui_but_is_pushed_ex(but, value) && (!(but->drawflag & UI_BUT_INDETERMINATE)))
{
case true:
but->flag |= UI_SELECT;
break;
@ -3785,11 +3786,15 @@ static void ui_but_build_drawstr_int(uiBut *but, int value)
*/
static void ui_but_update_ex(uiBut *but, const bool validate)
{
if (but->type != UI_BTYPE_LABEL) {
but->drawflag |= UI_BUT_INDETERMINATE;
}
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-06-09 16:52:36 +02:00
Outdated
Review
Copy Link

emdash isn't really useful either, this should be more descriptive. Would suggest something like UI_VALUE_INDETERMINATE_CHAR.

`emdash` isn't really useful either, this should be more descriptive. Would suggest something like `UI_VALUE_INDETERMINATE_CHAR`.
/* if something changed in the button */
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-07-04 15:21:21 +02:00
Outdated
Review
Copy Link

Would put this in UI_interface.h, together with other defines like UI_SEP_CHAR_S.

Would put this in `UI_interface.h`, together with other defines like `UI_SEP_CHAR_S`.
double value = UI_BUT_VALUE_UNSET;
/* Use emdash in place of text when in indeterminate state. */
const char *emdash = "\u2014";
const char *UI_VALUE_INDETERMINATE_CHAR = "\u2014";
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-07-04 17:19:45 +02:00
Outdated
Review
Copy Link

I still can't see an explanation on why this is done, only what it does. Testing the patch, when I simply enable UI_BUT_INDETERMINATE for all buttons, I get a whole bunch of interaction issues/glitches. That is because UI_SELECT is supposed to impact the behavior, not just the drawing.

I guess this should only affect drawing (it's controlled by a draw flag after all), so this should probably be done in ui_draw_but(). At the end of that we already do similar things to safely manipulate flags for drawing.

I still can't see an explanation on _why_ this is done, only what it does. Testing the patch, when I simply enable `UI_BUT_INDETERMINATE` for all buttons, I get a whole bunch of interaction issues/glitches. That is because `UI_SELECT` is supposed to impact the behavior, not just the drawing. I guess this should only affect drawing (it's controlled by a draw flag after all), so this should probably be done in `ui_draw_but()`. At the end of that we already do similar things to safely manipulate flags for drawing.
ui_but_update_select_flag(but, &value);
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-06-05 12:20:23 +02:00
Outdated
Review
Copy Link

I'd rather avoid flags that silently change other flags. Why exactly is this needed?

I'd rather avoid flags that silently change other flags. Why exactly is this needed?
@ -3800,13 +3805,6 @@ static void ui_but_update_ex(uiBut *but, const bool validate)
}
}
if (but->drawflag & UI_BUT_INDETERMINATE) {
/* Center text. */
but->drawflag &= ~(UI_BUT_TEXT_LEFT | UI_BUT_TEXT_RIGHT);
/* Do not show toggles as selected. */
but->flag &= ~(UI_SELECT);
}
/* test for min and max, icon sliders, etc */
switch (but->type) {
case UI_BTYPE_NUM:
@ -3872,7 +3870,8 @@ static void ui_but_update_ex(uiBut *but, const bool validate)
}
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-06-05 12:23:24 +02:00
Outdated
Review
Copy Link

It's not clear what \u2014 is, better make this a named constant.

It's not clear what `\u2014` is, better make this a named constant.
}
if (but->drawflag & UI_BUT_INDETERMINATE) {
STRNCPY(but->drawstr, emdash);
STRNCPY(but->drawstr, UI_VALUE_INDETERMINATE_CHAR);
but->drawflag &= ~(UI_BUT_TEXT_LEFT | UI_BUT_TEXT_RIGHT);
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-07-04 17:28:11 +02:00
Outdated
Review
Copy Link

Same here, not sure if we should permanently modify the drawing flags here. Can be done just before drawing.

Actually, let's not even override the draw-string here, and just do that in widget_draw_text() by setting drawstr to UI_VALUE_INDETERMINATE_CHAR for specific button types.

Same here, not sure if we should permanently modify the drawing flags here. Can be done just before drawing. Actually, let's not even override the draw-string here, and just do that in `widget_draw_text()` by setting `drawstr` to `UI_VALUE_INDETERMINATE_CHAR` for specific button types.
}
else {
STRNCPY(but->drawstr, but->str);
@ -3887,7 +3886,8 @@ static void ui_but_update_ex(uiBut *but, const bool validate)
}
UI_GET_BUT_VALUE_INIT(but, value);
if (but->drawflag & UI_BUT_INDETERMINATE) {
STRNCPY(but->drawstr, emdash);
STRNCPY(but->drawstr, UI_VALUE_INDETERMINATE_CHAR);
but->drawflag &= ~(UI_BUT_TEXT_LEFT | UI_BUT_TEXT_RIGHT);
}
else if (ui_but_is_float(but)) {
ui_but_build_drawstr_float(but, value);
@ -3913,7 +3913,8 @@ static void ui_but_update_ex(uiBut *but, const bool validate)
case UI_BTYPE_SEARCH_MENU:
if (!but->editstr) {
if (but->drawflag & UI_BUT_INDETERMINATE) {
STRNCPY(but->drawstr, emdash);
STRNCPY(but->drawstr, UI_VALUE_INDETERMINATE_CHAR);
but->drawflag &= ~(UI_BUT_TEXT_LEFT | UI_BUT_TEXT_RIGHT);
}
else {
char str[UI_MAX_DRAW_STR];