[PATCH weston 2/2] desktop-shell: fix minimization of fullscreen surfaces, pointer focus

[Pekka Paalanen]

On Mon, 16 Feb 2015 11:52:47 +0100
Manuel Bachmann <manuel.bachmann at open.eurogiciel.org> wrote:

> From: Manuel Bachmann <manuel.bachmann at open.eurogiciel.org>
> 
> 2 things were wrong with the minimization code :
> - if the minimized surface was fullscreen, the shell would keep
>   its fullscreen mode on and only display a black background ;
> - keyboard focus was unset, but pointer focus was not.
> 
> On the other hand, make sure a fullscreen surface gets centered
> again when refocused with Mod+Tab.
> 
> Author: Manuel Bachmann <manuel.bachmann at open.eurogiciel.org>
> Author: Nicolas Guyomard <nicolas.guyomard at open.eurogiciel.org>
> Signed-off-by: Manuel Bachmann <manuel.bachmann at open.eurogiciel.org>
> ---
>  desktop-shell/shell.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 5900c4d..830c25d 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2657,13 +2657,20 @@ set_minimized(struct weston_surface *surface)
>  	weston_layer_entry_remove(&view->layer_link);
>  	weston_layer_entry_insert(&shsurf->shell->minimized_layer.view_list, &view->layer_link);
>  
> +	if (shsurf->state.fullscreen)
> +		unset_fullscreen(shsurf);
> +
>  	drop_focus_state(shsurf->shell, current_ws, view->surface);
>  	wl_list_for_each(seat, &shsurf->shell->compositor->seat_list, link) {
>  		if (!seat->keyboard)
>  			continue;
>  		focus = weston_surface_get_main_surface(seat->keyboard->focus);
> -		if (focus == view->surface)
> +		if (focus == view->surface) {
>  			weston_keyboard_set_focus(seat->keyboard, NULL);
> +			weston_pointer_set_focus(seat->pointer, NULL,
> +						 wl_fixed_from_int(0),
> +						 wl_fixed_from_int(0));
> +		}
>  	}

Hi,

I'm not so sure about the pointer_set_focus here. Shuffling surfaces is
supposed to trigger a repick, which will reassign the pointer focus.
However, it seems something is off. The keyboards focus is different to
pointer focus, because keyboard focus is "sticky" and needs to be
explicitly managed. Pointer focus generally follows picking unless the
pointer is grabbed.

With this patch applied:

[2030091.702]  -> xdg_surface at 15.set_minimized()
[2030091.737]  -> xdg_popup at 21.destroy()
[2030091.768]  -> wl_surface at 25.destroy()
[2030091.805]  -> wl_buffer at 29.destroy()
[2030091.892]  -> wl_shm_pool at 28.destroy()
[2030092.413] wl_display at 1.delete_id(21)
[2030092.469] wl_display at 1.delete_id(25)
[2030092.503] wl_display at 1.delete_id(29)
[2030092.537] wl_display at 1.delete_id(28)
[2030092.570] wl_keyboard at 16.leave(315, wl_surface at 14)
[2030092.633] xdg_surface at 15.configure(1024, 600, array, 316)
[2030092.722]  -> xdg_surface at 15.ack_configure(316)
[2030092.762]  -> xdg_surface at 15.set_title("pq at erebos:~/git/weston")
[2030092.798] wl_pointer at 17.leave(317, nil)
[2030092.853] wl_keyboard at 16.modifiers(318, 0, 0, 0, 0)
[2030093.019] wl_pointer at 17.enter(318, nil, 69.000000, 109.000000)
[2030093.154] xdg_shell at 12.ping(319)
[2030093.195]  -> xdg_shell at 12.pong(319)
[2030093.265] wl_pointer at 17.leave(320, nil)
[2030093.322]  -> wl_compositor at 4.create_region(new id wl_region at 28)
[2030093.372]  -> xdg_surface at 15.set_title("pq at erebos:~/git/weston")
[2030093.409]  -> wl_compositor at 4.create_region(new id wl_region at 29)
[2030093.455]  -> wl_region at 29.add(0, 0, 1024, 600)
[2030093.553]  -> wl_region at 28.add(0, 0, 1024, 600)
[2030093.642]  -> wl_surface at 14.frame(new id wl_callback at 25)
[2030098.044]  -> wl_surface at 14.set_opaque_region(wl_region at 28)
[2030098.142]  -> wl_region at 28.destroy()
[2030098.170]  -> wl_surface at 14.set_input_region(wl_region at 29)
[2030098.208]  -> wl_region at 29.destroy()
[2030098.291]  -> wl_surface at 14.attach(wl_buffer at 24, 0, 0)
[2030098.379]  -> wl_surface at 14.damage(0, 0, 1024, 600)
[2030098.436]  -> wl_surface at 14.commit()
[2030103.646] wl_display at 1.delete_id(28)
[2030103.727] wl_display at 1.delete_id(29)

This was made with weston-terminal patched to have a "minimize" option
in the context menu. I just pushed that patch to master, btw. so
everyone can test.

I assume the first wl_pointer.leave(nil) is for the popup surface, so
that's ok. The enter for nil looks very suspicious though. Maybe that
is caused by this patch?

My opinion is that this looks okay apart from the pointer focus.

>  
>  	shell_surface_update_child_surface_layers(shsurf);
> @@ -5937,8 +5944,11 @@ switcher_next(struct switcher *switcher)
>  		view->alpha = 1.0;
>  
>  	shsurf = get_shell_surface(switcher->current);
> -	if (shsurf && shsurf->state.fullscreen)
> -		shsurf->fullscreen.black_view->alpha = 1.0;
> +	if (shsurf && shsurf->state.fullscreen) {
> +		center_on_output(shsurf->view, shsurf->fullscreen_output);
> +		if (shsurf->fullscreen.black_view)
> +			shsurf->fullscreen.black_view->alpha = 1.0;

Just curious, how can the surface go off-center? Some output
hot-unplugging involved?

When wouldn't there be a black_view?

> +	}
>  }
>  
>  static void

Thanks,
pq