[PATCH 2/2] shell: account for the subsurfaces when going fullscreen or maximizing

Pekka Paalanen ppaalanen at gmail.com
Tue Feb 26 02:35:08 PST 2013


On Mon, 25 Feb 2013 18:59:51 +0100
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> We must calculate the bounding box of the surface + subsurfaces set and use
> that when maximizing the window or going fullscreen.
> ---
>  src/shell.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 99 insertions(+), 19 deletions(-)
> 
> diff --git a/src/shell.c b/src/shell.c
> index 5273bc1..38249b1 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -1195,6 +1195,81 @@ static const struct wl_pointer_grab_interface resize_grab_interface = {
>  	resize_grab_button,
>  };
>  
> +/*
> + * Returns the width of a surface and all its sub-surfaces, in the surface
> + * coordinates frame. */
> +static int32_t
> +surface_boundingbox_width(struct weston_surface *surface)
> +{
> +	int32_t min_x = 0;
> +	int32_t max_x = surface->geometry.width;
> +	int32_t x;
> +	struct weston_subsurface *subsurface;
> +
> +	wl_list_for_each(subsurface, &surface->subsurface_list, parent_link) {
> +		x = subsurface->position.x;
> +		if (x < min_x)
> +			min_x = x;
> +		x += subsurface->surface->geometry.width;
> +		if (x > max_x)
> +			max_x = x;
> +	}
> +
> +	return max_x - min_x;
> +}
> +
> +static int32_t
> +surface_boundingbox_height(struct weston_surface *surface)
> +{
> +	int32_t min_y = 0;
> +	int32_t max_y = surface->geometry.height;
> +	int32_t y;
> +	struct weston_subsurface *subsurface;
> +
> +	wl_list_for_each(subsurface, &surface->subsurface_list, parent_link) {
> +		y = subsurface->position.y;
> +		if (y < min_y)
> +			min_y = y;
> +		y += subsurface->surface->geometry.height;
> +		if (y > max_y)
> +			max_y = y;
> +	}
> +
> +	return max_y - min_y;
> +}
> +
> +static int32_t
> +surface_boundingbox_left(struct weston_surface *surface)
> +{
> +	int32_t min_x = 0;
> +	int32_t x;
> +	struct weston_subsurface *subsurface;
> +
> +	wl_list_for_each(subsurface, &surface->subsurface_list, parent_link) {
> +		x = subsurface->position.x;
> +		if (x < min_x)
> +			min_x = x;
> +	}
> +
> +	return min_x;
> +}
> +
> +static int32_t
> +surface_boundingbox_top(struct weston_surface *surface)
> +{
> +	int32_t min_y = 0;
> +	int32_t y;
> +	struct weston_subsurface *subsurface;
> +
> +	wl_list_for_each(subsurface, &surface->subsurface_list, parent_link) {
> +		y = subsurface->position.y;
> +		if (y < min_y)
> +			min_y = y;
> +	}
> +
> +	return min_y;
> +}

Hi Giulio,

I think all the above could be replaced with a single function (or a
variable containing the result from) composing a pixman_region32_t from
all the sub-surface rectangles and calling pixman_region32_extents() to
get the bounding box in main surface coordinates.

Your code is correct, this would be just a simplification.

