[fullscreen-v8 PATCH 2/3] shell: Add implementation of fullscreen.

Kristian Hoegsberg hoegsberg at gmail.com
Tue Feb 28 11:13:38 PST 2012


On Tue, Feb 28, 2012 at 06:08:38PM +0800, zhiwen.wu at linux.intel.com wrote:
> From: Alex Wu <zhiwen.wu at linux.intel.com>
> 
> All the fullscreen things (black surface, raise atop panels, transform, positioning)
> are handled in map() or configure().

Ok, looking good, we're almost there now :) A few comments below (and
please try to keep the commit message less than 78 chars wide).

thanks,
Kristian

> Signed-off-by: Alex Wu <zhiwen.wu at linux.intel.com>
> Signed-off-by: Juan Zhao <juan.j.zhao at linux.intel.com>
> ---
>  src/shell.c |  231 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 214 insertions(+), 17 deletions(-)
> 
> diff --git a/src/shell.c b/src/shell.c
> index d949d0c..add9547 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -38,6 +38,8 @@
>  
>  struct shell_surface;
>  
> +#define INVALID_X (-1)
> +#define INVALID_Y (-1)
>  struct wl_shell {
>  	struct weston_compositor *compositor;
>  	struct weston_shell shell;
> @@ -108,6 +110,13 @@ struct shell_surface {
>  		int32_t initial_up;
>  	} popup;
>  
> +	struct {
> +		enum wl_shell_surface_fullscreen_method type;
> +		struct weston_transform transform; /* matrix from x, y */
> +		uint32_t framerate;
> +		struct weston_surface *black_surface;
> +	} fullscreen;
> +
>  	struct weston_output *fullscreen_output;
>  	struct weston_output *output;
>  	struct wl_list link;
> @@ -134,6 +143,14 @@ struct rotate_grab {
>  };
>  
>  static void
> +activate(struct weston_shell *base, struct weston_surface *es,
> +	 struct weston_input_device *device, uint32_t time);
> +
> +static void
> +center_on_output(struct weston_surface *surface,
> +		 struct weston_output *output);
> +
> +static void
>  shell_configuration(struct wl_shell *shell)
>  {
>  	char *config_file;
> @@ -303,7 +320,8 @@ weston_surface_resize(struct shell_surface *shsurf,
>  {
>  	struct weston_resize_grab *resize;
>  
> -	/* FIXME: Reject if fullscreen */
> +	if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> +		return 0;
>  
>  	if (edges == 0 || edges > 15 ||
>  	    (edges & 3) == 3 || (edges & 12) == 12)
> @@ -334,7 +352,8 @@ shell_surface_resize(struct wl_client *client, struct wl_resource *resource,
>  	struct weston_input_device *wd = input_resource->data;
>  	struct shell_surface *shsurf = resource->data;
>  
> -	/* FIXME: Reject if fullscreen */
> +	if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> +		return;
>  
>  	if (wd->input_device.button_count == 0 ||
>  	    wd->input_device.grab_time != time ||
> @@ -352,15 +371,22 @@ get_default_output(struct weston_compositor *compositor)
>  			    struct weston_output, link);
>  }
>  
> +static void
> +shell_unset_fullscreen(struct shell_surface *shsurf)
> +{
> +	shsurf->fullscreen_output = NULL;
> +	shsurf->fullscreen.type = WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT;
> +	shsurf->surface->force_configure = 1;
> +	wl_list_remove(&shsurf->fullscreen.transform.link);
> +	wl_list_init(&shsurf->fullscreen.transform.link);
> +}
> +
>  static int
>  reset_shell_surface_type(struct shell_surface *surface)
>  {
>  	switch (surface->type) {
>  	case SHELL_SURFACE_FULLSCREEN:
> -		weston_surface_set_position(surface->surface,
> -					    surface->saved_x,
> -					    surface->saved_y);
> -		surface->fullscreen_output = NULL;
> +		shell_unset_fullscreen(surface);
>  		break;
>  	case SHELL_SURFACE_MAXIMIZED:
>  		surface->output = get_default_output(surface->surface->compositor);
> @@ -487,6 +513,109 @@ shell_surface_set_maximized(struct wl_client *client,
>  	shsurf->type = SHELL_SURFACE_MAXIMIZED;
>  }
>  
> +static struct weston_surface *
> +create_black_surface(struct weston_compositor *ec)
> +{
> +	struct weston_surface *surface = NULL;
> +
> +	surface = weston_surface_create(ec);
> +	if (surface == NULL) {
> +		fprintf(stderr, "no memory\n");
> +		return NULL;
> +	}
> +
> +	weston_surface_configure(surface, 0, 0, 8192, 8192);
> +	weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1);

This surface (8192x8192 at 0,0) will cover all outputs.  We need to
make a surface for a specific output such that the size and position
matches the output.

> +	return surface;
> +}
> +
> +/*
> + * Handle size dismatch and positioning according to the method.
> + */
> +static void
> +shell_configure_fullscreen(struct shell_surface *shsurf)
> +{
> +	struct weston_output *output = shsurf->fullscreen_output;
> +	struct weston_surface *surface = shsurf->surface;
> +	struct weston_matrix *matrix;
> +	float scale;
> +
> +	center_on_output(surface, output);
> +
> +	switch (shsurf->fullscreen.type) {
> +	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT:
> +		break;
> +	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_SCALE:
> +		matrix = &shsurf->fullscreen.transform.matrix;
> +		weston_matrix_init(matrix);
> +		scale = (float)output->current->width/(float)surface->geometry.width;
> +		weston_matrix_scale(matrix, scale, scale, 1);
> +		wl_list_remove(&shsurf->fullscreen.transform.link);
> +		wl_list_insert(surface->geometry.transformation_list.prev,
> +			       &shsurf->fullscreen.transform.link);
> +		weston_surface_set_position(surface, output->x, output->y);
> +		break;
> +	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER:
> +		break;
> +	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_FILL:
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +/*
> + * Handle the stacking order of the fullscreen surface and the associated black surface.
> + */
> +static void
> +shell_stack_fullscreen(struct shell_surface *shsurf, bool mapped)
> +{
> +	struct weston_surface *surface = shsurf->surface;
> +	struct wl_shell *shell = shell_surface_get_shell(shsurf);
> +	struct wl_list *list;
> +
> +	if (mapped) {
> +		wl_list_remove(&surface->link);
> +		wl_list_remove(&shsurf->fullscreen.black_surface->link);
> +	}
> +
> +	if (shell->locked) {
> +		wl_list_insert(&shell->hidden_surface_list, &surface->link);
> +		wl_list_insert(&surface->link, &shsurf->fullscreen.black_surface->link);
> +	} else {
> +		list = weston_compositor_top(surface->compositor);
> +		wl_list_insert(list, &surface->link);
> +		wl_list_insert(&surface->link, &shsurf->fullscreen.black_surface->link);
> +
> +		/*assign output*/
> +		surface->output = shsurf->fullscreen_output;
> +		shsurf->fullscreen.black_surface->output = shsurf->fullscreen_output;
> +		if (!wl_list_empty(&surface->frame_callback_list)) {
> +			wl_list_insert_list(surface->output->frame_callback_list.prev,
> +					    &surface->frame_callback_list);
> +			wl_list_init(&surface->frame_callback_list);
> +		}
> +
> +		weston_compositor_repick(surface->compositor);
> +		weston_surface_damage(surface);
> +		weston_surface_damage(shsurf->fullscreen.black_surface);
> +	}
> +}
> +
> +static void
> +shell_map_fullscreen(struct shell_surface *shsurf)
> +{
> +	shell_configure_fullscreen(shsurf);
> +	shell_stack_fullscreen(shsurf, false);
> +}
> +
> +static void
> +shell_raise_fullscreen(struct shell_surface *shsurf)
> +{
> +	shell_configure_fullscreen(shsurf);
> +	shell_stack_fullscreen(shsurf, true);
> +}
> +
>  static void
>  shell_surface_set_fullscreen(struct wl_client *client,
>  			     struct wl_resource *resource,
> @@ -496,21 +625,28 @@ shell_surface_set_fullscreen(struct wl_client *client,
>  {
>  	struct shell_surface *shsurf = resource->data;
>  	struct weston_surface *es = shsurf->surface;
> -	struct weston_output *output;
> +
> +	if (output_resource)
> +		shsurf->output = output_resource->data;
> +	else
> +		shsurf->output = get_default_output(es->compositor);
>  
>  	if (reset_shell_surface_type(shsurf))
>  		return;
>  
> -	/* FIXME: Fullscreen on first output */
> -	/* FIXME: Handle output going away */
> -	output = get_default_output(es->compositor);
> -
>  	shsurf->saved_x = es->geometry.x;
>  	shsurf->saved_y = es->geometry.y;
> -	shsurf->output = output;
> -	shsurf->fullscreen_output = output;
> +	shsurf->fullscreen_output = shsurf->output;
> +	shsurf->fullscreen.type = method;
> +	shsurf->fullscreen.framerate = framerate;
>  	shsurf->type = SHELL_SURFACE_FULLSCREEN;
>  
> +	if (shsurf->fullscreen.black_surface == NULL)
> +		shsurf->fullscreen.black_surface = create_black_surface(es->compositor);
> +
> +	if (es->output)
> +		shsurf->surface->force_configure = 1;
> +
>  	wl_resource_post_event(resource,
>  			       WL_SHELL_SURFACE_CONFIGURE,
>  			       weston_compositor_get_time(), 0,
> @@ -653,6 +789,9 @@ destroy_shell_surface(struct wl_resource *resource)
>  	if (shsurf->surface)
>  		wl_list_remove(&shsurf->surface_destroy_listener.link);
>  
> +	if (shsurf->fullscreen.black_surface)
> +		weston_surface_destroy(shsurf->fullscreen.black_surface);
> +

I think we should just destroy the black surface in configure() when
we switch away from fullscreen.  It's cheap to create and destroy and
there's no reason to keep it around.

>  	wl_list_remove(&shsurf->link);
>  	free(shsurf);
>  }
> @@ -715,7 +854,15 @@ shell_get_shell_surface(struct wl_client *client,
>  		(void (**)(void)) &shell_surface_implementation;
>  	shsurf->resource.data = shsurf;
>  
> +	shsurf->saved_x = INVALID_X;
> +	shsurf->saved_y = INVALID_Y;

-1,-1 is a valid surface position and we can't use that to indicate
that the surface never received an initial top-level position.  We
could use INT32_MIN, I guess, or just add a saved_position_valid flag
to the struct.

>  	shsurf->surface = surface;
> +	shsurf->fullscreen.type = WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT;
> +	shsurf->fullscreen.framerate = 0;
> +	shsurf->fullscreen.black_surface = NULL;
> +	wl_list_init(&shsurf->fullscreen.transform.link);
> +	weston_matrix_init(&shsurf->fullscreen.transform.matrix);

Shouldn't need to init the matrix here.

> +
>  	shsurf->surface_destroy_listener.func = shell_handle_surface_destroy;
>  	wl_list_insert(surface->surface.resource.destroy_listener_list.prev,
>  		       &shsurf->surface_destroy_listener.link);
> @@ -1274,6 +1421,9 @@ activate(struct weston_shell *base, struct weston_surface *es,
>  				       &es->link);
>  		}
>  		break;
> +	case SHELL_SURFACE_FULLSCREEN:
> +		/* should on top of panels*/
> +		break;
>  	default:
>  		if (!shell->locked) {
>  			list = weston_compositor_top(compositor);
> @@ -1290,14 +1440,22 @@ activate(struct weston_shell *base, struct weston_surface *es,
>  
>  static void
>  click_to_activate_binding(struct wl_input_device *device,
> -                         uint32_t time, uint32_t key,
> +			  uint32_t time, uint32_t key,
>  			  uint32_t button, uint32_t state, void *data)
>  {
>  	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;
> +	upper = container_of(focus->link.prev, struct weston_surface, link);
> +	if (get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN) {
> +		printf("%s: focus is black surface, raise its fullscreen surf\n", __func__);
> +		shell_raise_fullscreen (get_shell_surface(upper));
> +		focus = upper;
> +	}
> +

This causes a crash when I click in a just started compositor (no
clients, except desktop-shell).  You can't just follow the prev
pointer like that, it may point to compositor->surface_list.  Instead
you have to

        if (focus->link.prev != &compositor->surface_list &&
            get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN) {


>  	if (state && focus && device->pointer_grab == &device->default_pointer_grab)
>  		activate(compositor->shell, focus, wd, time);
>  }
> @@ -1438,9 +1596,11 @@ map(struct weston_shell *base, struct weston_surface *surface,
>  					    10 + random() % 400);
>  		break;
>  	case SHELL_SURFACE_SCREENSAVER:
> -	case SHELL_SURFACE_FULLSCREEN:
>  		center_on_output(surface, shsurf->fullscreen_output);
>  		break;
> +	case SHELL_SURFACE_FULLSCREEN:
> +		shell_map_fullscreen(shsurf);
> +		break;
>  	case SHELL_SURFACE_MAXIMIZED:
>  		/*use surface configure to set the geometry*/
>  		panel_height = get_output_panel_height(shell,surface->output);
> @@ -1490,6 +1650,9 @@ map(struct weston_shell *base, struct weston_surface *surface,
>  		}
>  		do_configure = 0;
>  		break;
> +	case SHELL_SURFACE_FULLSCREEN:
> +		do_configure = 0;
> +		break;
>  	case SHELL_SURFACE_NONE:
>  		do_configure = 0;
>  		break;
> @@ -1520,7 +1683,7 @@ map(struct weston_shell *base, struct weston_surface *surface,
>  		if (!shell->locked)
>  			activate(base, surface,
>  				 (struct weston_input_device *)
> -					compositor->input_device,
> +				 compositor->input_device,
>  				 weston_compositor_get_time());
>  		break;
>  	default:
> @@ -1538,6 +1701,7 @@ configure(struct weston_shell *base, struct weston_surface *surface,
>  	struct wl_shell *shell = container_of(base, struct wl_shell, shell);
>  	enum shell_surface_type surface_type = SHELL_SURFACE_NONE;
>  	struct shell_surface *shsurf;
> +	bool do_activate = false;
>  
>  	shsurf = get_shell_surface(surface);
>  	if (shsurf)
> @@ -1551,15 +1715,42 @@ configure(struct weston_shell *base, struct weston_surface *surface,
>  
>  	switch (surface_type) {
>  	case SHELL_SURFACE_SCREENSAVER:
> -	case SHELL_SURFACE_FULLSCREEN:
>  		center_on_output(surface, shsurf->fullscreen_output);
>  		break;
> +	case SHELL_SURFACE_FULLSCREEN:
> +		if (wl_list_empty(&shsurf->fullscreen.black_surface->link)) {
> +			shell_raise_fullscreen(shsurf);
> +			do_activate = true;
> +		} else {
> +			/*just do the positioning*/
> +			shell_configure_fullscreen(shsurf);
> +		}
> +		break;
>  	case SHELL_SURFACE_MAXIMIZED:
>  		/*setting x, y and using configure to change that geometry*/
>  		surface->geometry.x = surface->output->x;
>  		surface->geometry.y = surface->output->y +
>  			get_output_panel_height(shell,surface->output);
>  		break;
> +	case SHELL_SURFACE_TOPLEVEL:
> +		if (!wl_list_empty(&shsurf->fullscreen.black_surface->link)) {

I think we want to save the previous surface type, and then do this in
a separate switch on shsurf->previous_type above this one.  That
switch will undo/tear down whatever the previous surface type was, and
the second switch will be responsible for setting up for the new
surface type.

> +			wl_list_remove(&surface->link);
> +			wl_list_init(&surface->link);
> +			weston_surface_damage_below(shsurf->fullscreen.black_surface);
> +			wl_list_remove(&shsurf->fullscreen.black_surface->link);
> +			wl_list_init(&shsurf->fullscreen.black_surface->link);
> +			shsurf->fullscreen.black_surface->output = NULL;
> +			if (shsurf->saved_x != INVALID_X &&
> +					shsurf->saved_y != INVALID_Y &&
> +					shsurf->saved_x != surface->geometry.x &&
> +					shsurf->saved_y != surface->geometry.y) {
> +				weston_surface_set_position(surface,
> +						shsurf->saved_x,
> +						shsurf->saved_y);
> +			}

If saved_x/y wasn't valid we need to pick an initial position in the else clause of this if statement.

> +			do_activate = true;

Why do we activate when we go from fullscreen to toplevel?

> +		}
> +		break;
>  	default:
>  		break;
>  	}
> @@ -1573,6 +1764,12 @@ configure(struct weston_shell *base, struct weston_surface *surface,
>  		else if (surface_type == SHELL_SURFACE_MAXIMIZED)
>  			surface->output = shsurf->output;
>  	}
> +
> +	if (do_activate && !shell->locked)
> +		activate(surface->compositor->shell, surface,
> +				(struct weston_input_device *)
> +				surface->compositor->input_device,
> +				weston_compositor_get_time());
>  }
>  
>  static int launch_desktop_shell_process(struct wl_shell *shell);
> -- 
> 1.7.5.4
> 


More information about the wayland-devel mailing list