[Bf-taskforce25] interface operator XXX's

[Ton Roosendaal]

Hi others!

I've went over this with Brecht extensively.

In short: we've agreed on a couple of reshuffles in the code that will 
make UI button and menu handling simpler and easier to manage.
One of the solutions is to not require operators (and operator API) for 
menus or buttons, but use designated "uiBlock handlers". Operators then 
are only involved as the result of a button or menu. :)

-Ton-

------------------------------------------------------------------------
Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
Blender Institute BV  Entrepotdok 57A  1018AD Amsterdam The Netherlands

On 4 Dec, 2008, at 20:42, Brecht Van Lommel wrote:

>
> Hi Ton, others,
>
> I went over the XXX's marked in the interface code after the changes 
> and
> tried to fix them. These are corner cases that I tried to work around
> with suboptimal code, but I think that solving them can't be done just
> in the interface code and most likely requires window manager changes.
>
> 1) button_activate_state:
> 	/* note we move the handler to the region when the block is open,
> 	 * so we don't interfere with the events as long as it's open */
> 	/* XXX (to brecht) removing this makes thing work proper */
> 	//WM_event_remove_modal_handler(&C->window->handlers, data->operator);
> 	//WM_event_add_modal_handler(C, &data->region->handlers, 
> data->operator);
>
> This workaround was added to solve an issue in overlapping menus. This
> happens for example in this case:
> - Open the View menu in the timeline window.
> - Go to highlight Sub Menu and, the menu pops open.
> - Now move diagonally downwards (not too fast) to an
>   item in the lower part of the sub menu.
> - The menu submenu will close right when you enter it.
>
> This happened because the menus overlap 2 pixels or so, and both get 
> the
> event, so the lower menu will get its button hightlight. This should
> probably be solved in the window manager somehow, such that overlapping
> regions block events for regions below it. This should be optional
> though, because it would give issues with tooltips.
>
> 2) button_activate_modal:
> 	data= op->customdata;
> 	/* XXX (for brecht) this happens when cancel() frees, and modal 
> handler still runs */
> 	if(data==NULL)
> 	    return OPERATOR_FINISHED;
>
> I don't think this can happen, it could happen indeed for the 
> menu_block
> operator since that can call cancel itself when it returns, but that
> does not cancel the operator itself yet then. So I don't think this
> check has to be here, unless there is a scenario I'm not thinking of,
> but the equivalent check in menu_block_handle_modal should be kept
> indeed.
>
> 3) menu_block_handle_modal:
> 	ar= bhandle->region;
> 	block= ar->uiblocks.first;
>
> 	/* XXX (for brecht) this happens when click on menu */
> 	if(block==NULL)
> 	    return OPERATOR_FINISHED;
>
> It goes wrong earlier than this check, the region this is using is 
> freed
> already at this point. This operator is invoked manually in
> ui_menu_block_create (interface_ops.c) when the menu block is created.
> It should basically live as long as the menu block is open, but
> currently it does not get stopped when the menu blocks' region is
> removed. The modalops list was a way to attach this operator to the
> region and have it automatically cancelled when the region is removed.
> The problem is that the operator has to run at the window level since 
> it
> still works in the "safe zone" outside of the actual menu. Not sure how
> this operator should be linked to the region, modalops made this
> possible but there is probably a better way, but I'm not sure about the
> solution here.
>
> 4) ui_menu_block_create:
> 	SWAP(ARegion*, C->region, ar); /* XXX 2.50 bad context swapping */
> 	WM_operator_invoke(C, 
> WM_operatortype_find("ED_UI_OT_menu_block_handle"), NULL);
> 	SWAP(ARegion*, C->region, ar);
>
> This is related to the above problem. Here it invokes that operator, 
> but
> it does it from a different context from the menu block region, since
> the button opening the menu block is not in this region of course.
>
> 5) menu_block_handle_activate_button
>     ot= WM_operatortype_find("ED_UI_OT_button_activate");
>
>     // XXX  WM_operator_cancel(C, &butregion->modalops, ot);
>     but->activateflag= activateflag;
>
>     SWAP(ARegion*, C->region, butregion); /* XXX 2.50 bad state 
> manipulation? */
>     WM_operator_invoke(C, ot, event);
>     SWAP(ARegion*, C->region, butregion);
>
> The second XXX, setting the right regions is not needed anymore now,
> since the region is set into the context when the operator runs (even
> though its handler is at the window level), because it was invoked with
> the region in the context.
>
> The first XXX is because we need to cancel the previous highlighted
> button before a new one can be highlighted. This is tied to the
> button_activate operator. Maybe the highlight code has to be moved out
> of that operator so no operator needs to be running for this. That's 
> not
> trivial, since you then need the other part of the operator to register
> more complex keymaps rather than just handling these keys in
> button_activate_modal (timer for tooltip or menu open, enter for 
> action,
> buttons, ctrl+c/v, ..).
>
> Brecht.
>
> _______________________________________________
> Bf-taskforce25 mailing list
> Bf-taskforce25 at blender.org
> http://lists.blender.org/mailman/listinfo/bf-taskforce25
>