[weston,2/4] weston: Move output position setting to compositor

Submitted by Armin Krezović on Sept. 30, 2016, 9:25 p.m.

Details

Message ID 20160930212530.25850-3-krezovic.armin@gmail.com
State New
Headers show
Series "RFC: Output layout configuration" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Armin Krezović Sept. 30, 2016, 9:25 p.m.
This moves current output positioning code and scale/transform
application to the compositor itself, so the compositor
can configure output layouts any way it wants.

A helper function for setting x and y coordinates is also
added, and couple of assertions to weston_output_enable()
as well, to make sure everything has been set up.

Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
---
 compositor/main.c      | 26 ++++++++++++++++++++++++++
 libweston/compositor.c | 40 ++++++++++++++++++++++------------------
 libweston/compositor.h |  3 +++
 3 files changed, 51 insertions(+), 18 deletions(-)

Patch hide | download patch | download mbox

diff --git a/compositor/main.c b/compositor/main.c
index fca9778..503016e 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -468,6 +468,22 @@  wet_init_parsed_options(struct weston_compositor *ec)
 	return config;
 }
 
+static void
+wet_set_output_position(struct weston_output *output)
+{
+	struct weston_output *iterator;
+	int x = 0, y = 0;
+
+	iterator = container_of(output->compositor->output_list.prev,
+				struct weston_output, link);
+
+	if (!wl_list_empty(&output->compositor->output_list))
+		x = iterator->x + iterator->width;
+
+	weston_output_transform_scale_init(output);
+	weston_output_set_position(output, x, y);
+}
+
 WL_EXPORT struct weston_config *
 wet_get_config(struct weston_compositor *ec)
 {
@@ -1082,6 +1098,8 @@  wet_configure_windowed_output_from_config(struct weston_output *output,
 		return -1;
 	}
 
+	wet_set_output_position(output);
+
 	weston_output_enable(output);
 
 	return 0;
@@ -1167,6 +1185,8 @@  drm_backend_output_configure(struct wl_listener *listener, void *data)
 	api->set_seat(output, seat);
 	free(seat);
 
+	wet_set_output_position(output);
+
 	weston_output_enable(output);
 }
 
@@ -1314,6 +1334,8 @@  rdp_backend_output_configure(struct wl_listener *listener, void *data)
 		return;
 	}
 
+	wet_set_output_position(output);
+
 	weston_output_enable(output);
 }
 
@@ -1388,6 +1410,8 @@  fbdev_backend_output_configure(struct wl_listener *listener, void *data)
 	wet_output_set_transform(output, section, WL_OUTPUT_TRANSFORM_NORMAL, UINT32_MAX);
 	weston_output_set_scale(output, 1);
 
+	wet_set_output_position(output);
+
 	weston_output_enable(output);
 }
 