> +
>  static int
>  surface_resize(struct shell_surface *shsurf,
>  	       struct weston_seat *ws, uint32_t edges)
> @@ -1213,8 +1288,8 @@ surface_resize(struct shell_surface *shsurf,
>  		return -1;
>  
>  	resize->edges = edges;
> -	resize->width = shsurf->surface->geometry.width;
> -	resize->height = shsurf->surface->geometry.height;
> +	resize->width = surface_boundingbox_width(shsurf->surface);
> +	resize->height = surface_boundingbox_height(shsurf->surface);
>  
>  	shell_grab_start(&resize->base, &resize_grab_interface, shsurf,
>  			 ws->seat.pointer, edges);
> @@ -1712,6 +1787,7 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
>  	struct weston_surface *surface = shsurf->surface;
>  	struct weston_matrix *matrix;
>  	float scale, output_aspect, surface_aspect, x, y;
> +	int32_t width, height;
>  
>  	if (!shsurf->fullscreen.black_surface)
>  		shsurf->fullscreen.black_surface =
> @@ -1726,6 +1802,9 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
>  		       &shsurf->fullscreen.black_surface->layer_link);
>  	shsurf->fullscreen.black_surface->output = output;
>  
> +	width = surface_boundingbox_width(surface);
> +	height = surface_boundingbox_height(surface);
> +
>  	switch (shsurf->fullscreen.type) {
>  	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT:
>  		if (surface->buffer_ref.buffer)
> @@ -1733,9 +1812,10 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
>  		break;
>  	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_SCALE:
>  		/* 1:1 mapping between surface and output dimensions */
> -		if (output->width == surface->geometry.width &&
> -		    output->height == surface->geometry.height) {
> -			weston_surface_set_position(surface, output->x, output->y);
> +		if (output->width == width &&
> +		    output->height == height) {
> +			weston_surface_set_position(surface, output->x - surface_boundingbox_left(surface),
> +										output->y - surface_boundingbox_top(surface));
>  			break;
>  		}
>  
> @@ -1748,33 +1828,33 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
>  			(float) surface->geometry.height;
>  		if (output_aspect < surface_aspect)
>  			scale = (float) output->width /
> -				(float) surface->geometry.width;
> +				(float) width;
>  		else
>  			scale = (float) output->height /
> -				(float) surface->geometry.height;
> +				(float) height;
>  
>  		weston_matrix_scale(matrix, scale, scale, 1);
>  		wl_list_remove(&shsurf->fullscreen.transform.ptr.link);
>  		wl_list_insert(&surface->geometry.transformation_list,
>  			       &shsurf->fullscreen.transform.ptr.link);
> -		x = output->x + (output->width - surface->geometry.width * scale) / 2;
> -		y = output->y + (output->height - surface->geometry.height * scale) / 2;
> +		x = output->x + (output->width - width * scale) / 2 - surface_boundingbox_left(surface);
> +		y = output->y + (output->height - height * scale) / 2 - surface_boundingbox_top(surface);
>  		weston_surface_set_position(surface, x, y);
>  
>  		break;
>  	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER:
>  		if (shell_surface_is_top_fullscreen(shsurf)) {
>  			struct weston_mode mode = {0,
> -				surface->geometry.width,
> -				surface->geometry.height,
> +				width,
> +				height,
>  				shsurf->fullscreen.framerate};
>  
>  			if (weston_output_switch_mode(output, &mode) == 0) {
>  				weston_surface_configure(shsurf->fullscreen.black_surface,
> -					                 output->x, output->y,
> +					                 output->x - surface_boundingbox_left(surface),
> +									 output->y - surface_boundingbox_top(surface),
>  							 output->width,
>  							 output->height);
> -				weston_surface_set_position(surface, output->x, output->y);
>  				break;
>  			}
>  		}
> @@ -3029,8 +3109,8 @@ center_on_output(struct weston_surface *surface, struct weston_output *output)
>  	int32_t height = weston_surface_buffer_height(surface);
>  	float x, y;
>  
> -	x = output->x + (output->width - width) / 2;
> -	y = output->y + (output->height - height) / 2;
> +	x = output->x + (output->width - width) / 2 - surface_boundingbox_left(surface) / 2;
> +	y = output->y + (output->height - height) / 2 - surface_boundingbox_top(surface) / 2;
>  
>  	weston_surface_configure(surface, x, y, width, height);
>  }
> @@ -3125,8 +3205,8 @@ map(struct desktop_shell *shell, struct weston_surface *surface,
>  	case SHELL_SURFACE_MAXIMIZED:
>  		/* use surface configure to set the geometry */
>  		panel_height = get_output_panel_height(shell,surface->output);
> -		weston_surface_set_position(surface, shsurf->output->x,
> -					    shsurf->output->y + panel_height);
> +		weston_surface_set_position(surface, shsurf->output->x - surface_boundingbox_left(shsurf->surface),
> +					    shsurf->output->y + panel_height - surface_boundingbox_top(shsurf->surface));
>  		break;
>  	case SHELL_SURFACE_POPUP:
>  		shell_map_popup(shsurf);
> @@ -3214,9 +3294,9 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
>  		break;
>  	case SHELL_SURFACE_MAXIMIZED:
>  		/* setting x, y and using configure to change that geometry */
> -		surface->geometry.x = surface->output->x;
> +		surface->geometry.x = surface->output->x - surface_boundingbox_left(surface);
>  		surface->geometry.y = surface->output->y +
> -			get_output_panel_height(shell,surface->output);
> +			get_output_panel_height(shell,surface->output) - surface_boundingbox_top(surface);
>  		break;
>  	case SHELL_SURFACE_TOPLEVEL:
>  		break;

Other than that, looks good.

We probably don't want to bother caching this composite window box, but
compute it when needed like you do. It's relative rarely needed.

With these changes, I'd like to take these two patches to my subsurface
branches and carry them there.


Thanks,
pq


More information about the wayland-devel mailing list