popup menus behave poorly when they have not enough width for all their columns #39234

New Issue

Alexandros Alexandrou · 2014-03-17T20:37:34+01:00

2014-03-17 20:37:34 +01:00

System Information
Ubuntu 12.04

Blender Version
Broken: 2.70 RC 2

Short description of error
When "Undo History" popup is filled, some items are not shown because of insufficient screen space. I don't think it is a bug, but just an inconsistency.

Probable solution
In case items need more space than screen can provide them, a slider or an arrow could be added, so remaining items can be seen and selected.

Exact steps for others to reproduce the error

Set "Undo Steps" in "User Preferences" to 64.
Make 64 steps, of which at least one has a long name (so the problem is visible).
Go to "Undo History" and you will see the following.

**System Information** Ubuntu 12.04 **Blender Version** Broken: 2.70 RC 2 **Short description of error** When "Undo History" popup is filled, some items are not shown because of insufficient screen space. I don't think it is a bug, but just an inconsistency. **Probable solution** In case items need more space than screen can provide them, a slider or an arrow could be added, so remaining items can be seen and selected. **Exact steps for others to reproduce the error** 1. Set "Undo Steps" in "User Preferences" to 64. 2. Make 64 steps, of which at least one has a long name (so the problem is visible). 3. Go to "Undo History" and you will see the following. ![screen2.png](https://archive.blender.org/developer/F81621/screen2.png)

Alexandros Alexandrou commented

2014-03-17 20:37:34 +01:00

Changed status to: 'Open'

Alexandros Alexandrou commented

2014-03-17 20:37:34 +01:00

Added subscriber: @alex1

Bastien Montagne commented

2014-03-18 15:32:16 +01:00

Added subscriber: @mont29

Bastien Montagne commented

2014-03-18 15:41:04 +01:00

Can’t reproduce such issue error here, I just get up/down arrows…

What is your screen definition? looks very small (I tried here with a Blender window less than 800*600 and still could show the four columns, even with a quite long op name).

Can’t reproduce such issue error here, I just get up/down arrows… What is your screen definition? looks *very* small (I tried here with a Blender window less than 800*600 and still could show the four columns, even with a quite long op name).

Alexandros Alexandrou commented

2014-03-18 19:23:04 +01:00

My screen resolution is 1280x800.

Sure, up/down arrows appear on menus with height exceeding Blender window height. But such arrows do not appear on "Undo History" popup, which was what I had pointed out. In fact, some items can't be accessed and an arrow or a slider is needed there.

My screen resolution is 1280x800. Sure, up/down arrows appear on menus with height exceeding Blender window height. But such arrows do not appear on "Undo History" popup, which was what I had pointed out. In fact, some items can't be accessed and an arrow or a slider is needed there.

Bastien Montagne commented

2014-03-19 20:28:33 +01:00

Ok, could reproduce it by setting a much bigger DPI size… We might enhance behavior of that kind of popup in two ways, imho:

Do not set same width for all columns!
Reduce number of columns when there is not enough width.

Will have a look at what we can do here.

Ok, could reproduce it by setting a much bigger DPI size… We might enhance behavior of that kind of popup in two ways, imho: * Do not set same width for all columns! * Reduce number of columns when there is not enough width. Will have a look at what we can do here.

Bastien Montagne self-assigned this 2014-03-19 20:28:41 +01:00

Bastien Montagne changed title from ~~When "Undo History" popup is filled, some items are hidden~~ to popup menus behave poorly when they have not enough width for all their columns

2014-03-19 20:29:41 +01:00

Bastien Montagne commented

2014-04-14 20:17:48 +02:00

Added subscriber: @brecht

Bastien Montagne commented

2014-04-14 20:17:48 +02:00

Following patch "fixes" the issue by:

Not having constant width for all columns, but adapt each to its content's width;
Adapting undo' menu height to undo list length (so that we never have more than three columns).

