[PATCH weston 3/4] compositor-drm: Enable seat constraining when configured in weston.ini

[Rob Bradford]

On Tue, Jun 11, 2013 at 03:33:46PM +0300, Pekka Paalanen wrote:
> Hi Rob,
> 
> I think patches 2, 3, and 4 should be all squashed, and I would like to
> know more of what is the use case here.
> 
> More questions below.
> 
> 
> On Mon, 10 Jun 2013 15:17:30 +0100
> Rob Bradford <robert.bradford at intel.com> wrote:
> 
> > From: Rob Bradford <rob at linux.intel.com>
> > 
> > ---
> >  src/compositor-drm.c | 16 ++++++++++++++++
> >  src/compositor.h     |  2 ++
> >  src/input.c          |  2 ++
> >  src/udev-seat.c      |  3 +++
> >  4 files changed, 23 insertions(+)
> > 
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 76d0810..7d33977 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -1796,6 +1796,22 @@ create_output_for_connector(struct drm_compositor *ec,
> >  	transform = parse_transform(s, output->base.name);
> >  	free(s);
> >  
> > +	weston_config_section_get_string(section, "seat", &s, "");
> > +	if (strcmp(s, "") != 0) {
> > +		struct udev_seat *seat;
> > +
> > +		seat = udev_seat_get_named(&ec->base, s);
> > +		if (seat) {
> > +			seat->base.output = &output->base;
> > +			if (seat->base.pointer) {
> > +				weston_pointer_output_center(seat->base.pointer,
> > +							     &output->base);

Hi Pekka,

Thanks for the feedback below. I'll do my best to answer everything.

> Does this mean, that every time an output is hotplugged in this seat,
> the pointer will warp?
> 
> What if a seat has several outputs?
> 

Excellent point. I think I could instead go with a
weston_pointer_ensure_valid_location.

This function would be based off of the code in pointer_clip_motion()
and then that function could be written in terms of this.

Thus warping would only happen if the seat was previously unconstrained
and the output that came about wanted it constrained to it.

> > +	if (seat->base.output && seat->base.pointer)
> > +		weston_pointer_output_center(seat->base.pointer, seat->base.output);
> 
> Do I understand right, that whenever any input device is hotplugged,
> like a keyboard, the pointer will warp?

Yeh I think you're right here. The function I mentioned above could
probably fix this too. But it would be nice to be cleverer about when to
call this function

> > +
> >  	return 0;
> >  }
> >  
> 
> From the man page patch:
> 
> +.BI "seat=" name
> +The logical seat name that that this output should be associated with. If this
> +is set then the seat's input will be confined to the output that has the seat
> +set on it. The expectation is that this functionality will be used in a
> +multiheaded environment with a single compositor for multiple output and
> +input configurations.
> 
> So, this is an output key. It is almost as if you were trying to
> implement seats by using wl_seats. I don't think that will work. Like
> here, you try to confine the pointer, but what if a user alt+tabs? No
> constraints there. I'm confused what this is trying to achieve.
> 

> Let me explain. I believe that wl_seat is misnamed. wl_seat is a set of
> foci for a set on input devices, used by one person, or by several
> people collaborating on a common desktop. Everyone has access to
> everything, using their own input devices.
> 
> A seat is a physical entity, a console; an output or outputs, and the
> associated input devices. Each seat has its own user session(s), which
> is easiest to implement by running a compositor per seat. Sessions are
> separate by definition, and for security. Separating the sessions within
> a compositor is very hard.
> 
> I think, that seats are configured in udev, and wl_seats will be
> configured in weston.ini or equivalent per-session configuration. A seat
> may have multiple wl_seats, if you want e.g. multi-pointer. Currently,
> we have no implementation for configuring multiple wl_seats;
> device_added() will ignore all devices that do not match the wanted
> ID_SEAT property, and only one udev_input with a wanted seat_id is
> created in a backend.
> 
> Could you elaborate on what you are trying to do?

The requirement we have for this this is to be able to run a
single compositor (with it's own custom shell) that would be driving
multiple outputs using the same GPU. Clients might need to multiple
outputs or move outputs - so having multiple compositor instances would
be complicated. (Also can we have multiple compositor instances using
the same GPU - and driving different outputs....?)

Each of these outputs has some input devices associated with it -
probably a touch screen and perhaps some buttons. Obviously if a
touchscreen is part of a particular physical display

In my patches that are now merged I added support to create multiple
udev_seats based on labelling the devices in udev with ENV{WL_SEAT}.

The goal of these combined patch sets is to be able to create multiple
independent logical seats with some primitive isolation: touch and
pointer confinement. Of course the custom shell will need to make sure
client surface appear on the correct outputs as well as making sure
things like "Alt-Tab" you mentioned don't happen.

Do you think improving the terminology - using "logical seat" to refer
to the soft weston seat would be helpful.

Of course i'll work on avoiding the pointer warping problem.

Rob