@@ -1538,6 +1562,8 @@  wayland_backend_output_configure_hotplug(struct wl_listener *listener, void *dat
 {
 	struct weston_output *output = data;
 
+	wet_set_output_position(output);
+
 	/* This backend has all values hardcoded, so nothing can be configured here */
 	weston_output_enable(output);
 }
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 7ebc08c..fdb2089 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -4421,6 +4421,13 @@  weston_output_set_transform(struct weston_output *output,
 	output->transform = transform;
 }
 
+WL_EXPORT void
+weston_output_set_position(struct weston_output *output, int x, int y)
+{
+	output->x = x;
+	output->y = y;
+}
+
 /** Initializes a weston_output object with enough data so
  ** an output can be configured.
  *
@@ -4441,6 +4448,7 @@  weston_output_init(struct weston_output *output,
 	assert(output->name);
 
 	wl_list_init(&output->link);
+	wl_signal_init(&output->destroy_signal);
 
 	output->enabled = false;
 
@@ -4454,6 +4462,11 @@  weston_output_init(struct weston_output *output,
 	output->transform = UINT32_MAX;
 
 	output->current_mode = NULL;
+
+	output->width = 0;
+	output->height = 0;
+	output->x = -1;
+	output->y = -1;
 }
 
 /** Adds weston_output object to pending output list.
@@ -4476,11 +4489,8 @@  weston_compositor_add_pending_output(struct weston_output *output,
  *
  * \param output The weston_output object that needs to be enabled.
  *
- * Output coordinates are calculated and each new output is by default
- * assigned to the right of previous one.
- *
- * Sets up the transformation, zoom, and geometry of the output using
- * the properties that need to be configured by the compositor.
+ * Sets up the zoom and geometry of the output using the properties
+ * that need to be configured by the compositor.
  *
  * Establishes a repaint timer for the output with the relevant display
  * object's event loop. See output_repaint_timer_handler().
@@ -4508,42 +4518,36 @@  WL_EXPORT int
 weston_output_enable(struct weston_output *output)
 {
 	struct weston_compositor *c = output->compositor;
-	struct weston_output *iterator;
 	struct wl_event_loop *loop;
-	int x = 0, y = 0;
 
 	assert(output->enable);
 
-	iterator = container_of(c->output_list.prev,
-				struct weston_output, link);
-
-	if (!wl_list_empty(&c->output_list))
-		x = iterator->x + iterator->width;
-
 	/* Make sure the scale is set up */
 	assert(output->scale);
 
 	/* Make sure we have a transform set */
 	assert(output->transform != UINT32_MAX);
 
+	/* Make sure output size has been calculated. */
+	assert(output->width > 0 && output->height > 0);
+
+	/* Make sure position has been set. */
+	assert(output->x >= 0 && output->y >= 0);
+
 	/* Remove it from pending/disabled output list */
 	wl_list_remove(&output->link);
 
 	/* Verify we haven't reached the limit of 32 available output IDs */
 	assert(ffs(~c->output_id_pool) > 0);
 
-	output->x = x;
-	output->y = y;
 	output->dirty = 1;
 
-	weston_output_transform_scale_init(output);
 	weston_output_init_zoom(output);
 
-	weston_output_init_geometry(output, x, y);
+	weston_output_init_geometry(output, output->x, output->y);
 	weston_output_damage(output);
 
 	wl_signal_init(&output->frame_signal);
-	wl_signal_init(&output->destroy_signal);
 	wl_list_init(&output->animation_list);
 	wl_list_init(&output->resource_list);
 	wl_list_init(&output->feedback_list);
diff --git a/libweston/compositor.h b/libweston/compositor.h
index a39b327..2eb7442 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1816,6 +1816,9 @@  weston_output_set_transform(struct weston_output *output,
 			    uint32_t transform);
 
 void
+weston_output_set_position(struct weston_output *output, int x, int y);
+
+void
 weston_output_init(struct weston_output *output,
 		   struct weston_compositor *compositor);
 

Comments

On Fri, 30 Sep 2016 23:25:28 +0200
Armin Krezović <krezovic.armin@gmail.com> wrote:

> This moves current output positioning code and scale/transform
> application to the compositor itself, so the compositor
> can configure output layouts any way it wants.
> 
> A helper function for setting x and y coordinates is also
> added, and couple of assertions to weston_output_enable()
> as well, to make sure everything has been set up.
> 
> Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
> ---
>  compositor/main.c      | 26 ++++++++++++++++++++++++++
>  libweston/compositor.c | 40 ++++++++++++++++++++++------------------
>  libweston/compositor.h |  3 +++
>  3 files changed, 51 insertions(+), 18 deletions(-)

Hi,

moving the output positioning out from libweston and into the
compositor is very good. The setter for x,y is good too.

I'm just not sure we should assume that outputs cannot occupy the
negative coordinates. If one hotplugs an output to the left, I'd assume
the existing windows would stay put on the right. If we cannot use
negative coordinates, everything has to be moved to keep them at the
same position from the user point of view.


Thanks,
pq
On 07.10.2016 11:58, Pekka Paalanen wrote:
> On Fri, 30 Sep 2016 23:25:28 +0200
> Armin Krezović <krezovic.armin@gmail.com> wrote:
> 
>> This moves current output positioning code and scale/transform
>> application to the compositor itself, so the compositor
>> can configure output layouts any way it wants.
>>
>> A helper function for setting x and y coordinates is also
>> added, and couple of assertions to weston_output_enable()
>> as well, to make sure everything has been set up.
>>
>> Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
>> ---
>>  compositor/main.c      | 26 ++++++++++++++++++++++++++
>>  libweston/compositor.c | 40 ++++++++++++++++++++++------------------
>>  libweston/compositor.h |  3 +++
>>  3 files changed, 51 insertions(+), 18 deletions(-)
> 
> Hi,
> 
> moving the output positioning out from libweston and into the
> compositor is very good. The setter for x,y is good too.
> 
> I'm just not sure we should assume that outputs cannot occupy the
> negative coordinates. If one hotplugs an output to the left, I'd assume
> the existing windows would stay put on the right. If we cannot use
> negative coordinates, everything has to be moved to keep them at the
> same position from the user point of view.
> 
> 
> Thanks,
> pq
> 

Hi,

I'd rather go for the second approach - move everything to the output it
was on, in case an output gets attached on the left of an output that had
something displayed (weston_output_move() can be easily adapted).

Thanks, Armin.
On Sun, 9 Oct 2016 19:04:19 +0200
Armin Krezović <krezovic.armin@gmail.com> wrote:

> On 07.10.2016 11:58, Pekka Paalanen wrote:
> > On Fri, 30 Sep 2016 23:25:28 +0200
> > Armin Krezović <krezovic.armin@gmail.com> wrote:
> >   
> >> This moves current output positioning code and scale/transform
> >> application to the compositor itself, so the compositor
> >> can configure output layouts any way it wants.
> >>
> >> A helper function for setting x and y coordinates is also
> >> added, and couple of assertions to weston_output_enable()
> >> as well, to make sure everything has been set up.
> >>
> >> Signed-off-by: Armin Krezović <krezovic.armin@gmail.com>
> >> ---
> >>  compositor/main.c      | 26 ++++++++++++++++++++++++++
> >>  libweston/compositor.c | 40 ++++++++++++++++++++++------------------
> >>  libweston/compositor.h |  3 +++
> >>  3 files changed, 51 insertions(+), 18 deletions(-)  
> > 
> > Hi,
> > 
> > moving the output positioning out from libweston and into the
> > compositor is very good. The setter for x,y is good too.
> > 
> > I'm just not sure we should assume that outputs cannot occupy the
> > negative coordinates. If one hotplugs an output to the left, I'd assume
> > the existing windows would stay put on the right. If we cannot use
> > negative coordinates, everything has to be moved to keep them at the
> > same position from the user point of view.
> > 
> > 
> > Thanks,
> > pq
> >   
> 
> Hi,
> 
> I'd rather go for the second approach - move everything to the output it
> was on, in case an output gets attached on the left of an output that had
> something displayed (weston_output_move() can be easily adapted).

I wonder if that is possible in libweston core rather than delegating
to all shells individually.

See e.g. handle_output_move() in shell.c.

You are also going to have to move all input device coordinates, so
that the pointers stay on the same outputs as they used to, and
touch-points that are down do the same.

It doesn't sound easier to me.

Moving the global coordinate frame origin involves much more than just
moving outputs. If you only move outputs, your relative input devices
may warp away from the output they were on. I'm not sure how absolute
input devices like touch screens behave. So the question is, will the
user expect the warp or not?

There are cases when you cannot avoid moving existing outputs and that
is natural. But there are also cases where you could, if you were able
to use negative coordinates (or place the first output at 1M,1M instead
of 0,0).


Thanks,
pq