It is still possible to get issues in extreme cases (small screen, high DPI size, long op names), but this should now be rare corner cases.

I also fixed a minor glitch with undo menu (first column had one item less than the others…).

@brecht would need your green light here before I commit, since column width change will also affect menus like modifiers or node sockets ones (imho it’s better now, wastes less space, but…). :)

P37: Fix #39234

diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c
index 7e6e00b..5622288 100644
--- a/source/blender/editors/interface/interface.c
+++ b/source/blender/editors/interface/interface.c
@@ -207,43 +207,41 @@ void ui_block_translate(uiBlock *block, int x, int y)
 static void ui_text_bounds_block(uiBlock *block, float offset)
 {
        uiStyle *style = UI_GetStyle();
-       uiBut *bt;
-       int i = 0, j, x1addval = offset, nextcol;
-       int lastcol = 0, col = 0;
-       
+       uiBut *bt, *init_col_bt, *col_bt;
+       int i = 0, j, x1addval = offset;
+
        uiStyleFontSet(&style->widget);
-       
-       for (bt = block->buttons.first; bt; bt = bt->next) {
+
+       for (init_col_bt = bt = block->buttons.first; bt; bt = bt->next) {
                if (!ELEM(bt->type, SEPR, SEPRLINE)) {
                        j = BLF_width(style->widget.uifont_id, bt->drawstr, sizeof(bt->drawstr));
 
-                       if (j > i) i = j;
+                       if (j > i)
+                               i = j;
                }
 
-               if (bt->next && bt->rect.xmin < bt->next->rect.xmin)
-                       lastcol++;
-       }
+               if (bt->next && bt->rect.xmin < bt->next->rect.xmin) {
+                       /* End of this column, and it’s not the last one. */
+                       for (col_bt = init_col_bt; col_bt->prev != bt; col_bt = col_bt->next) {
+                               col_bt->rect.xmin = x1addval;
+                               col_bt->rect.xmax = x1addval + i + block->bounds;
 
-       /* cope with multi collumns */
-       bt = block->buttons.first;
-       while (bt) {
-               nextcol = (bt->next && bt->rect.xmin < bt->next->rect.xmin);
-               
-               bt->rect.xmin = x1addval;
-               bt->rect.xmax = bt->rect.xmin + i + block->bounds;
-               
-               if (col == lastcol) {
-                       bt->rect.xmax = max_ff(bt->rect.xmax, offset + block->minbounds);
-               }
+                               ui_check_but(col_bt);  /* clips text again */
+                       }
 
-               ui_check_but(bt);  /* clips text again */
-               
-               if (nextcol) {
+                       /* And we prepare next column. */
                        x1addval += i + block->bounds;
-                       col++;
+                       i = 0;
+                       init_col_bt = col_bt;
                }
-               
-               bt = bt->next;
+       }
+
+       /* Last column. */
+       for (col_bt = init_col_bt; col_bt; col_bt = col_bt->next) {
+               col_bt->rect.xmin = x1addval;
+               col_bt->rect.xmax = max_ff(x1addval + i + block->bounds, offset + block->minbounds);
+
+               ui_check_but(col_bt);  /* clips text again */
        }
 }
 
diff --git a/source/blender/editors/util/undo.c b/source/blender/editors/util/undo.c
index 434f1e3..733b45f 100644
--- a/source/blender/editors/util/undo.c
+++ b/source/blender/editors/util/undo.c
@@ -533,14 +533,20 @@ static int undo_history_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSE
                        uiLayout *layout = uiPupMenuLayout(pup);
                        uiLayout *split = uiLayoutSplit(layout, 0.0f, false);
                        uiLayout *column = NULL;
+                       const int col_size = 20 + totitem / 12;
                        int i, c;
+                       bool add_col = true;
                        
-                       for (c = 0, i = totitem - 1; i >= 0; i--, c++) {
-                               if ( (c % 20) == 0)
+                       for (c = 0, i = totitem; i--;) {
+                               if (add_col && !(c % col_size)) {
                                        column = uiLayoutColumn(split, false);
-                               if (item- [x].identifier)
+                                       add_col = false;
+                               }
+                               if (item- [x].identifier) {
                                        uiItemIntO(column, item- [x].name, item- [x].icon, op->type->idname, "item", item- [x].value);
-                               
+                                       ++c;
+                                       add_col = true;
+                               }
                        }
                        
                        MEM_freeN(item);

Following patch "fixes" the issue by: * Not having constant width for all columns, but adapt each to its content's width; * Adapting undo' menu height to undo list length (so that we never have more than three columns). It is still possible to get issues in extreme cases (small screen, high DPI size, long op names), but this should now be rare corner cases. I also fixed a minor glitch with undo menu (first column had one item less than the others…). @brecht would need your green light here before I commit, since column width change will also affect menus like modifiers or node sockets ones (imho it’s better now, wastes less space, but…). :) [P37: Fix #39234](https://archive.blender.org/developer/P37.txt) ```diff diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c index 7e6e00b..5622288 100644 --- a/source/blender/editors/interface/interface.c +++ b/source/blender/editors/interface/interface.c @@ -207,43 +207,41 @@ void ui_block_translate(uiBlock *block, int x, int y) static void ui_text_bounds_block(uiBlock *block, float offset) { uiStyle *style = UI_GetStyle(); - uiBut *bt; - int i = 0, j, x1addval = offset, nextcol; - int lastcol = 0, col = 0; - + uiBut *bt, *init_col_bt, *col_bt; + int i = 0, j, x1addval = offset; + uiStyleFontSet(&style->widget); - - for (bt = block->buttons.first; bt; bt = bt->next) { + + for (init_col_bt = bt = block->buttons.first; bt; bt = bt->next) { if (!ELEM(bt->type, SEPR, SEPRLINE)) { j = BLF_width(style->widget.uifont_id, bt->drawstr, sizeof(bt->drawstr)); - if (j > i) i = j; + if (j > i) + i = j; } - if (bt->next && bt->rect.xmin < bt->next->rect.xmin) - lastcol++; - } + if (bt->next && bt->rect.xmin < bt->next->rect.xmin) { + /* End of this column, and it’s not the last one. */ + for (col_bt = init_col_bt; col_bt->prev != bt; col_bt = col_bt->next) { + col_bt->rect.xmin = x1addval; + col_bt->rect.xmax = x1addval + i + block->bounds; - /* cope with multi collumns */ - bt = block->buttons.first; - while (bt) { - nextcol = (bt->next && bt->rect.xmin < bt->next->rect.xmin); - - bt->rect.xmin = x1addval; - bt->rect.xmax = bt->rect.xmin + i + block->bounds; - - if (col == lastcol) { - bt->rect.xmax = max_ff(bt->rect.xmax, offset + block->minbounds); - } + ui_check_but(col_bt); /* clips text again */ + } - ui_check_but(bt); /* clips text again */ - - if (nextcol) { + /* And we prepare next column. */ x1addval += i + block->bounds; - col++; + i = 0; + init_col_bt = col_bt; } - - bt = bt->next; + } + + /* Last column. */ + for (col_bt = init_col_bt; col_bt; col_bt = col_bt->next) { + col_bt->rect.xmin = x1addval; + col_bt->rect.xmax = max_ff(x1addval + i + block->bounds, offset + block->minbounds); + + ui_check_but(col_bt); /* clips text again */ } } diff --git a/source/blender/editors/util/undo.c b/source/blender/editors/util/undo.c index 434f1e3..733b45f 100644 --- a/source/blender/editors/util/undo.c +++ b/source/blender/editors/util/undo.c @@ -533,14 +533,20 @@ static int undo_history_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSE uiLayout *layout = uiPupMenuLayout(pup); uiLayout *split = uiLayoutSplit(layout, 0.0f, false); uiLayout *column = NULL; + const int col_size = 20 + totitem / 12; int i, c; + bool add_col = true; - for (c = 0, i = totitem - 1; i >= 0; i--, c++) { - if ( (c % 20) == 0) + for (c = 0, i = totitem; i--;) { + if (add_col && !(c % col_size)) { column = uiLayoutColumn(split, false); - if (item- [x].identifier) + add_col = false; + } + if (item- [x].identifier) { uiItemIntO(column, item- [x].name, item- [x].icon, op->type->idname, "item", item- [x].value); - + ++c; + add_col = true; + } } MEM_freeN(item); ```

Brecht Van Lommel commented

2014-04-15 15:32:48 +02:00

This patch doesn't apply here, it has spaces instead of tabs and in a way that makes it difficult to search and replace them.

The code looks ok to me though, just not sure how the result looks.

This patch doesn't apply here, it has spaces instead of tabs and in a way that makes it difficult to search and replace them. The code looks ok to me though, just not sure how the result looks.

Bastien Montagne commented

2014-04-15 16:39:39 +02:00

@brecht ack, sorry, copy/paste directly from terminal is not a good idea, after all… :/

This one should be OK:

P38: Fix #39234

diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c
index 7e6e00b..5622288 100644
--- a/source/blender/editors/interface/interface.c
+++ b/source/blender/editors/interface/interface.c
@@ -207,43 +207,41 @@ void ui_block_translate(uiBlock *block, int x, int y)
 static void ui_text_bounds_block(uiBlock *block, float offset)
 {
 	uiStyle *style = UI_GetStyle();
-	uiBut *bt;
-	int i = 0, j, x1addval = offset, nextcol;
-	int lastcol = 0, col = 0;
-	
+	uiBut *bt, *init_col_bt, *col_bt;
+	int i = 0, j, x1addval = offset;
+
 	uiStyleFontSet(&style->widget);
-	
-	for (bt = block->buttons.first; bt; bt = bt->next) {
+
+	for (init_col_bt = bt = block->buttons.first; bt; bt = bt->next) {
 		if (!ELEM(bt->type, SEPR, SEPRLINE)) {
 			j = BLF_width(style->widget.uifont_id, bt->drawstr, sizeof(bt->drawstr));
 
-			if (j > i) i = j;
+			if (j > i)
+				i = j;
 		}
 
-		if (bt->next && bt->rect.xmin < bt->next->rect.xmin)
-			lastcol++;
-	}
+		if (bt->next && bt->rect.xmin < bt->next->rect.xmin) {
+			/* End of this column, and it’s not the last one. */
+			for (col_bt = init_col_bt; col_bt->prev != bt; col_bt = col_bt->next) {
+				col_bt->rect.xmin = x1addval;
+				col_bt->rect.xmax = x1addval + i + block->bounds;
 
-	/* cope with multi collumns */
-	bt = block->buttons.first;
-	while (bt) {
-		nextcol = (bt->next && bt->rect.xmin < bt->next->rect.xmin);
-		
-		bt->rect.xmin = x1addval;
-		bt->rect.xmax = bt->rect.xmin + i + block->bounds;
-		
-		if (col == lastcol) {
-			bt->rect.xmax = max_ff(bt->rect.xmax, offset + block->minbounds);
-		}
+				ui_check_but(col_bt);  /* clips text again */
+			}
 
-		ui_check_but(bt);  /* clips text again */
-		
-		if (nextcol) {
+			/* And we prepare next column. */
 			x1addval += i + block->bounds;
-			col++;
+			i = 0;
+			init_col_bt = col_bt;
 		}
-		
-		bt = bt->next;
+	}
+
+	/* Last column. */
+	for (col_bt = init_col_bt; col_bt; col_bt = col_bt->next) {
+		col_bt->rect.xmin = x1addval;
+		col_bt->rect.xmax = max_ff(x1addval + i + block->bounds, offset + block->minbounds);
+
+		ui_check_but(col_bt);  /* clips text again */
 	}
 }
 
diff --git a/source/blender/editors/util/undo.c b/source/blender/editors/util/undo.c
index 434f1e3..733b45f 100644
--- a/source/blender/editors/util/undo.c
+++ b/source/blender/editors/util/undo.c
@@ -533,14 +533,20 @@ static int undo_history_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSE
 			uiLayout *layout = uiPupMenuLayout(pup);
 			uiLayout *split = uiLayoutSplit(layout, 0.0f, false);
 			uiLayout *column = NULL;
+			const int col_size = 20 + totitem / 12;
 			int i, c;
+			bool add_col = true;
 			
-			for (c = 0, i = totitem - 1; i >= 0; i--, c++) {
-				if ( (c % 20) == 0)
+			for (c = 0, i = totitem; i--;) {
+				if (add_col && !(c % col_size)) {
 					column = uiLayoutColumn(split, false);
-				if (item- [x].identifier)
+					add_col = false;
+				}
+				if (item- [x].identifier) {
 					uiItemIntO(column, item- [x].name, item- [x].icon, op->type->idname, "item", item- [x].value);
-				
+					++c;
+					add_col = true;
+				}
 			}
 			
 			MEM_freeN(item);

@brecht ack, sorry, copy/paste directly from terminal is not a good idea, after all… :/ This one should be OK: [P38: Fix #39234](https://archive.blender.org/developer/P38.txt) ```diff diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c index 7e6e00b..5622288 100644 --- a/source/blender/editors/interface/interface.c +++ b/source/blender/editors/interface/interface.c @@ -207,43 +207,41 @@ void ui_block_translate(uiBlock *block, int x, int y) static void ui_text_bounds_block(uiBlock *block, float offset) { uiStyle *style = UI_GetStyle(); - uiBut *bt; - int i = 0, j, x1addval = offset, nextcol; - int lastcol = 0, col = 0; - + uiBut *bt, *init_col_bt, *col_bt; + int i = 0, j, x1addval = offset; + uiStyleFontSet(&style->widget); - - for (bt = block->buttons.first; bt; bt = bt->next) { + + for (init_col_bt = bt = block->buttons.first; bt; bt = bt->next) { if (!ELEM(bt->type, SEPR, SEPRLINE)) { j = BLF_width(style->widget.uifont_id, bt->drawstr, sizeof(bt->drawstr)); - if (j > i) i = j; + if (j > i) + i = j; } - if (bt->next && bt->rect.xmin < bt->next->rect.xmin) - lastcol++; - } + if (bt->next && bt->rect.xmin < bt->next->rect.xmin) { + /* End of this column, and it’s not the last one. */ + for (col_bt = init_col_bt; col_bt->prev != bt; col_bt = col_bt->next) { + col_bt->rect.xmin = x1addval; + col_bt->rect.xmax = x1addval + i + block->bounds; - /* cope with multi collumns */ - bt = block->buttons.first; - while (bt) { - nextcol = (bt->next && bt->rect.xmin < bt->next->rect.xmin); - - bt->rect.xmin = x1addval; - bt->rect.xmax = bt->rect.xmin + i + block->bounds; - - if (col == lastcol) { - bt->rect.xmax = max_ff(bt->rect.xmax, offset + block->minbounds); - } + ui_check_but(col_bt); /* clips text again */ + } - ui_check_but(bt); /* clips text again */ - - if (nextcol) { + /* And we prepare next column. */ x1addval += i + block->bounds; - col++; + i = 0; + init_col_bt = col_bt; } - - bt = bt->next; + } + + /* Last column. */ + for (col_bt = init_col_bt; col_bt; col_bt = col_bt->next) { + col_bt->rect.xmin = x1addval; + col_bt->rect.xmax = max_ff(x1addval + i + block->bounds, offset + block->minbounds); + + ui_check_but(col_bt); /* clips text again */ } } diff --git a/source/blender/editors/util/undo.c b/source/blender/editors/util/undo.c index 434f1e3..733b45f 100644 --- a/source/blender/editors/util/undo.c +++ b/source/blender/editors/util/undo.c @@ -533,14 +533,20 @@ static int undo_history_invoke(bContext *C, wmOperator *op, const wmEvent *UNUSE uiLayout *layout = uiPupMenuLayout(pup); uiLayout *split = uiLayoutSplit(layout, 0.0f, false); uiLayout *column = NULL; + const int col_size = 20 + totitem / 12; int i, c; + bool add_col = true; - for (c = 0, i = totitem - 1; i >= 0; i--, c++) { - if ( (c % 20) == 0) + for (c = 0, i = totitem; i--;) { + if (add_col && !(c % col_size)) { column = uiLayoutColumn(split, false); - if (item- [x].identifier) + add_col = false; + } + if (item- [x].identifier) { uiItemIntO(column, item- [x].name, item- [x].icon, op->type->idname, "item", item- [x].value); - + ++c; + add_col = true; + } } MEM_freeN(item); ```

Brecht Van Lommel commented

2014-04-15 16:44:05 +02:00

Thanks, looks good to me.

blender-admin commented

2014-04-15 16:49:49 +02:00

This issue was referenced by blender/blender-addons-contrib@09e5aa5156

This issue was referenced by blender/blender-addons-contrib@09e5aa5156486585b3d23f52f927db744fb9a055

blender-admin commented

2014-04-15 16:49:49 +02:00

This issue was referenced by 09e5aa5156

This issue was referenced by 09e5aa5156486585b3d23f52f927db744fb9a055

Bastien Montagne commented

2014-04-15 16:57:07 +02:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2014-04-15 16:57:07 +02:00

Bastien Montagne commented

2014-04-15 16:57:07 +02:00

Closed by commit 09e5aa5156.

Closed by commit 09e5aa5156.

Eneko Castresana Vara commented

2016-06-22 20:01:45 +02:00

Added subscriber: @ecv

Eneko Castresana Vara commented

2016-06-22 20:01:45 +02:00

Changed status from 'Resolved' to: 'Open'

Eneko Castresana Vara reopened this issue

2016-06-22 20:01:45 +02:00

Eneko Castresana Vara commented

2016-06-22 20:01:45 +02:00

Hello, this is still broken in 2.77a

I tried to reproduce it on purpose a number of times and I couldn't get it. However after working for a while it will eventually mess up.

My setup: Windows 7, two monitors both at 1024x768

Feel free to ask for any other specifics or actions.

Cheers,
Eneko

Hello, this is still broken in 2.77a I tried to reproduce it on purpose a number of times and I couldn't get it. However after working for a while it will eventually mess up. My setup: Windows 7, two monitors both at 1024x768 Feel free to ask for any other specifics or actions. Cheers, Eneko

Bastien Montagne commented

2016-06-22 21:38:31 +02:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2016-06-22 21:38:31 +02:00

Bastien Montagne commented

2016-06-22 21:38:31 +02:00

Please do not resurrect mummified reports like that… If you believe this is still valid, then open a new report and optionally add a link/reference to this one in it (typing #39234 would be enough).

Further more, as explained in closing commit, we cannot tackle all possible corner cases, Blender is not exactly designed to be used on a smartphone screen e.g.

Please do not resurrect mummified reports like that… If you believe this is still valid, then open a new report and optionally add a link/reference to this one in it (typing #39234 would be enough). Further more, as explained in closing commit, we cannot tackle all possible corner cases, Blender is not exactly designed to be used on a smartphone screen e.g.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

popup menus behave poorly when they have not enough width for all their columns #39234