[PATCH v2 1/4] shell: add fullscreen path for activate() and switcher

[Kristian Hoegsberg]

On Tue, Mar 13, 2012 at 11:18:23PM +0800, zhiwen.wu at linux.intel.com wrote:
> From: Alex Wu <zhiwen.wu at linux.intel.com>
> 
> For activate(), stack the surface atop fullscreen layer instead of
> toplevel layer.
> For switcher, involve the fullscreen surfaces into surface iteration,
> and make fullscreen surface and its black surface transparent if
> necessary.

Sorry for the long delay in reviewing this.  Comments below.

Kristian

> ---
>  src/shell.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 59 insertions(+), 11 deletions(-)
> 
> diff --git a/src/shell.c b/src/shell.c
> index 765b0a4..984c75d 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -144,6 +144,9 @@ struct rotate_grab {
>  	} center;
>  };
>  
> +static struct shell_surface *
> +get_shell_surface(struct weston_surface *surface);
> +
>  static void
>  center_on_output(struct weston_surface *surface,
>  		 struct weston_output *output);
> @@ -1344,6 +1347,7 @@ activate(struct weston_shell *base, struct weston_surface *es,
>  {
>  	struct wl_shell *shell = container_of(base, struct wl_shell, shell);
>  	struct weston_compositor *compositor = shell->compositor;
> +	struct wl_list *lst;
>  
>  	weston_surface_activate(es, device, time);
>  
> @@ -1364,14 +1368,48 @@ activate(struct weston_shell *base, struct weston_surface *es,
>  		break;
>  	case SHELL_SURFACE_FULLSCREEN:
>  		/* should on top of panels */
> +		shell_stack_fullscreen(get_shell_surface(es));
>  		break;
>  	default:
> -		weston_surface_restack(es,
> -				       &shell->toplevel_layer.surface_list);
> +		if (wl_list_empty(&shell->fullscreen_layer.surface_list))
> +			lst = &shell->toplevel_layer.surface_list;
> +		else
> +			lst = &shell->fullscreen_layer.surface_list;
> +		weston_surface_restack(es, lst);

Isn't this putting the surface (which could be a regular toplevel)
into the fullscreen layer?  What we need instead is to move the
fullscreen surface down into the toplevel layer if another surface is
activated.

>  		break;
>  	}
>  }
>  
> +
> +static struct weston_surface *
> +get_upper_surface(struct weston_surface *es)
> +{
> +	if (es->link.prev == &es->compositor->surface_list)
> +		return NULL;
> +
> +	return container_of(es->link.prev, struct weston_surface, link);
> +}
> +
> +static struct weston_surface *
> +get_lower_surface(struct weston_surface *es)
> +{
> +	if (es->link.next == &es->compositor->surface_list)
> +		return NULL;
> +
> +	return container_of(es->link.next, struct weston_surface, link);
> +}
> +
> +static bool
> +is_black_surface(struct weston_surface *es)
> +{
> +	struct weston_surface *upper;
> +
> +	if ((upper = get_upper_surface(es)) && 
> +            get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN)
> +		return true;
> +	return false;
> +}

Instead of this fragile get_upper/get_lower stuff, I think we just
need to set a destroy listener on the black surface so we can
recognize it like we recognize shell surfaces in get_shell_surface.
The destroy function shouldn't do anything, but we'll be able to
recognize the black surface by it.  Embed the wl_listener as
shell_surface.fullscreen.destroy_listener, and that way
we can get the associated fullscreen surface as

  shsurf = container_of(listener, struct shell_surface, fullscreen.destroy_listener);

then we could do something like

  is_black_surface(surface, &upper);

which does all this and returns the upper surface if it's a black
surface.

>  static void
>  click_to_activate_binding(struct wl_input_device *device,
>  			  uint32_t time, uint32_t key,
> @@ -1380,19 +1418,13 @@ click_to_activate_binding(struct wl_input_device *device,
>  	struct weston_input_device *wd = (struct weston_input_device *) device;
>  	struct weston_compositor *compositor = data;
>  	struct weston_surface *focus;
> -	struct weston_surface *upper;
>  
>  	focus = (struct weston_surface *) device->pointer_focus;
>  	if (!focus)
>  		return;
>  
> -	upper = container_of(focus->link.prev, struct weston_surface, link);
> -	if (focus->link.prev != &compositor->surface_list &&
> -	    get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN) {
> -		printf("%s: focus is black surface, raise its fullscreen surface\n", __func__);
> -		shell_stack_fullscreen(get_shell_surface(upper));
> -		focus = upper;
> -	}
> +	if (is_black_surface(focus))
> +		focus = get_upper_surface(focus);
>  
>  	if (state && device->pointer_grab == &device->default_pointer_grab)
>  		activate(compositor->shell, focus, wd, time);
> @@ -1833,6 +1865,12 @@ switcher_next(struct switcher *switcher)
>  			weston_surface_damage(surface);
>  			break;
>  		default:
> +			if (is_black_surface(surface)) {
> +				weston_surface_set_color(surface, 
> +						         0.0, 0.0, 0.0, (GLfloat)64/(GLfloat)255);

We should just set the alpha for solid color surfaces here... which I
just added support for in master :) Also, I would put the if statement
outside the switch, which is just for shell surface types.

> +				surface->geometry.dirty = 1;
> +				weston_surface_damage(surface);
> +			}
>  			break;
>  		}
>  	}
> @@ -1840,12 +1878,19 @@ switcher_next(struct switcher *switcher)
>  	if (next == NULL)
>  		next = first;
>  
> +	if (next == NULL)
> +		return;
> +
>  	wl_list_remove(&switcher->listener.link);
>  	wl_list_insert(next->surface.resource.destroy_listener_list.prev,
>  		       &switcher->listener.link);
>  
>  	switcher->current = next;
>  	next->alpha = 255;
> +
> +	if (get_shell_surface_type(next) == SHELL_SURFACE_FULLSCREEN) {
> +		weston_surface_set_color(get_lower_surface(next), 0.0, 0.0, 0.0, 1.0);

This shouldn't be nececssary now.

> +	}
>  }
>  
>  static void
> @@ -1867,11 +1912,14 @@ switcher_destroy(struct switcher *switcher, uint32_t time)
>  		(struct weston_input_device *) switcher->grab.input_device;
>  
>  	wl_list_for_each(surface, &compositor->surface_list, link) {
> +		if (is_black_surface(surface))
> +			weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1.0);

This shouldn't be nececssary now.

>  		surface->alpha = 255;
>  		weston_surface_damage(surface);
>  	}
>  
> -	activate(compositor->shell, switcher->current, device, time);
> +	if (switcher->current)
> +		activate(compositor->shell, switcher->current, device, time);
>  	wl_list_remove(&switcher->listener.link);
>  	wl_input_device_end_keyboard_grab(&device->input_device, time);
>  	free(switcher);
> -- 
> 1.7.5.4
>