Re: Libinput: Halfkey/Mirrorboard implementation
On Tue, Aug 18, 2015 at 05:47:43PM +0100, Kieran Bingham wrote: > Hi Peter, > > Sorry for the late reply here, I've moved back from South America to England > and had to look in to some of my assumptions! > > On 27 July 2015 at 06:07, Peter Hutterer wrote: > > > > Hi Kieran, > > > > On Fri, Jul 24, 2015 at 04:28:04PM -0500, Kieran Bingham wrote: > > > Hi Guys, > > > > > > I've been working on a personal project to implement one-handed touch > > > typing as an accessibility feature. > > > > > > There are software solutions for this available for Win/MacOS > > > http://www.onehandkeyboard.org, and an (expensive) > > > hardware solution available. There is also a solution which uses xkb > > > mappings to utilise the caps-lock as a modifier, > > > but this didn't meet my needs; along with a couple of outdated other linux > > > alternatives mentioned at > > > http://www.onehandkeyboard.org/linux-one-handed-keyboards/ > > > > so first of all, I appreciate your work. the first question I have here is > > why the current solutions are insufficient, especially the xkb one. > > > The xkb solution is the only real alternate contender here for me. > (Hardware = cost, windows/mac != linux, other solutions = old+broken) > > My difficulties with xkb are in creating a mapping where by the space bar > acts as both a modifier *and* a space bar. right. I don't think xkb will let you do this, at least not as a either/or case depending on e.g. how long you press. you'll need to designate a separate key for it. > > > I had originally started this using XLib hooks to capture all keyboard > > > input and re-introduce keys through XTest. > > > This wasn't the best solution however, and while reading up on wayland, I > > > discovered libinput provides a good > > > location for this work. And using the xf86-input-libinput wrapper for X > > > I'm > > > now using it on my laptop. > > > > > > Anyway, I thought I had got to a stage where I could share some code > > > (although this is still a work in progress) > > > > > > I'd love to hear your feedback as to whether this is something that you > > > would like to integrate, or for me to continue working on. > > > (I'll continue to use it for personal use, but if its desired, I can work > > > on improvements to upstream it) > > > > I'm wondering if libinput is the right position in the stack to handle this. > > libinput is a hw abstraction that makes the hw easier to deal with, more > > complex things should go into the higher layers. > > Exactly - I had assumed (which now I think may be wrong) that libinput was > effectively passing up the equivalent to scancodes which are then modified > by X to perform the keyboard mappings. That's correct, libinput passes evdev codes (see the KEY_* defines in linux/input.h) to the xf86-input-libinput driver which simply adds 8 (historical reasons) and passes that on. in that it's no different to e.g. evdev. > I was hoping to be able to generate > a solution which will act 'under' the keyboard layouts so that it doesn't > matter > what the key is ... it is being mirrored as if it was a hardware mirror. that is possible, but only while you're assuming that the HW is mostly identical, and this is where the difficulty lies. take your normal PC keyboard and it won't matter if the layout up-top is us or fr. take a laptop keyboard and it starts to matter, since the keys are suddenly all over the place. Cheers, Peter > I'd (foolisly) hoped for a single solution that would map on to french azerty > keyboards, just the same as it would map to a us/en qwerty by simply mirroring > under the layout definitions. Now that I've dug deeper into xkb - I see that > it > would of course be more complicated than that anyway. > > > > > > Flipping keyboard layouts is semantically more complicated than what > > libinput knows about the keyboard (e.g. we don't handle keyboard layouts) so > > you'll likely run into a number of issues here when the keyboard layouts > > differ from the us default. That's a nonissue for a local installation, but > > a bigger issue when we'd try to merge this as a generic solution for all > > keyboards. > > > > Understood. > > > > > > having this in xkb is also more discoverable for clients than in libinput > > where it's more or less hidden and done magically. > > This is an important point. > It certainly would need a switch. > The design goal is that the keyboard functions 'normally' except when a > modifier > is held at the same time as a key. However, in my testing, I have > noted that a fast > typist (like me) occasionally holds the space down longer, which would give > them > a surprise mirrored key. > > > > > > > > funnily enough, I can also imagine this as a daemon emulating a uinput > > device that does the mapping on demand to emulate a virtual hardware device. > > In that case everthing would be truly hidden and we'd pretend this is a > > custom hardware device with this functionality by default, so it
Re: [PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
On Tue, Aug 18, 2015 at 06:06:56PM -0700, Bill Spitzak wrote:
> On Mon, Aug 17, 2015 at 10:08 PM, Peter Hutterer
> wrote:
>
> > A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY,
> > initializes
> > as keyboard and then segfaults when we send x/y coordinates - pointer
> > acceleration never initializes.
> >
> > Ignore the events and log a bug instead. This intentionally only papers
> > over
> > the underlying issue, let's wait for a real device to trigger this and then
> > look at the correct solution.
> >
> > +static inline bool
> > +evdev_reject_relative(struct evdev_device *device,
> > + const struct input_event *e,
> > + uint64_t time)
> > +{
> > + struct libinput *libinput = device->base.seat->libinput;
> > +
> > + if ((e->code == REL_X || e->code == REL_Y) &&
> > + (device->seat_caps & EVDEV_DEVICE_POINTER) == 0) {
> > + switch (ratelimit_test(&device->nonpointer_rel_limit)) {
> > + case RATELIMIT_PASS:
> > + log_bug_libinput(libinput,
> > +"REL_X/Y from device '%s', but
> > this device is not a pointer\n",
> > +device->devname);
> > + break;
> > + case RATELIMIT_THRESHOLD:
> > + log_bug_libinput(libinput,
> > +"REL_X/Y event flood from '%s'\n",
> > +device->devname);
> > + break;
> > + case RATELIMIT_EXCEEDED:
> > + break;
> > + }
> > +
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> >
>
> It seems like some kind of ratelimit_log(&limit, "format", ...) function
> might be useful so this is not copied everywhere.
good idea, patch is in flight.
> Also wondering if there is a direct test for "pointer acceleration never
> initializes" and perhaps you should do that test instead.
not the same, we can concievably have a device that is a pointer but does
not have pointer acceleration. So checking for the seat caps here is better,
even though the crash is casued by the accel filters missing.
> > +
> > static inline void
> > evdev_process_relative(struct evdev_device *device,
> >struct input_event *e, uint64_t time)
> > @@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
> > struct normalized_coords wheel_degrees = { 0.0, 0.0 };
> > struct discrete_coords discrete = { 0.0, 0.0 };
> >
> > + if (evdev_reject_relative(device, e, time))
> > + return;
> > +
> > switch (e->code) {
> > case REL_X:
> > if (device->pending_event != EVDEV_RELATIVE_MOTION)
> >
>
> I think it would look better and be clearer if you put the
> evdev_reject_relative call in the REL_X and REL_Y cases of the switch (you
> could also remove the test for REL_X/Y from the function itself).
it's a generic function even though it only handles one case at this
point, see evdev_reject_device() which is similar. The reason is simple:
mental capacity is limited, this approach separates the "when do we reject
events" from "how do we handle events", you only need to care about one at a
time.
Cheers,
Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] Add log_*_ratelimit wrappers
Don't open-code the rate-limited log messages, use a simple wrapper instead.
Signed-off-by: Peter Hutterer
---
src/evdev.c| 37 -
src/libinput-private.h | 14 ++
src/libinput.c | 25 +
3 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 97c007c..303e447 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -647,21 +647,10 @@ evdev_reject_relative(struct evdev_device *device,
if ((e->code == REL_X || e->code == REL_Y) &&
(device->seat_caps & EVDEV_DEVICE_POINTER) == 0) {
- switch (ratelimit_test(&device->nonpointer_rel_limit)) {
- case RATELIMIT_PASS:
- log_bug_libinput(libinput,
-"REL_X/Y from device '%s', but this
device is not a pointer\n",
-device->devname);
- break;
- case RATELIMIT_THRESHOLD:
- log_bug_libinput(libinput,
-"REL_X/Y event flood from '%s'\n",
-device->devname);
- break;
- case RATELIMIT_EXCEEDED:
- break;
- }
-
+ log_bug_libinput_ratelimit(libinput,
+ &device->nonpointer_rel_limit,
+ "REL_X/Y from device '%s', but this
device is not a pointer\n",
+ device->devname);
return true;
}
@@ -1371,20 +1360,10 @@ evdev_device_dispatch(void *data)
rc = libevdev_next_event(device->evdev,
LIBEVDEV_READ_FLAG_NORMAL, &ev);
if (rc == LIBEVDEV_READ_STATUS_SYNC) {
- switch (ratelimit_test(&device->syn_drop_limit)) {
- case RATELIMIT_PASS:
- log_info(libinput, "SYN_DROPPED event from "
-"\"%s\" - some input events have "
-"been lost.\n", device->devname);
- break;
- case RATELIMIT_THRESHOLD:
- log_info(libinput, "SYN_DROPPED flood "
-"from \"%s\"\n",
-device->devname);
- break;
- case RATELIMIT_EXCEEDED:
- break;
- }
+ log_info_ratelimit(libinput,
+ &device->syn_drop_limit,
+ "SYN_DROPPED event from \"%s\" -
some input events have been lost.\n",
+ device->devname);
/* send one more sync event so we handle all
currently pending events before we sync up
diff --git a/src/libinput-private.h b/src/libinput-private.h
index 5d0826d..8b161cc 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -256,6 +256,20 @@ typedef void (*libinput_source_dispatch_t)(void *data);
#define log_bug_libinput(li_, ...) log_msg((li_), LIBINPUT_LOG_PRIORITY_ERROR,
"libinput bug: " __VA_ARGS__)
#define log_bug_client(li_, ...) log_msg((li_), LIBINPUT_LOG_PRIORITY_ERROR,
"client bug: " __VA_ARGS__)
+#define log_debug_ratelimit(li_, r_, ...) log_msg_ratelimit((li_), (r_),
LIBINPUT_LOG_PRIORITY_DEBUG, __VA_ARGS__)
+#define log_info_ratelimit(li_, r_, ...) log_msg_ratelimit((li_), (r_),
LIBINPUT_LOG_PRIORITY_INFO, __VA_ARGS__)
+#define log_error_ratelimit(li_, r_, ...) log_msg_ratelimit((li_), (r_),
LIBINPUT_LOG_PRIORITY_ERROR, __VA_ARGS__)
+#define log_bug_kernel_ratelimit(li_, r_, ...) log_msg_ratelimit((li_), (r_),
LIBINPUT_LOG_PRIORITY_ERROR, "kernel bug: " __VA_ARGS__)
+#define log_bug_libinput_ratelimit(li_, r_, ...) log_msg_ratelimit((li_),
(r_), LIBINPUT_LOG_PRIORITY_ERROR, "libinput bug: " __VA_ARGS__)
+#define log_bug_client_ratelimit(li_, r_, ...) log_msg_ratelimit((li_), (r_),
LIBINPUT_LOG_PRIORITY_ERROR, "client bug: " __VA_ARGS__)
+
+void
+log_msg_ratelimit(struct libinput *libinput,
+ struct ratelimit *ratelimit,
+ enum libinput_log_priority priority,
+ const char *format, ...)
+ LIBINPUT_ATTRIBUTE_PRINTF(4, 5);
+
void
log_msg(struct libinput *libinput,
enum libinput_log_priority priority,
diff --git a/src/libinput.c b/src/libinput.c
index 4673073..e564571 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -166,6 +166,31 @@ log_msg(struct libinput *libinput,
va_end(args);
}
+void
+log_msg_ratelimit(struct libinput *libinput,
+ struct ratelimit *ratelimit,
+ enum libi
Re: [PATCH libinput] evdev: drop relative x/y motion from a device not marked as pointer
On Mon, Aug 17, 2015 at 10:08 PM, Peter Hutterer
wrote:
> A device with REL_X/Y and keys gets marked only as ID_INPUT_KEY,
> initializes
> as keyboard and then segfaults when we send x/y coordinates - pointer
> acceleration never initializes.
>
> Ignore the events and log a bug instead. This intentionally only papers
> over
> the underlying issue, let's wait for a real device to trigger this and then
> look at the correct solution.
>
> +static inline bool
> +evdev_reject_relative(struct evdev_device *device,
> + const struct input_event *e,
> + uint64_t time)
> +{
> + struct libinput *libinput = device->base.seat->libinput;
> +
> + if ((e->code == REL_X || e->code == REL_Y) &&
> + (device->seat_caps & EVDEV_DEVICE_POINTER) == 0) {
> + switch (ratelimit_test(&device->nonpointer_rel_limit)) {
> + case RATELIMIT_PASS:
> + log_bug_libinput(libinput,
> +"REL_X/Y from device '%s', but
> this device is not a pointer\n",
> +device->devname);
> + break;
> + case RATELIMIT_THRESHOLD:
> + log_bug_libinput(libinput,
> +"REL_X/Y event flood from '%s'\n",
> +device->devname);
> + break;
> + case RATELIMIT_EXCEEDED:
> + break;
> + }
> +
> + return true;
> + }
> +
> + return false;
> +}
>
It seems like some kind of ratelimit_log(&limit, "format", ...) function
might be useful so this is not copied everywhere.
Also wondering if there is a direct test for "pointer acceleration never
initializes" and perhaps you should do that test instead.
> +
> static inline void
> evdev_process_relative(struct evdev_device *device,
>struct input_event *e, uint64_t time)
> @@ -645,6 +675,9 @@ evdev_process_relative(struct evdev_device *device,
> struct normalized_coords wheel_degrees = { 0.0, 0.0 };
> struct discrete_coords discrete = { 0.0, 0.0 };
>
> + if (evdev_reject_relative(device, e, time))
> + return;
> +
> switch (e->code) {
> case REL_X:
> if (device->pending_event != EVDEV_RELATIVE_MOTION)
>
I think it would look better and be clearer if you put the
evdev_reject_relative call in the REL_X and REL_Y cases of the switch (you
could also remove the test for REL_X/Y from the function itself).
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Libinput: Halfkey/Mirrorboard implementation
Hi Peter, Sorry for the late reply here, I've moved back from South America to England and had to look in to some of my assumptions! On 27 July 2015 at 06:07, Peter Hutterer wrote: > > Hi Kieran, > > On Fri, Jul 24, 2015 at 04:28:04PM -0500, Kieran Bingham wrote: > > Hi Guys, > > > > I've been working on a personal project to implement one-handed touch > > typing as an accessibility feature. > > > > There are software solutions for this available for Win/MacOS > > http://www.onehandkeyboard.org, and an (expensive) > > hardware solution available. There is also a solution which uses xkb > > mappings to utilise the caps-lock as a modifier, > > but this didn't meet my needs; along with a couple of outdated other linux > > alternatives mentioned at > > http://www.onehandkeyboard.org/linux-one-handed-keyboards/ > > so first of all, I appreciate your work. the first question I have here is > why the current solutions are insufficient, especially the xkb one. The xkb solution is the only real alternate contender here for me. (Hardware = cost, windows/mac != linux, other solutions = old+broken) My difficulties with xkb are in creating a mapping where by the space bar acts as both a modifier *and* a space bar. > > > > I had originally started this using XLib hooks to capture all keyboard > > input and re-introduce keys through XTest. > > This wasn't the best solution however, and while reading up on wayland, I > > discovered libinput provides a good > > location for this work. And using the xf86-input-libinput wrapper for X I'm > > now using it on my laptop. > > > > Anyway, I thought I had got to a stage where I could share some code > > (although this is still a work in progress) > > > > I'd love to hear your feedback as to whether this is something that you > > would like to integrate, or for me to continue working on. > > (I'll continue to use it for personal use, but if its desired, I can work > > on improvements to upstream it) > > I'm wondering if libinput is the right position in the stack to handle this. > libinput is a hw abstraction that makes the hw easier to deal with, more > complex things should go into the higher layers. Exactly - I had assumed (which now I think may be wrong) that libinput was effectively passing up the equivalent to scancodes which are then modified by X to perform the keyboard mappings. I was hoping to be able to generate a solution which will act 'under' the keyboard layouts so that it doesn't matter what the key is ... it is being mirrored as if it was a hardware mirror. I'd (foolisly) hoped for a single solution that would map on to french azerty keyboards, just the same as it would map to a us/en qwerty by simply mirroring under the layout definitions. Now that I've dug deeper into xkb - I see that it would of course be more complicated than that anyway. > > Flipping keyboard layouts is semantically more complicated than what > libinput knows about the keyboard (e.g. we don't handle keyboard layouts) so > you'll likely run into a number of issues here when the keyboard layouts > differ from the us default. That's a nonissue for a local installation, but > a bigger issue when we'd try to merge this as a generic solution for all > keyboards. > Understood. > > having this in xkb is also more discoverable for clients than in libinput > where it's more or less hidden and done magically. This is an important point. It certainly would need a switch. The design goal is that the keyboard functions 'normally' except when a modifier is held at the same time as a key. However, in my testing, I have noted that a fast typist (like me) occasionally holds the space down longer, which would give them a surprise mirrored key. > > > funnily enough, I can also imagine this as a daemon emulating a uinput > device that does the mapping on demand to emulate a virtual hardware device. > In that case everthing would be truly hidden and we'd pretend this is a > custom hardware device with this functionality by default, so it'd sit below > libinput. This is sort of what my earlier solution did with XLib/XTest. Disconnected the input source, and re-injected after modification. > > > long term though I think the xkb solution would be the best, so again: why > wasn't xkb sufficient? > In summary, I haven't been able to map the space bar to act as a modifier to flip the keyboard, and a space bar when pressed on its own. However, I agree with your responses, and have spent a lot of today sinking my head back into xkb mappings. Now that my head hurts, I've signed up to the xkb mailinglist over at https://groups.google.com/a/listserv.bat.ru/forum/#!forum/xkb and will beg for assistance there :) It may be that I need to extend the compat actions to support a modifier/action key somehow - but I haven't got my head round it yet. Regards Kieran ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.or
Re: [PATCH libinput] tablet: Do not create a tablet device for Wacom touchscreens
On 8/17/2015 7:21 PM, Peter Hutterer wrote:
> On Mon, Aug 17, 2015 at 05:29:14PM -0700, Jason Gerecke wrote:
>> Similar to the issue mentioned in the commit message of 2365f7d, libwacom
>> assigns both ID_INPUT_TABLET and ID_INPUT_TOUCHSCREEN to touchscreens like
>> the Cintiq 24HDT. This patch ensures that neither touchpads nor touchscreens
>> will accidentally be handled by the tablet code.
>
> I'm confused, isnt this the same as 10ca39cf80698cedf92?
>
> Cheers,
>Peter
>
Looks like my wires got crossed. I was apparently working off of your
Github repository instead of FDO. Pardon the noise.
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours
>>
>> Signed-off-by: Jason Gerecke
>> ---
>> src/evdev.c | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/evdev.c b/src/evdev.c
>> index bbc3dce..a4bdb9a 100644
>> --- a/src/evdev.c
>> +++ b/src/evdev.c
>> @@ -1495,8 +1495,10 @@ evdev_configure_device(struct evdev_device *device)
>> /* libwacom assigns touchpad _and_ tablet to the tablet touch bits,
>> so make sure we don't initialize the tablet interface for the
>> touch device */
>> -if ((udev_tags & (EVDEV_UDEV_TAG_TABLET|EVDEV_UDEV_TAG_TOUCHPAD)) ==
>> - EVDEV_UDEV_TAG_TABLET) {
>> +if ((udev_tags & (EVDEV_UDEV_TAG_TABLET |
>> + EVDEV_UDEV_TAG_TOUCHPAD |
>> + EVDEV_UDEV_TAG_TOUCHSCREEN)) ==
>> +EVDEV_UDEV_TAG_TABLET) {
>> device->dispatch = evdev_tablet_create(device);
>> device->seat_caps |= EVDEV_DEVICE_TABLET;
>> log_info(libinput,
>> --
>> 2.5.0
>>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not removed from list of ivi_layer when the ivi_surface is removed from the compositor.
Hi Tanibata-san,
Tel. +49 5121 49 6937
> -Original Message-
> From: wayland-devel [mailto:wayland-devel-
> boun...@lists.freedesktop.org] On Behalf Of Tanibata, Nobuhiko
> (ADITJ/SWG)
> Sent: Montag, 17. August 2015 08:01
> To: Pekka Paalanen; Nobuhiko Tanibata
> Cc: Ishikawa, Tetsuri (ADITJ/SWG); securitych...@denso.co.jp; wayland-
> de...@lists.freedesktop.org
> Subject: RE: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not
> removed from list of ivi_layer when the ivi_surface is removed from the
> compositor.
>
>
>
> > -Original Message-
> > From: wayland-devel
> > [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of
> > Pekka Paalanen
> > Sent: Thursday, August 13, 2015 10:21 PM
> > To: Nobuhiko Tanibata
> > Cc: securitych...@denso.co.jp; wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is
> > not removed from list of ivi_layer when the ivi_surface is removed
> > from the compositor.
> >
> > On Fri, 7 Aug 2015 09:47:02 +0900
> > Nobuhiko Tanibata wrote:
> >
> > > The api, ivi_layout_layer_remove_surface, shall remove a ivi_surface
> > > from a list of ivi_layer. In previous code, there is no trigger to
> > > refresh order of list, removing the ivi_surface, in commit_layer_list.
> > >
> > > To fix this bug, set a mask; IVI_NOTIFICATION_REMOVE in order to
> > > trigger refresh list of surface in commit_layer_list.
> > >
> > > In commit_layer_list, this patch also removes duplicated code in two
> > > conditions for IVI_NOTIFICATION_ADD/REMOVE.
> > >
> > > Signed-off-by: Nobuhiko Tanibata
> > >
> > > ---
> > > v2 changes:
> > > - fix 8 spaces to tab.
> > > - clean up duplicate code in commit_layer_list.
> > > - improve commit message.
> > >
> > > ivi-shell/ivi-layout.c | 28 +---
> > > 1 file changed, 9 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c index
> > > 2974bb7..1b45003 100644
> > > --- a/ivi-shell/ivi-layout.c
> > > +++ b/ivi-shell/ivi-layout.c
> > > @@ -812,25 +812,7 @@ commit_layer_list(struct ivi_layout *layout)
> > > if (!(ivilayer->event_mask &
> > > (IVI_NOTIFICATION_ADD |
> > IVI_NOTIFICATION_REMOVE)) ) {
> > > continue;
> >
> > Hi Tanibata-san,
> >
> > using 'continue', there is no need to have an else-branch. This would
> > save one level of indent in the remaining code.
> >
> > > - }
> > > -
> > > - if (ivilayer->event_mask & IVI_NOTIFICATION_REMOVE) {
> > > - wl_list_for_each_safe(ivisurf, next,
> > > - &ivilayer->order.surface_list,
> > order.link) {
> > > -
> > remove_ordersurface_from_layer(ivisurf);
> > > -
> > > - if
> > (!wl_list_empty(&ivisurf->order.link)) {
> > > -
> > wl_list_remove(&ivisurf->order.link);
> > > - }
> > > -
> > > - wl_list_init(&ivisurf->order.link);
> > > - ivisurf->event_mask |=
> > IVI_NOTIFICATION_REMOVE;
> >
> > You are removing this setting of the flag IVI_NOTIFICATION_REMOVE, but
> > in the code that remains I do not see that happening anymore in
> > commit_layer_list().
> >
> > However, I see it being set by ivi_layout_layer_remove_surface(). At
> > which time should the flag be set? Should the ADD flag not work the same
> way?
> >
> > Would it be essential to flag ivi-surfaces that get removed from
> > ivilayer->order.surface_list with IVI_NOTIFICATION_REMOVE, and
> > ivilayer->surfaces
> > that are added to flag with IVI_NOTIFICATION_ADD, and all remaining
> > surfaces even if they are reordered not be flagged at all?
> >
> > Or is it intended that all surfaces that are originally in the
> > ivilayer->order.surface_list are flagged as REMOVE, and all surfaces
> > ivilayer->in
> > the pending list are flagged as ADD? That would imply that surfaces
> > that were and still remain in the order list are flagged as both REMOVE and
> ADD.
> >
> > > - }
> > > -
> > > - wl_list_init(&ivilayer->order.surface_list);
> > > - }
> > > -
> > > - if (ivilayer->event_mask & IVI_NOTIFICATION_ADD) {
> > > + } else {
> > > wl_list_for_each_safe(ivisurf, next,
> > >
> > &ivilayer->order.surface_list, order.link) {
> > >
> > remove_ordersurface_from_layer(ivisurf);
> > > @@ -843,6 +825,13 @@ commit_layer_list(struct ivi_layout *layout)
> > > }
> > >
> > > wl_list_init(&ivilayer->order.surface_list);
> > > +
> > > + /**
> > > + * Following ivilayer->pending.surface_list must
> > be maintained by
> > > + * a function who will set these masks. Order of
> > surfaces in a
> > > + * layer is restructured here. if there is no
> > surface in
> > > + * surface_list, the following code is skipped.
> > > +
Re: [PATCH wayland] Revert "client: require WAYLAND_DISPLAY to be set"
All of these arguments makes sense, so I guess I agree with reverting this change. I didn't know about the goal of reducing the number of environment variables. Also, the fact that wayland displays are per user makes it different from the X11 displays. Sorry for all the trouble! On Tue, Aug 18, 2015 at 12:34 AM, Sjoerd Simons < sjoerd.sim...@collabora.co.uk> wrote: > On Mon, 2015-08-17 at 09:58 -0700, Bill Spitzak wrote: > > On Mon, Aug 17, 2015 at 7:48 AM, Ray Strode > > wrote: > > > > > Hi, > > > > > > > This reverts commit fb7e13021730d0a5516ecbd3712ea4235e05d24d. > > > > > > thanks, you've got my vote. > > > > > > Acked-by: Ray Strode > > > > > > --Ray > > > > > > Seems right to me question about the method of nesting Wayland in X > > and X > > in wayland; > > > > > The original problem of running Weston in a window in an existing > > > GNOME > > > X11 session and getting applications unintentionally launched into > > > Weston can be circumvented by letting Weston use a non-default > > > socket > > > name, leaving wayland-0 unused. > > > > If Wayland is already running and using wayland-0 this would prevent > > these > > programs from using the X11 api. For instance if you wished to test > > the X11 > > api inside a Wayland session. > > Not really it just means you require a way to override the order in > which they try their display backends (which e.g. for Gtk+ is > possible). As Ray previously mentioned, keeping environment variables > for corner cases is entirely fine. > > > Would it make sense that if DISPLAY is set and WAYLAND_DISPLAY is not > > set, > > that a program capable of doing both should prefer the X11 api? This > > would > > mean that I could force use of the X11 api by unsetting > > WAYLAND_DISPLAY. > > That seems rather fragile (and really up to the toolkit/program to > implement, wayland can't know if the program supports X11). > > > This will require changes to the client (unless the wayland connect > was > > changed to fail in this case, which I suspect is not a good idea) > > which > > probably explains why the idea of renaming the socket was suggested. > > > > As for the patch, I agree. The Foundry software would check if > > DISPLAY was > > not set, and set it directly to ":0", so that X would work always (it > > did > > not open the display by name because it wanted to fix child programs > > as > > well). This was commercial software and this was demanded by qc. So > > effectively they wanted X11 to work without an environment variable. > > the problem with DISPLAY is that it's not namespaced per user, so if > you have two users logged in on an X graphical session one will have > e.g. DISPLAY=:0 the other DISPLAY=:1. Because of that defaulting to :0 > on X isn't a great idea on multi-user systems. > > However for wayland, the sockets are namespaced per user as they reside > in their respective XDG_RUNTIME_DIR, which means two people can start a > graphical session and both use wayland-0 for their compositor just > fine. > > -- > Sjoerd Simons > Collabora Ltd. > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] Revert "client: require WAYLAND_DISPLAY to be set"
On Mon, 2015-08-17 at 09:58 -0700, Bill Spitzak wrote: > On Mon, Aug 17, 2015 at 7:48 AM, Ray Strode > wrote: > > > Hi, > > > > > This reverts commit fb7e13021730d0a5516ecbd3712ea4235e05d24d. > > > > thanks, you've got my vote. > > > > Acked-by: Ray Strode > > > > --Ray > > > Seems right to me question about the method of nesting Wayland in X > and X > in wayland; > > > The original problem of running Weston in a window in an existing > > GNOME > > X11 session and getting applications unintentionally launched into > > Weston can be circumvented by letting Weston use a non-default > > socket > > name, leaving wayland-0 unused. > > If Wayland is already running and using wayland-0 this would prevent > these > programs from using the X11 api. For instance if you wished to test > the X11 > api inside a Wayland session. Not really it just means you require a way to override the order in which they try their display backends (which e.g. for Gtk+ is possible). As Ray previously mentioned, keeping environment variables for corner cases is entirely fine. > Would it make sense that if DISPLAY is set and WAYLAND_DISPLAY is not > set, > that a program capable of doing both should prefer the X11 api? This > would > mean that I could force use of the X11 api by unsetting > WAYLAND_DISPLAY. That seems rather fragile (and really up to the toolkit/program to implement, wayland can't know if the program supports X11). > This will require changes to the client (unless the wayland connect > was > changed to fail in this case, which I suspect is not a good idea) > which > probably explains why the idea of renaming the socket was suggested. > > As for the patch, I agree. The Foundry software would check if > DISPLAY was > not set, and set it directly to ":0", so that X would work always (it > did > not open the display by name because it wanted to fix child programs > as > well). This was commercial software and this was demanded by qc. So > effectively they wanted X11 to work without an environment variable. the problem with DISPLAY is that it's not namespaced per user, so if you have two users logged in on an X graphical session one will have e.g. DISPLAY=:0 the other DISPLAY=:1. Because of that defaulting to :0 on X isn't a great idea on multi-user systems. However for wayland, the sockets are namespaced per user as they reside in their respective XDG_RUNTIME_DIR, which means two people can start a graphical session and both use wayland-0 for their compositor just fine. -- Sjoerd Simons Collabora Ltd. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] Revert "client: require WAYLAND_DISPLAY to be set"
On Mon, 2015-08-17 at 16:17 +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen
>
> This reverts commit fb7e13021730d0a5516ecbd3712ea4235e05d24d.
Acked-By: Sjoerd Simons
> Developers have been trying to reduce the number of by default
> required
> environment variables, and the mentioned commit is a step backwards
> in
> that sense. The fundamental assumption is that a user has only one
> main
> (Wayland) display server where all programs should connect to by
> default, and do so with an a priori known socket name.
>
> The commit also broke various use cases in the wild, some
> accidentally
> due to other causes, some intentionally. This revert allows those use
> cases to continue.
>
> The original problem of running Weston in a window in an existing
> GNOME
> X11 session and getting applications unintentionally launched into
> Weston can be circumvented by letting Weston use a non-default socket
> name, leaving wayland-0 unused.
>
> Discussion:
> http://lists.freedesktop.org/archives/wayland-devel/2015
> -August/023927.html
> http://lists.freedesktop.org/archives/wayland-devel/2015
> -August/023937.html
>
> Cc: Dima Ryazanov
> Cc: Giulio Camuffo
> Cc: Daniel Stone
> Cc: Jasper St. Pierre
> Cc: Ryo Munakata
> Cc: Ray Strode
> Cc: Peter Hutterer
> Cc: Matthias Clasen
> Cc: Sjoerd Simons
> Signed-off-by: Pekka Paalanen
> ---
> doc/man/wl_display_connect.xml| 5 +++--
> doc/publican/sources/Protocol.xml | 8
> src/wayland-client.c | 10 --
> 3 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/doc/man/wl_display_connect.xml
> b/doc/man/wl_display_connect.xml
> index ded3cbd..7e6e05c 100644
> --- a/doc/man/wl_display_connect.xml
> +++ b/doc/man/wl_display_connect.xml
> @@ -57,8 +57,9 @@
>that was previously opened by a Wayland server. The server
> socket must
>be placed in XDG_RUNTIME_DIR for this
> function to
>find it. The name argument specifies
> the name of
> - the socket or NULL to use the default
> - (which is the value of WAYLAND_DISPLAY). If
> + the socket or NULL to use the default
> (which is
> + "wayland-0"). The environment
> variable
> + WAYLAND_DISPLAY replaces the default value.
> If
>WAYLAND_SOCKET is set, this function
> behaves like
>wl_display_connect_to_fd with the
> file-descriptor
>number taken from the environment variable.
> diff --git a/doc/publican/sources/Protocol.xml
> b/doc/publican/sources/Protocol.xml
> index 9464953..477063b 100644
> --- a/doc/publican/sources/Protocol.xml
> +++ b/doc/publican/sources/Protocol.xml
> @@ -60,10 +60,10 @@
> Wire Format
>
>The protocol is sent over a UNIX domain stream socket, where
> the endpoint
> - name is determined by the WAYLAND_DISPLAY
> - environment variable. Its value will usually be
> - wayland-0. The
> protocol is message-based.
> - A message sent by a client to the server is called request. A
> message
> + usually is named wayland
> -0
> + (although it can be changed via
> WAYLAND_DISPLAY
> + in the environment). The protocol is message-based. A
> + message sent by a client to the server is called request. A
> message
>from the server to a client is called event. Every message is
>structured as 32-bit words, values are represented in the
> host's
>byte-order.
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index ffbca4b..09c594a 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -764,11 +764,8 @@ connect_to_socket(const char *name)
>
> if (name == NULL)
> name = getenv("WAYLAND_DISPLAY");
> - if (name == NULL) {
> - wl_log("error: WAYLAND_DISPLAY not set in the
> environment.\n");
> - errno = ENOENT;
> - return -1;
> - }
> + if (name == NULL)
> + name = "wayland-0";
>
> fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
> if (fd < 0)
> @@ -872,7 +869,8 @@ wl_display_connect_to_fd(int fd)
> * \return A \ref wl_display object or \c NULL on failure
> *
> * Connect to the Wayland display named \c name. If \c name is \c
> NULL,
> - * its value will be replaced with the WAYLAND_DISPLAY environment
> variable.
> + * its value will be replaced with the WAYLAND_DISPLAY environment
> + * variable if it is set, otherwise display "wayland-0" will be
> used.
> *
> * \memberof wl_display
> */
--
Sjoerd Simons
Collabora Ltd.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
