[PATCH v2 weston 1/2] shell: Implement the probe_area request on the shell surface

[Pekka Paalanen]

On Fri,  8 Feb 2013 16:52:57 +0000
Rob Bradford <robert.bradford at intel.com> wrote:

> From: Rob Bradford <rob at linux.intel.com>
> 

Hi Rob,

at least this version is not using weston_surface::geometry.{x,y}
directly anymore, thanks for that. Unfortunately, the problem is more
complex, more below.

However, I don't think this is a critical problem, just a bug that could
be recorded in a comment in the code, and have this patch committed
then.

> ---
>  src/shell.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/src/shell.c b/src/shell.c
> index 368fa5b..5bddf12 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -1917,6 +1917,89 @@ shell_surface_set_popup(struct wl_client *client,
>  	shsurf->popup.y = y;
>  }
>  
> +
> +static void
> +shell_surface_probe_area(struct wl_client *client,
> +			 struct wl_resource *resource,
> +			 int x,
> +			 int y,
> +			 int w,
> +			 int h,
> +			 uint32_t result_id)
> +{
> +	struct wl_resource result;
> +	struct shell_surface *shsurf = resource->data;
> +	struct {
> +		struct {
> +			float x;
> +			float y;
> +		} tl, br; /* tl = top left, br = bottom right */
> +	} orig, new;
> +
> +	int output_w, output_h;
> +
> +	float new_x, new_y, new_w, new_h;
> +
> +	output_w = shsurf->surface->output->current->width;
> +	output_h = shsurf->surface->output->current->height;
> +
> +	/* orig and new are in global co-ordinates */
> +	weston_surface_to_global_float(shsurf->surface,
> +				       x, y,
> +				       &orig.tl.x, &orig.tl.y);
> +	weston_surface_to_global_float(shsurf->surface,
> +				       x + w, y + h,
> +				       &orig.br.x, &orig.br.y);
> +

Are you now assuming, that the candidate rectangle in global
coordinates is the axis-aligned rectangle orig.tl - orig.br?
If you consider a case where the surface is rotated 45 degrees, and
w==h, then this rectanle will have zero width or height, which is
obviously wrong. And this is even before any clipping to output region.

Unfortunately, I don't have a good suggestion on how this should be
dealt with. However my intuition says, that instead of transforming
surface coordinates back and forth, you could transform the output
region into the surface coordinate system. Then you have the
axis-aligned candidate rectangle from the client, and four half-planes
to clip it with.

Then the hard question is, how should one squeeze the polygon resulting
from clipping into the axis-aligned rectangle format you need to send
back to the client? It's an ill-posed problem, since there will often
be a whole family of solutions, so we need some heuristics here to
pick one. I didn't think it further, but it probably needs to account
for how a client will use the information, so that the whole
surface positioning algorithm (compositor+client) will produce an
acceptable solution in the least number of iterations.

> +	new = orig;
> +
> +	/* Clamp the top left so it is inside */
> +	if (orig.tl.x < 0)
> +	  new.tl.x = 0;
> +	if (orig.tl.y < 0)
> +	  new.tl.y = 0;
> +	if (orig.tl.x > output_w)
> +	  new.tl.x = output_w;
> +	if (orig.tl.y > output_h)
> +	  new.tl.y = output_h;
> +
> +	/* Clamp the bottom right so it is inside */
> +	if (orig.br.x < 0)
> +	  new.br.x = 0;
> +	if (orig.br.y < 0)
> +	  new.br.y = 0;
> +	if (orig.br.x > output_w)
> +	  new.br.x = output_w;
> +	if (orig.br.y > output_h)
> +	  new.br.y = output_h;

You need to account for weston_output::{x,y}, too, i.e. convert global
coordinates into output coordinates, since you are clipping in output
coordinate system.

> +
> +	/* Translate back into relative co-ordinates */
> +	weston_surface_from_global_float(shsurf->surface,
> +					 orig.tl.x, orig.tl.y,
> +					 &new_x, &new_y);
> +
> +	/* and relative size */
> +	new_w = new.br.x - ceilf(new.tl.x);
> +	new_h = new.br.y - ceilf(new.tl.y);
> +
> +	memset(&result, 0, sizeof(result));
> +
> +	result.object.interface = &wl_probe_result_interface;
> +	result.object.id = result_id;
> +	result.destroy = NULL;
> +	result.client = client;
> +	result.data = NULL;
> +
> +	wl_client_add_resource(client, &result);
> +
> +	wl_resource_post_event(&result,
> +			       WL_PROBE_RESULT_VISIBLE_AREA,
> +			       (int)ceilf(new_x), (int)ceilf(new_y),
> +			       (int)floorf(new_w), (int)floorf(new_h));
> +
> +	wl_resource_destroy(&result);
> +}
> +
>  static const struct wl_shell_surface_interface shell_surface_implementation = {
>  	shell_surface_pong,
>  	shell_surface_move,
> @@ -1927,7 +2010,8 @@ static const struct wl_shell_surface_interface shell_surface_implementation = {
>  	shell_surface_set_popup,
>  	shell_surface_set_maximized,
>  	shell_surface_set_title,
> -	shell_surface_set_class
> +	shell_surface_set_class,
> +	shell_surface_probe_area
>  };
>  
>  static void

Thanks,
pq