[RFC,weston] compositor: Implement runtime output transform changes

Submitted by Ilia Bozhinov on June 26, 2017, 11:23 a.m.

Details

Message ID 20170626112310.28406-1-ammen99@gmail.com
State New
Headers show
Series "compositor: Implement runtime output transform changes" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Ilia Bozhinov June 26, 2017, 11:23 a.m.
From: Ilia Bozhinov <iliyabo@hotmail.com>

Up to now we could set the transform only on output initialization.
However, on certain situations(like tablets and convertible laptops),
screen orientation can change while the compositor is running and thus
the need for change of the output transform arises.

When the transform changes, we must update the output geometry,
output->region and output->previous_damage, as well as send this change
to clients. We also have to check whether any of the pointers are inside
the output which is being rotated. If this is the case, they are moved
to the new center, because otherwise the pointer is stuck outside of the
screen ans "lost" to the user.

What is more, after calling this function compositors should check if
any view is now outside of the screen and move it according to their
wish.

Signed-off-by: Ilia Bozhinov <iliyabo@hotmail.com>
---
 libweston/compositor.c | 65 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 2a3074db..ca77bdfc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -4587,9 +4587,6 @@  weston_output_set_scale(struct weston_output *output,
  * \param output    The weston_output object that the transform is set for.
  * \param transform Transform value for the given output.
  *
- * It only supports setting transform for an output that is
- * not enabled and it can only be ran once.
- *
  * Refer to wl_output::transform section located at
  * https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_output
  * for list of values that can be passed to this function.
@@ -4598,13 +4595,65 @@  WL_EXPORT void
 weston_output_set_transform(struct weston_output *output,
 			    uint32_t transform)
 {
-	/* We can only set transform on a disabled output */
-	assert(!output->enabled);
+	struct weston_pointer_motion_event ev;
+	struct wl_resource *resource;
+	struct weston_seat *seat;
+	pixman_region32_t old_region;
+	int mid_x, mid_y;
+
+	if (!output->enabled && output->transform == UINT32_MAX) {
+		output->transform = transform;
+		return;
+	}
 
-	/* We only want to set transform once */
-	assert(output->transform == UINT32_MAX);
+	if (transform == output->transform)
+		return;
 
-	output->transform = transform;
+	weston_output_transform_scale_init(output, transform, output->scale);
+
+	pixman_region32_init(&old_region);
+	pixman_region32_copy(&old_region, &output->region);
+
+	pixman_region32_fini(&output->region);
+	pixman_region32_fini(&output->previous_damage);
+
+	weston_output_init_geometry(output, output->x, output->y);
+
+	output->dirty = 1;
+
+	/* Notify clients of the change for output transform. */
+	wl_resource_for_each(resource, &output->resource_list) {
+		wl_output_send_geometry(resource,
+					output->x,
+					output->y,
+					output->mm_width,
+					output->mm_height,
+					output->subpixel,
+					output->make,
+					output->model,
+					output->transform);
+
+		if (wl_resource_get_version(resource) >= WL_OUTPUT_DONE_SINCE_VERSION)
+			wl_output_send_done(resource);
+	}
+
+	/* we must ensure that pointers are inside output, otherwise they disappear */
+	mid_x = output->x + output->width / 2;
+	mid_y = output->y + output->height / 2;
+
+	ev.mask = WESTON_POINTER_MOTION_ABS;
+	ev.x = wl_fixed_to_double(wl_fixed_from_int(mid_x));
+	ev.y = wl_fixed_to_double(wl_fixed_from_int(mid_y));
+
+	wl_list_for_each(seat, &output->compositor->seat_list, link) {
+		struct weston_pointer *pointer = weston_seat_get_pointer(seat);
+
+		if (pointer && pixman_region32_contains_point(&old_region,
+							      wl_fixed_to_int(pointer->x),
+							      wl_fixed_to_int(pointer->y),
+							      NULL))
+			weston_pointer_move(pointer, &ev);
+	}
 }
 
 /** Initializes a weston_output object with enough data so

Comments

On 2017-06-26 06:23 AM, Ilia Bozhinov wrote:
> From: Ilia Bozhinov <iliyabo@hotmail.com>
> 
> Up to now we could set the transform only on output initialization.
> However, on certain situations(like tablets and convertible laptops),
> screen orientation can change while the compositor is running and thus
> the need for change of the output transform arises.
> 
> When the transform changes, we must update the output geometry,
> output->region and output->previous_damage, as well as send this change
> to clients. We also have to check whether any of the pointers are inside
> the output which is being rotated. If this is the case, they are moved
> to the new center, because otherwise the pointer is stuck outside of the
> screen ans "lost" to the user.
> 
> What is more, after calling this function compositors should check if
> any view is now outside of the screen and move it according to their
> wish.
> 
> Signed-off-by: Ilia Bozhinov <iliyabo@hotmail.com>
> ---
>   libweston/compositor.c | 65 +++++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 57 insertions(+), 8 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 2a3074db..ca77bdfc 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4587,9 +4587,6 @@ weston_output_set_scale(struct weston_output *output,
>    * \param output    The weston_output object that the transform is set for.
>    * \param transform Transform value for the given output.
>    *
> - * It only supports setting transform for an output that is
> - * not enabled and it can only be ran once.
> - *
>    * Refer to wl_output::transform section located at
>    * https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_output
>    * for list of values that can be passed to this function.
> @@ -4598,13 +4595,65 @@ WL_EXPORT void
>   weston_output_set_transform(struct weston_output *output,
>   			    uint32_t transform)
>   {
> -	/* We can only set transform on a disabled output */
> -	assert(!output->enabled);
> +	struct weston_pointer_motion_event ev;
> +	struct wl_resource *resource;
> +	struct weston_seat *seat;
> +	pixman_region32_t old_region;
> +	int mid_x, mid_y;
> +
> +	if (!output->enabled && output->transform == UINT32_MAX) {
> +		output->transform = transform;
> +		return;
> +	}
>   
> -	/* We only want to set transform once */
> -	assert(output->transform == UINT32_MAX);
> +	if (transform == output->transform)
> +		return;
>   
> -	output->transform = transform;
> +	weston_output_transform_scale_init(output, transform, output->scale);
> +
> +	pixman_region32_init(&old_region);
> +	pixman_region32_copy(&old_region, &output->region);
> +
> +	pixman_region32_fini(&output->region);
> +	pixman_region32_fini(&output->previous_damage);
> +
> +	weston_output_init_geometry(output, output->x, output->y);
> +
> +	output->dirty = 1;
> +
> +	/* Notify clients of the change for output transform. */
> +	wl_resource_for_each(resource, &output->resource_list) {
> +		wl_output_send_geometry(resource,
> +					output->x,
> +					output->y,
> +					output->mm_width,
> +					output->mm_height,
> +					output->subpixel,
> +					output->make,
> +					output->model,
> +					output->transform);
> +
> +		if (wl_resource_get_version(resource) >= WL_OUTPUT_DONE_SINCE_VERSION)
> +			wl_output_send_done(resource);
> +	}

Some of the above looks like it could maybe be refactored and shared 
with weston_output_move?

Otherwise this looks fine to me:

Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

Daniel, you singled this out for review - does that count as an Ack?

Thanks,
Derek

> +
> +	/* we must ensure that pointers are inside output, otherwise they disappear */
> +	mid_x = output->x + output->width / 2;
> +	mid_y = output->y + output->height / 2;
> +
> +	ev.mask = WESTON_POINTER_MOTION_ABS;
> +	ev.x = wl_fixed_to_double(wl_fixed_from_int(mid_x));
> +	ev.y = wl_fixed_to_double(wl_fixed_from_int(mid_y));
> +
> +	wl_list_for_each(seat, &output->compositor->seat_list, link) {
> +		struct weston_pointer *pointer = weston_seat_get_pointer(seat);
> +
> +		if (pointer && pixman_region32_contains_point(&old_region,
> +							      wl_fixed_to_int(pointer->x),
> +							      wl_fixed_to_int(pointer->y),
> +							      NULL))
> +			weston_pointer_move(pointer, &ev);
> +	}
>   }
>   
>   /** Initializes a weston_output object with enough data so
>
Hi Derek,

On 17 January 2018 at 20:25, Derek Foreman <derekf@osg.samsung.com> wrote:
> Some of the above looks like it could maybe be refactored and shared with
> weston_output_move?
>
> Otherwise this looks fine to me:
>
> Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

Thanks for checking this out!

> Daniel, you singled this out for review - does that count as an Ack?

What I meant by dumping it in my list of links with no comment, is
that I'd looked over it, and it seemed OK, like good low-hanging fruit
to land. It was as much of a reminder to myself to finish review as
anything. It's a wonder no-one decoded that really.

Anyway, I've landed this now with your review and my ack, thanks! :)

Cheers,
Daniel