Ability for popups to refresh their UI
AbandonedPublic
Actions

Authored by Campbell Barton (campbellbarton) on Jun 3 2014, 8:34 AM.

Details

Reviewers

Andrew Peel (andyrexic)

Summary

Motivation

Currently theres the buttons in a popup are fixed, once the popup is made, theres no way to modify the buttons based on changed state.

This is ok for menu's, but for Operator-redo popup (or similar popups for the operator UI), this can be quite annoying.
For example, some buttons may show, depending on other options. Options may become inactive.
This works in the toolbar for operators, but not in popups.

This patch adds the ability for popups to re-initialize so the UI can be dynamic.

note:
(The color picker works around this by hiding buttons, so it doesn't need to redraw the UI)

note:
This patch changes editmesh_tools.c, only so you can easily test the change.

Testing

To test the patch:

Add 2 shape keys to the default cube.
enter editmode
Spacebar "Blend from shape"
Press F6 (to open popup)
Press Add,

Notice the Blend button toggles on and off This serves no purpose, its just an example

Implementation details

RGN_DRAW_REFRESH_UI is added to tag regions to refresh their buttons.
Each regions draw functions is responsible for handling & clearing this flag.
ED_region_tag_refresh_ui sets RGN_DRAW_REFRESH_UI
Currently this flag only makes sense for popups, but it could be used elsewhere later.
Some low level functions had to be split-out, because the old-new button comparisons must be delayed until after the popups buttons have been relocated.
uiBlockUpdateFromOld has been split from uiEndBlock into its own function.
callbacks to ui_popup_block_create can't run uiEndBlock because any old-buttons aren't relocated to the popup's position.

Concerns

RGN_DRAW_REFRESH_UI may not be the best way to tag the UI for updating, I did attempt to do this as a callback (but this ended up being more complicated since each popup handler then needed some way to flag the UI to redraw). Nevertheless this could be done differently.
Up until now we rely on ED_region_do_draw to call uiFreeInactiveBlocks, but I couldn't find a way to enforce updating the UI, without a manual call here. This also involves clearing block->oldblock, and running uiBlockUpdateFromOld explicitly, am not so happy about this.
The mouse location when first opening the popup needs to be stored and used when refreshing, because popup layout functions use. This isnt great either, to avoid we'd need to pass mouse xy coords to uiEndBlock

Diff Detail

Event Timeline

Excellent! I have been looking into how to implement this for the last couple of days. It is extremely helpful to see how you went about implementing this. I am going to be doing some testing on this patch today. I'll let you know how it goes. Cheers.

I tried your patch here, but it doesn't seem to be working for an operator popup: P80
Do I need to do any change to set the UI to be refreshable?

Fix for WM_operator_props_dialog_popup as reported by @Dalai Felinto (dfelinto)

@Dalai Felinto (dfelinto), Note, for P80 to work you need a check function which returns True (denoted redraw)

See: P81

source/blender/editors/interface/interface.c
1085	simply extracted from `uiEndBlock`, no functional changes.

remove uiEndBlock in id_search_menu

Just about everything appears to be working. The only problem i have encountered is when trying to display lists. ( i.e. layout.template_list("MATERIAL_UL_matslots", "", obj, "material_slots", obj, "active_material_index", rows=1) ) The list displays fine, but when updating the UI, Blender crashes. Displaying lists in dialog popups is not a very common, but it can be helpful. Really great stuff @Campbell Barton (campbellbarton). Thanks.

I like to display properties about the active object in popup dialogs, but the functionality that allows users to hover over properties and use the right click menu to add drivers or insert keyframes doesn't work in popups. Since we were on the topic of prop dialog boxes i figured i would bring it up, but this might be a different task since this was a problem that existed before this patch.

@Andrew Peel (andyrexic)

ui-lists in popups are likely a bit more involved (currently they save data in the UI so reload works).
I'd have to investigate, perhaps @Bastien Montagne (mont29) knows off hand.

Adding drivers and similar is probably easy to get working.

If you find something that fails it would be good if you could post some Python example so others can check quickly what issues are.

Well, uiList uses the current region to store its settings, since popups use a temp region, I guess the issue is here, if the temp region is re-created each time the popup is refreshed… Should not be that hard to fix, though, probably just a matter of handling correctly uiList's pointers in uiBlockUpdateFromOld() or so - though if we can’t have access to old temp region, it would become a bit useless (since scrolling etc. info would be lost at each redraw).

@Andrew Peel (andyrexic), can you please attach here your code to add an uiList to a popup?

Btw, I’m very interested to see this patch in master too, that is an old missing feature in our UI! :D

@Bastien Montagne (mont29) - The temp region, isn't free'd, so this should be able to work. (the uiBlock is freed though)

I created a simple test operator that demonstrates the UIList in a popup and I also included some object properties that will be helpful when testing the add driver feature from the right click menu: P82

Ok, so culprit was that there is no region in context given to those UI elements in a popup… This very quick and dirty hack fixes it (apply it on top of this branch):

P83 Masterwork From Distant Lands

1	diff --git a/source/blender/editors/interface/interface_regions.c b/source/blender/editors/interface/interface_regions.c
2	index 6ec143d..43d7fc9 100644
3	--- a/source/blender/editors/interface/interface_regions.c
4	+++ b/source/blender/editors/interface/interface_regions.c
5	@@ -1423,11 +1423,14 @@ static void ui_block_region_draw(const bContext C, ARegion ar)
6
7	if (ar->do_draw & RGN_DRAW_REFRESH_UI) {
8	uiBlock *block_next;
9	+ ARegion *back_ar = CTX_wm_region(C);
10	+ CTX_wm_region_set(C, ar);
11	ar->do_draw &= ~RGN_DRAW_REFRESH_UI;
12	for (block = ar->uiblocks.first; block; block = block_next) {
13	block_next = block->next;
14	ui_popup_block_refresh((bContext *)C, block->handle, NULL, NULL);
15	}
16	+ CTX_wm_region_set(C, back_ar);
17	}
18
19	for (block = ar->uiblocks.first; block; block = block->next)
20	@@ -1669,7 +1672,13 @@ uiPopupBlockHandle ui_popup_block_create(bContext C, ARegion *butregion, uiBut
21
22	UI_add_region_handlers(&ar->handlers);
23
24	- block = ui_popup_block_refresh(C, handle, butregion, but);
25	+ {
26	+ ARegion *back_ar = CTX_wm_region(C);
27	+ CTX_wm_region_set(C, ar);
28	+ block = ui_popup_block_refresh(C, handle, butregion, but);
29	+ CTX_wm_region_set(C, back_ar);
30	+ }
31	+
32	handle = block->handle;
33
34	return handle;

Now uiList works OK (except that resizing it with the drag does not trigger refresh, but that’s probably a minor unrelated issue, some missing update tagging or so).

Committed rBea2043eb3a31468d9afeb1d6cc140567e219565f

Julian Eisel (Severin) mentioned this in D834: Don't close menus when clicking on checkboxes..Oct 20 2014, 4:30 PM

Campbell Barton (campbellbarton) mentioned this in rBSea2043eb3a31: UI: Add support for popups to refresh their layput (D578).Nov 20 2014, 9:45 AM