(wayland sever/client) multiple binding to a global resources.

2016-02-03 Thread nicesj
Dear all,

I'm still learning of the philosphy of the wayland world.

while seeing and writing some codes, I got a question regarding mutiple binding 
to a global objects.

The client should register a event callback for global registry.

also the server will announce its global resources.

in this corner, when a client gets global registry event callback, it checks 
interface string and then tries to bind it to a specific resource such as 
wl_compositor.

at this time, if our lovely client developer tries to bind one several times, 
how the server handles this request?

just bind and create them? (ex, wl_compositor_interface, wl_shm_interface, and 
so on)

or should it be deat as an error case?

I'm newbie, so my question could be a dummy silly ugly question.
please let me know it to not be a dummer ;)

Best regards,
Sung-jae Park.

Sent from my android device.___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH libinput] touchpad: if we have a serio keyboard, override any previous dwt pairing

2016-02-03 Thread Peter Hutterer
If a USB keyboard like the YubiKey is found before the internal keyboard, it
will be paired with the touchpad when it is seen. The internal keyboard is
seen later but ignored because we already have a keyboard paired with the
touchpad.

This is obviously wrong. For now, give priority to serio keyboards, and
override existing dwt pairings with the new keyboard.

https://bugs.freedesktop.org/show_bug.cgi?id=93983

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c|  48 ---
 test/Makefile.am   |   2 +
 test/litest-device-synaptics-i2c.c | 102 ++
 test/litest-device-yubikey.c   | 169 +
 test/litest.c  |   4 +
 test/litest.h  |   2 +
 test/touchpad.c| 103 ++
 7 files changed, 416 insertions(+), 14 deletions(-)
 create mode 100644 test/litest-device-synaptics-i2c.c
 create mode 100644 test/litest-device-yubikey.c

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index d0d1ddd..2be9d40 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1336,6 +1336,38 @@ tp_want_dwt(struct evdev_device *touchpad,
 }
 
 static void
+tp_dwt_pair_keyboard(struct evdev_device *touchpad,
+struct evdev_device *keyboard)
+{
+   struct tp_dispatch *tp = (struct tp_dispatch*)touchpad->dispatch;
+   unsigned int bus_kbd = libevdev_get_id_bustype(keyboard->evdev);
+
+   if (!tp_want_dwt(touchpad, keyboard))
+   return;
+
+   /* If we already have a keyboard paired, override it if the new one
+* is a serio device. Otherwise keep the current one */
+   if (tp->dwt.keyboard) {
+   if (bus_kbd != BUS_I8042)
+   return;
+
+   memset(tp->dwt.key_mask, 0, sizeof(tp->dwt.key_mask));
+   
libinput_device_remove_event_listener(&tp->dwt.keyboard_listener);
+   }
+
+   libinput_device_add_event_listener(&keyboard->base,
+   &tp->dwt.keyboard_listener,
+   tp_keyboard_event, tp);
+   tp->dwt.keyboard = keyboard;
+   tp->dwt.keyboard_active = false;
+
+   log_debug(touchpad->base.seat->libinput,
+ "palm: dwt activated with %s<->%s\n",
+ touchpad->devname,
+ keyboard->devname);
+}
+
+static void
 tp_interface_device_added(struct evdev_device *device,
  struct evdev_device *added_device)
 {
@@ -1359,20 +1391,8 @@ tp_interface_device_added(struct evdev_device *device,
tp_trackpoint_event, tp);
}
 
-   if (added_device->tags & EVDEV_TAG_KEYBOARD &&
-   tp->dwt.keyboard == NULL &&
-   tp_want_dwt(device, added_device)) {
-   log_debug(tp_libinput_context(tp),
- "palm: dwt activated with %s<->%s\n",
- device->devname,
- added_device->devname);
-
-   libinput_device_add_event_listener(&added_device->base,
-   &tp->dwt.keyboard_listener,
-   tp_keyboard_event, tp);
-   tp->dwt.keyboard = added_device;
-   tp->dwt.keyboard_active = false;
-   }
+   if (added_device->tags & EVDEV_TAG_KEYBOARD)
+   tp_dwt_pair_keyboard(device, added_device);
 
if (tp->sendevents.current_mode !=
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE)
diff --git a/test/Makefile.am b/test/Makefile.am
index 27a2a36..c7e68ef 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -38,6 +38,7 @@ liblitest_la_SOURCES = \
litest-device-qemu-usb-tablet.c \
litest-device-synaptics.c \
litest-device-synaptics-hover.c \
+   litest-device-synaptics-i2c.c \
litest-device-synaptics-st.c \
litest-device-synaptics-t440.c \
litest-device-synaptics-x1-carbon-3rd.c \
@@ -53,6 +54,7 @@ liblitest_la_SOURCES = \
litest-device-wheel-only.c \
litest-device-xen-virtual-pointer.c \
litest-device-vmware-virtual-usb-mouse.c \
+   litest-device-yubikey.c \
litest.c
 liblitest_la_LIBADD = $(top_builddir)/src/libinput-util.la
 liblitest_la_CFLAGS = $(AM_CFLAGS) \
diff --git a/test/litest-device-synaptics-i2c.c 
b/test/litest-device-synaptics-i2c.c
new file mode 100644
index 000..b6b632b
--- /dev/null
+++ b/test/litest-device-synaptics-i2c.c
@@ -0,0 +1,102 @@
+/*
+ * Copyright © 2015 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to 

Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jonas Ådahl
On Wed, Feb 03, 2016 at 07:24:10PM -0800, Bill Spitzak wrote:
> On Wed, Feb 3, 2016 at 1:05 PM, Jasper St. Pierre 
> wrote:
> 
> > On Wed, Feb 3, 2016 at 1:00 PM, Bill Spitzak  wrote:
> > >
> > >
> > > On Wed, Feb 3, 2016 at 11:36 AM, Jasper St. Pierre <
> > jstpie...@mecheye.net>
> > > wrote:
> > >>
> > >> set_parent was moved to xdg_toplevel. Perhaps that's a good idea,
> > >> perhaps it's not. This does mean that a tooltip's parent can never
> > >> change, and a popup's parent can never change. That can help for
> > >> getting grab semantics right, though it might be an idea if we say
> > >> that set_parent on a popup / tooltip simply emit error if a parent is
> > >> already set. I'm -1 to that, though.
> > >
> > >
> > > xdg_toplevel *must* support the ability to change the parent.
> >
> > Somehow I get the feeling you're not even reading the words I'm
> > writing. xdg_toplevel still has a set_parent method, I said it was
> > moved *to* xdg_toplevel.
> >
> 
> Sorry I am confused then as to what the text "This does mean that a
> tooltip's parent can never change, and a popup's parent can never change".
> I think I misread it as "if we use xdg_surface set_parent, you cannot
> change the parent", thus implying that xdg_surface set_parent cannot change
> it. I think what you actually meant was "the current scheme does not allow
> you to change the parent, this proposal *does* allow the parent to change".

Just repeating what Jasper already wrote in this thread, about the
changes in this series.

xdg_surface doesn't have a set_parent any more.

> 
> If there are problems with making the grabs work, I think it is ok if
> attempting to change the parent is either an error or ignored for surfaces
> with the popup role.

Again, xdg_surface doesn't have a set_parent request any more and I
don't think there should be such a request there.

xdg_popup (and xdg_tooltip) doesn't allow changing the parent, nor did
they ever.

Only xdg_toplevel allows changing the parent. xdg_toplevel is what
xdg_surface was before. This is how it worked before.

This series doesn't introduce any changes what so ever regarding
setting/changing parents and stacking order. The only thing changed is
exactly where (what interface) the requests doing what was done before
is.


Jonas
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] shell: Don't send extra configure events on click

2016-02-03 Thread Bryce Harrington
On Thu, Dec 10, 2015 at 01:08:01PM -0600, Derek Foreman wrote:
> The click_to_activate handler fires on every mouse click for a surface
> so let's be a little quicker to early return if you're clicking on the
> surface that already has activation.
> 
> This prevents (among other side effects) the sending of two xdg_configure
> events for every mouse click.
> 
> This should also make having two seats with keyboards behave in the same
> way as a single seat.  Previously the second seat could have a keyboard
> focus on the surface and prevent some of the extra processing (including
> the extra configure events) from taking place.
> 
> Signed-off-by: Derek Foreman 
> ---
> 
> Difference from v1: only shortcut the click handler, don't change
> the activate() function that's called on other binds (like alt-tab)
> 
> 
>  desktop-shell/shell.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 780902d..c8c441f 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -5101,6 +5101,7 @@ activate_binding(struct weston_seat *seat,
>struct desktop_shell *shell,
>struct weston_view *focus_view)
>  {
> + struct focus_state *state;
>   struct weston_surface *focus;
>   struct weston_surface *main_surface;
>  
> @@ -5113,6 +5114,13 @@ activate_binding(struct weston_seat *seat,
>   if (get_shell_surface_type(main_surface) == SHELL_SURFACE_NONE)
>   return;
>  
> + state = ensure_focus_state(shell, seat);
> + if (state == NULL)
> + return;
> +
> + if (state->keyboard_focus == focus)
> + return;
> +
>   activate(shell, focus, seat, true);
>  }

Reviewed-by: Bryce Harrington 

Ok for 1.11.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter

2016-02-03 Thread Jonas Ådahl
On Wed, Feb 03, 2016 at 05:46:35PM -0800, Bryce Harrington wrote:
> On Wed, Feb 03, 2016 at 04:14:09PM +0100, Rui Matos wrote:
> > The last cursor frame we commited before the pointer left one of our
> > surfaces might not have been shown. In that case we'll have a cursor
> > surface frame callback pending which we need to clear so that we can
> > continue submitting new cursor frames.
> > 
> > Signed-off-by: Rui Matos 
> > ---
> > 
> > On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen  wrote:
> > > Xwayland commits a wl_buffer to a cursor wl_surface with a frame
> > > callback, and the frame callback may never be emitted by the
> > > compositor, right?
> > >
> > > Is Xwayland waiting for any previous frame callback to be signalled
> > > before it commits a buffer or re-sets the cursor role on the
> > > wl_surface? Even if the commit and re-set is caused by wl_pointer.enter?
> > >
> > > Would it be a better fix to destroy any pending frame callback and
> > > commit and re-set the role unconditionally on wl_pointer.enter?
> > 
> > Yes, this seems like the proper fix indeed. Thanks,
> > 
> > Rui
> > 
> >  hw/xwayland/xwayland-input.c | 10 ++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> > index 61ca70b..f9e3255 100644
> > --- a/hw/xwayland/xwayland-input.c
> > +++ b/hw/xwayland/xwayland-input.c
> > @@ -267,6 +267,16 @@ pointer_handle_enter(void *data, struct wl_pointer 
> > *pointer,
> >  for (i = 0; i < dev->button->numButtons; i++)
> >  if (BitIsOn(dev->button->down, i))
> >  QueuePointerEvents(dev, ButtonRelease, i, 0, &mask);
> > +
> > +/* The last cursor frame we commited before the pointer left one
> > + * of our surfaces might not have been shown. In that case we'll
> > + * have a cursor surface frame callback pending which we need to
> > + * clear so that we can continue submitting new cursor frames. */
> > +if (xwl_seat->cursor_frame_cb) {
> > +wl_callback_destroy(xwl_seat->cursor_frame_cb);
> > +xwl_seat->cursor_frame_cb = NULL;
> > +xwl_seat_set_cursor(xwl_seat);
> > +}

If think it'd be better to move this to just below
"mipointer->pSpriteCursor = (CursorPtr) 1;" and skip the
xwl_seat_set_cursor(xwl_seat) call. The "pSpriteCursor = 1" thing is
another hack to avoid skipping to set the cursor on enter, and it'd be
good to keep them close. xwl_seat_set_cursor() will be called as well
indirectly by the "CheckMotion()" call.

> >  }
> >  
> >  static void
> 
> Reviewed-by: Bryce Harrington 
> 
> How noticeable/severe is this problem?  Can this be left to 1.11?

This patch is for the xserver, not weston, so no need to worry about
this for the 1.10 release.


Jonas

> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter

2016-02-03 Thread Bryce Harrington
On Wed, Feb 03, 2016 at 04:14:09PM +0100, Rui Matos wrote:
> The last cursor frame we commited before the pointer left one of our
> surfaces might not have been shown. In that case we'll have a cursor
> surface frame callback pending which we need to clear so that we can
> continue submitting new cursor frames.
> 
> Signed-off-by: Rui Matos 
> ---
> 
> On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen  wrote:
> > Xwayland commits a wl_buffer to a cursor wl_surface with a frame
> > callback, and the frame callback may never be emitted by the
> > compositor, right?
> >
> > Is Xwayland waiting for any previous frame callback to be signalled
> > before it commits a buffer or re-sets the cursor role on the
> > wl_surface? Even if the commit and re-set is caused by wl_pointer.enter?
> >
> > Would it be a better fix to destroy any pending frame callback and
> > commit and re-set the role unconditionally on wl_pointer.enter?
> 
> Yes, this seems like the proper fix indeed. Thanks,
> 
> Rui
> 
>  hw/xwayland/xwayland-input.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 61ca70b..f9e3255 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -267,6 +267,16 @@ pointer_handle_enter(void *data, struct wl_pointer 
> *pointer,
>  for (i = 0; i < dev->button->numButtons; i++)
>  if (BitIsOn(dev->button->down, i))
>  QueuePointerEvents(dev, ButtonRelease, i, 0, &mask);
> +
> +/* The last cursor frame we commited before the pointer left one
> + * of our surfaces might not have been shown. In that case we'll
> + * have a cursor surface frame callback pending which we need to
> + * clear so that we can continue submitting new cursor frames. */
> +if (xwl_seat->cursor_frame_cb) {
> +wl_callback_destroy(xwl_seat->cursor_frame_cb);
> +xwl_seat->cursor_frame_cb = NULL;
> +xwl_seat_set_cursor(xwl_seat);
> +}
>  }
>  
>  static void

Reviewed-by: Bryce Harrington 

How noticeable/severe is this problem?  Can this be left to 1.11?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Jonas Ådahl
On Wed, Feb 03, 2016 at 01:44:27PM -0800, Bryce Harrington wrote:
> On Wed, Feb 03, 2016 at 01:37:21PM +0200, Pekka Paalanen wrote:
> > On Wed, 3 Feb 2016 10:52:07 +
> > Auke Booij  wrote:
> > > On 3 February 2016 at 09:34, Jonas Ådahl  wrote:
> > > > On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:  
> > > >> On Wed, 3 Feb 2016 09:56:20 +0900
> > > >> "Jaeyoon Jung"  wrote:
> > > >> > > > If someone starts hardening libwayland against programmer errors,
> > > >> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
> > > >> > > > you tell me why we should support recursive dispatch?  
> > > >> > >
> > > >> > > IMHO we should either do that immediately (before the 1.10 
> > > >> > > release) or
> > > >> > > revert this patch now.  Not that anything is wrong with the patch 
> > > >> > > - it
> > > >> > > doesn't introduce bugs in libwayland...
> > > >> > >
> > > >> > > Previously recursive dispatch was impossible, with this patch it's
> > > >> > > possible.  The patch was landed to allow recursive dispatch to 
> > > >> > > work,
> > > >> > > and it's obvious someone's about to start using it.  
> > > >>
> > > >> You're right, we should make a statement about this. I'd be happy to
> > > >> accept a doc patch to be included in 1.10 as AFAIK we have never
> > > >> intended recursive dispatch to work reliably in either server or client
> > > >> code. I do not consider it as a change in API/ABI or behaviour, just a
> > > >> doc improvement.
> 
> Yes, a doc patch would be fine to land right now.
> 
> > > >> > To be more exact, previously recursive dispatch allowed a message 
> > > >> > copy
> > > >> > from empty buffer and resulted in an irrelevant error post to the 
> > > >> > client.
> > > >> > If we want to disallow recursive dispatch, it would be nice to raise 
> > > >> > an
> > > >> > error like abort() when it is detected. Reverting this patch is just
> > > >> > going back to the past without resolving the issue.  
> > > >>
> > > >> Raising an error like abort() is a more touchy subject, because it
> > > >> indeed has potential to break existing programs that work by luck. We
> > > >> probably want an escape hatch for a while, like an environment variable
> > > >> to not abort() and just complain. Or maybe complaining is enough at
> > > >> first and add the abort() after a couple stable releases.
> > > >>
> > > >> I won't propose anyone to do that work now, because I am hoping someone
> > > >> might do it as a part of GSoC (yes, there are plans for GSoC, just not
> > > >> announced yet I think).
> > > >>  
> > > >> > I agree that recursive dispatch might be harmful and should be 
> > > >> > avoided.
> > > >> > However, I'm not sure if it is something must be done in libwayland.
> > > >> > Isn't it sufficient to be documented somewhere and let the 
> > > >> > implementor
> > > >> > make a decision?  
> > > >>
> > > >> If we document that recursive dispatch is not supported, then we won't
> > > >> bother with patches to fix problems with recursive dispatch, which
> > > >> implies that we could also just revert this patch since we won't care.
> > > >> But the patch is in, and I don't think we win anything by reverting it.
> > > >>
> > > >> If some usage pattern is known broken, I think it would be nice to
> > > >> catch them and at least complain in libwayland. So far we haven't added
> > > >> such code for even every obvious mistake, like destroying objects while
> > > >> other objects are still referencing them.
> > > >>  
> > > >> > > If we later intentionally break that code, we're jerks.
> > > >> > > -- From the jerk that posted a bunch of patches to do more 
> > > >> > > stringent
> > > >> > > checks on resource versions a few days ago...  arguably those have
> > > >> > > always been bugs, though. ;)  
> > > >>
> > > >> Yeah, recursive dispatch is less clear, so we should have a transition
> > > >> period.
> > > >>  
> > > >> > > So, with the release coming so quickly, I think we should either
> > > >> > > decide whether recursive dispatch is legal or illegal before the
> > > >> > > release, or revert the patch (thus re-breaking recursive dispatch) 
> > > >> > > and
> > > >> > > worry about it for 1.11.  
> > > >>
> > > >> I would declare recursive dispatch as illegal. Does anyone object?  
> > > >
> > > > I don't object declaring it illegal in libwayland-server since it was
> > > > previously broken, but doing so in libwayland-client I don't think we
> > > > should (at least yet) since it is used here and there in various clients
> > > > already.  
> > 
> > Ok, let's do it for server since that's what we are looking at right
> > now. (Warnings and aborts would be implemented later.)
> > 
> > Client side is indeed more complicated, since it provably works
> > somewhat, we've had it in some Weston demos even, that were since fixed
> > AFAIK.
> 
> Is there any chance that a client could make use of this to overload the
> server?  If there is, would that imply some securi

[PATCH libinput] touchpad: while a key is held down, don't disable dwt

2016-02-03 Thread Peter Hutterer
If a key enables dwt and is held down when the timeout expires, re-issue the
timeout.

There is a corner case where dwt may not work as expected:
1. key down and held down
2. dwt timer expires, dwt is re-issued
3. touch starts
4. key is released
5. dwt timer expires
6. touch now starts moving the pointer

This is an effect of the smart touch detection. A touch starting after the
last key press is released for pointer motion once dwt turns off again. This
is what happens in the above case, the dwt timer expiring is the last virtual
key press. This is a corner case and likely hard to trigger by a real user.

https://bugs.freedesktop.org/show_bug.cgi?id=93984

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c |  18 +-
 src/evdev-mt-touchpad.h |   1 +
 src/libinput-util.h |  14 +
 test/touchpad.c | 142 
 4 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index f249116..d0d1ddd 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1196,6 +1196,15 @@ tp_keyboard_timeout(uint64_t now, void *data)
 {
struct tp_dispatch *tp = data;
 
+   if (long_any_bit_set(tp->dwt.key_mask,
+ARRAY_LENGTH(tp->dwt.key_mask))) {
+   libinput_timer_set(&tp->dwt.keyboard_timer,
+  now + DEFAULT_KEYBOARD_ACTIVITY_TIMEOUT_2);
+   tp->dwt.keyboard_last_press_time = now;
+   log_debug(tp_libinput_context(tp), "palm: keyboard timeout 
refresh\n");
+   return;
+   }
+
tp_tap_resume(tp, now);
 
tp->dwt.keyboard_active = false;
@@ -1240,6 +1249,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
struct tp_dispatch *tp = data;
struct libinput_event_keyboard *kbdev;
unsigned int timeout;
+   unsigned int key;
 
if (!tp->dwt.dwt_enabled)
return;
@@ -1248,15 +1258,18 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
return;
 
kbdev = libinput_event_get_keyboard_event(event);
+   key = libinput_event_keyboard_get_key(kbdev);
 
/* Only trigger the timer on key down. */
if (libinput_event_keyboard_get_key_state(kbdev) !=
-   LIBINPUT_KEY_STATE_PRESSED)
+   LIBINPUT_KEY_STATE_PRESSED) {
+   long_clear_bit(tp->dwt.key_mask, key);
return;
+   }
 
/* modifier keys don't trigger disable-while-typing so things like
 * ctrl+zoom or ctrl+click are possible */
-   if (tp_key_ignore_for_dwt(libinput_event_keyboard_get_key(kbdev)))
+   if (tp_key_ignore_for_dwt(key))
return;
 
if (!tp->dwt.keyboard_active) {
@@ -1270,6 +1283,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
}
 
tp->dwt.keyboard_last_press_time = time;
+   long_set_bit(tp->dwt.key_mask, key);
libinput_timer_set(&tp->dwt.keyboard_timer,
   time + timeout);
 }
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 0053122..87d34b2 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -343,6 +343,7 @@ struct tp_dispatch {
struct libinput_event_listener keyboard_listener;
struct libinput_timer keyboard_timer;
struct evdev_device *keyboard;
+   unsigned long key_mask[NLONGS(KEY_CNT)];
 
uint64_t keyboard_last_press_time;
} dwt;
diff --git a/src/libinput-util.h b/src/libinput-util.h
index 6adbbc9..522c19c 100644
--- a/src/libinput-util.h
+++ b/src/libinput-util.h
@@ -25,6 +25,7 @@
 #ifndef LIBINPUT_UTIL_H
 #define LIBINPUT_UTIL_H
 
+#include 
 #include 
 #include 
 #include 
@@ -171,6 +172,19 @@ long_set_bit_state(unsigned long *array, int bit, int 
state)
long_clear_bit(array, bit);
 }
 
+static inline int
+long_any_bit_set(unsigned long *array, size_t size)
+{
+   unsigned long i;
+
+   assert(size > 0);
+
+   for (i = 0; i < size; i++)
+   if (array[i] != 0)
+   return 1;
+   return 0;
+}
+
 struct matrix {
float val[3][3]; /* [row][col] */
 };
diff --git a/test/touchpad.c b/test/touchpad.c
index 9376cd5..0d3aa03 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -2438,6 +2438,145 @@ START_TEST(touchpad_dwt_key_hold)
 }
 END_TEST
 
+START_TEST(touchpad_dwt_key_hold_timeout)
+{
+   struct litest_device *touchpad = litest_current_device();
+   struct litest_device *keyboard;
+   struct libinput *li = touchpad->libinput;
+
+   if (!has_disable_while_typing(touchpad))
+   return;
+
+   keyboard = dwt_init_paired_keyboard(li, touchpad);
+   litest_disable_tap(touchpad->libinput_device);
+   litest_drain_events(li);
+
+   litest_keyboard_key(keyboard, KEY

Re: [PATCH libinput] evdev: disable the mode button on the Cyborg RAT 5

2016-02-03 Thread Peter Hutterer
On Wed, Feb 03, 2016 at 10:43:03PM +, Joel Duncan wrote:
> Thanks for clearing that up, Can confirm the patch works.
> 
> udev rules were put in /usr/local/lib/udev by make command, moved to
> /usr/lib/udev then devadm hwdb --update.
> 

for next time:
   ./configure --prefix=/usr --libdir=/usr/lib64
that should put everything in the right folder (assuming you want to
overwrite the system one).

> Looking forward to seeing this in upstream Fedora one day.

I'll get this in over the next day or two

Cheers,
   Peter


> On Wed, Feb 3, 2016 at 9:30 PM, Peter Hutterer 
> wrote:
> >
> > On Wed, Feb 03, 2016 at 06:47:01PM +, Joel Duncan wrote:
> > >
> > > Hi guys,
> > >
> > > Not being a libinput dev I was looking for advice on using this patch.
> > > I noticed that a lot of the RAT config was in /test. After compiling the
> > > driver
> > > and installing it the problem still persists.
> > >
> > > I'm assuming it's due to needing a flag when compiling to include /test
> > > components?
> >
> > anything in test are libinput-internal tests, you don't need them to use
> > libinput.
> >
> > This fix relies on a udev tag applied to the mouse. So my best guess is
> > that you either didn't install the udev rule/hwdb entry correctly, or that
> > you didn't run sudo udevadm hwdb --update afterwards.
> >
> > run udevadm info /sys/class/input/eventX for whatever event node your
> mouse
> > has and check if the LIBINPUT_MODEL_CYBORG_RAT variable is set on the
> > device. I not, then you need to check the udev rule/hwdb install.
> > if it's set and it still doesn't work, the patch is buggy.
> >
> > Cheers,
> >Peter
> >
> > > Any advice would be great!
> > >
> > > Thanks,
> > >
> > > Joel
> > >
> > > On Fri, Jan 29, 2016 at 12:48 AM, Peter Hutterer <
> peter.hutte...@who-t.net>
> > > wrote:
> > >
> > > > This button sends a release N, press N+1 on each press, cycling
> through the
> > > > three event codes supported. This causes a stuck button since the
> current
> > > > mode
> > > > is never released.
> > > >
> > > > Long-term this better served by a set of switches that toggle
> accordingly,
> > > > for
> > > > now disable the button codes.
> > > >
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=92127
> > > >
> > > > Signed-off-by: Peter Hutterer 
> > > > ---
> > > >  src/evdev.c| 36 +
> > > >  src/evdev.h|  1 +
> > > >  test/Makefile.am   |  1 +
> > > >  test/device.c  | 33 
> > > >  test/litest-device-cyborg-rat-5.c  | 71
> > > > ++
> > > >  test/litest.c  |  2 +
> > > >  test/litest.h  |  1 +
> > > >  udev/90-libinput-model-quirks.hwdb |  7 
> > > >  udev/90-libinput-model-quirks.rules.in |  4 ++
> > > >  9 files changed, 156 insertions(+)
> > > >  create mode 100644 test/litest-device-cyborg-rat-5.c
> > > >
> > > > diff --git a/src/evdev.c b/src/evdev.c
> > > > index 8f0a607..66673a8 100644
> > > > --- a/src/evdev.c
> > > > +++ b/src/evdev.c
> > > > @@ -1677,6 +1677,7 @@ evdev_read_model_flags(struct evdev_device
> *device)
> > > > { "LIBINPUT_MODEL_JUMPING_SEMI_MT",
> > > > EVDEV_MODEL_JUMPING_SEMI_MT },
> > > > { "LIBINPUT_MODEL_ELANTECH_TOUCHPAD",
> > > > EVDEV_MODEL_ELANTECH_TOUCHPAD },
> > > > { "LIBINPUT_MODEL_APPLE_INTERNAL_KEYBOARD",
> > > > EVDEV_MODEL_APPLE_INTERNAL_KEYBOARD },
> > > > +   { "LIBINPUT_MODEL_CYBORG_RAT", EVDEV_MODEL_CYBORG_RAT
> },
> > > > { NULL, EVDEV_MODEL_DEFAULT },
> > > > };
> > > > const struct model_map *m = model_map;
> > > > @@ -2249,6 +2250,39 @@ evdev_drain_fd(int fd)
> > > > }
> > > >  }
> > > >
> > > > +static inline void
> > > > +evdev_pre_configure_model_quirks(struct evdev_device *device)
> > > > +{
> > > > +   /* The Cyborg RAT has a mode button that cycles through event
> > > > codes.
> > > > +* On press, we get a release for the current mode and a
> press for
> > > > the
> > > > +* next mode:
> > > > +* E: 0.01 0004 0004 589833 # EV_MSC / MSC_SCAN
> > > >  589833
> > > > +* E: 0.01 0001 0118    # EV_KEY / (null)
>   0
> > > > +* E: 0.01 0004 0004 589834 # EV_MSC / MSC_SCAN
> > > >  589834
> > > > +* E: 0.01 0001 0119 0001   # EV_KEY / (null)
>   1
> > > > +* E: 0.01      #  SYN_REPORT (0)
> > > > -- +0ms
> > > > +* E: 0.705000 0004 0004 589834 # EV_MSC / MSC_SCAN
> > > >  589834
> > > > +* E: 0.705000 0001 0119    # EV_KEY / (null)
>   0
> > > > +* E: 0.705000 0004 0004 589835 # EV_MSC / MSC_SCAN
> > > >  589835
> > > > +* E: 0.705000 0001 011a 0001   # EV_KEY / (null)
>   1
> > > > +* E: 0.705000      #  SYN_REPORT (0)
> > > > -- +705ms
> > > > +  

Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Bryce Harrington
On Wed, Feb 03, 2016 at 01:37:21PM +0200, Pekka Paalanen wrote:
> On Wed, 3 Feb 2016 10:52:07 +
> Auke Booij  wrote:
> > On 3 February 2016 at 09:34, Jonas Ådahl  wrote:
> > > On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:  
> > >> On Wed, 3 Feb 2016 09:56:20 +0900
> > >> "Jaeyoon Jung"  wrote:
> > >> > > > If someone starts hardening libwayland against programmer errors,
> > >> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
> > >> > > > you tell me why we should support recursive dispatch?  
> > >> > >
> > >> > > IMHO we should either do that immediately (before the 1.10 release) 
> > >> > > or
> > >> > > revert this patch now.  Not that anything is wrong with the patch - 
> > >> > > it
> > >> > > doesn't introduce bugs in libwayland...
> > >> > >
> > >> > > Previously recursive dispatch was impossible, with this patch it's
> > >> > > possible.  The patch was landed to allow recursive dispatch to work,
> > >> > > and it's obvious someone's about to start using it.  
> > >>
> > >> You're right, we should make a statement about this. I'd be happy to
> > >> accept a doc patch to be included in 1.10 as AFAIK we have never
> > >> intended recursive dispatch to work reliably in either server or client
> > >> code. I do not consider it as a change in API/ABI or behaviour, just a
> > >> doc improvement.

Yes, a doc patch would be fine to land right now.

> > >> > To be more exact, previously recursive dispatch allowed a message copy
> > >> > from empty buffer and resulted in an irrelevant error post to the 
> > >> > client.
> > >> > If we want to disallow recursive dispatch, it would be nice to raise an
> > >> > error like abort() when it is detected. Reverting this patch is just
> > >> > going back to the past without resolving the issue.  
> > >>
> > >> Raising an error like abort() is a more touchy subject, because it
> > >> indeed has potential to break existing programs that work by luck. We
> > >> probably want an escape hatch for a while, like an environment variable
> > >> to not abort() and just complain. Or maybe complaining is enough at
> > >> first and add the abort() after a couple stable releases.
> > >>
> > >> I won't propose anyone to do that work now, because I am hoping someone
> > >> might do it as a part of GSoC (yes, there are plans for GSoC, just not
> > >> announced yet I think).
> > >>  
> > >> > I agree that recursive dispatch might be harmful and should be avoided.
> > >> > However, I'm not sure if it is something must be done in libwayland.
> > >> > Isn't it sufficient to be documented somewhere and let the implementor
> > >> > make a decision?  
> > >>
> > >> If we document that recursive dispatch is not supported, then we won't
> > >> bother with patches to fix problems with recursive dispatch, which
> > >> implies that we could also just revert this patch since we won't care.
> > >> But the patch is in, and I don't think we win anything by reverting it.
> > >>
> > >> If some usage pattern is known broken, I think it would be nice to
> > >> catch them and at least complain in libwayland. So far we haven't added
> > >> such code for even every obvious mistake, like destroying objects while
> > >> other objects are still referencing them.
> > >>  
> > >> > > If we later intentionally break that code, we're jerks.
> > >> > > -- From the jerk that posted a bunch of patches to do more stringent
> > >> > > checks on resource versions a few days ago...  arguably those have
> > >> > > always been bugs, though. ;)  
> > >>
> > >> Yeah, recursive dispatch is less clear, so we should have a transition
> > >> period.
> > >>  
> > >> > > So, with the release coming so quickly, I think we should either
> > >> > > decide whether recursive dispatch is legal or illegal before the
> > >> > > release, or revert the patch (thus re-breaking recursive dispatch) 
> > >> > > and
> > >> > > worry about it for 1.11.  
> > >>
> > >> I would declare recursive dispatch as illegal. Does anyone object?  
> > >
> > > I don't object declaring it illegal in libwayland-server since it was
> > > previously broken, but doing so in libwayland-client I don't think we
> > > should (at least yet) since it is used here and there in various clients
> > > already.  
> 
> Ok, let's do it for server since that's what we are looking at right
> now. (Warnings and aborts would be implemented later.)
> 
> Client side is indeed more complicated, since it provably works
> somewhat, we've had it in some Weston demos even, that were since fixed
> AFAIK.

Is there any chance that a client could make use of this to overload the
server?  If there is, would that imply some security implications that
we need to account for?  It is probably wiser to close such holes before
releasing, even if it might risk delaying it.

> Maybe we could start client side sanitation by these steps release by
> release:
> 
> 1. Document that recursive dispatch is problematic and recommend
>against it.
> 
> 2. Add a

Re: [PATCH libinput] evdev: disable the mode button on the Cyborg RAT 5

2016-02-03 Thread Peter Hutterer
On Wed, Feb 03, 2016 at 06:47:01PM +, Joel Duncan wrote:
> ​​
> Hi guys,
> 
> Not being a libinput dev I was looking for advice on using this patch.
> I noticed that a lot of the RAT config was in /test. After compiling the
> driver
> and installing it the problem still persists.
> 
> I'm assuming it's due to needing a flag when compiling to include /test
> components?

anything in test are libinput-internal tests, you don't need them to use
libinput.

This fix relies on a udev tag applied to the mouse. So my best guess is 
that you either didn't install the udev rule/hwdb entry correctly, or that
you didn't run sudo udevadm hwdb --update afterwards.

run udevadm info /sys/class/input/eventX for whatever event node your mouse
has and check if the LIBINPUT_MODEL_CYBORG_RAT variable is set on the
device. I not, then you need to check the udev rule/hwdb install.
if it's set and it still doesn't work, the patch is buggy.

Cheers,
   Peter

> Any advice would be great!
> 
> Thanks,
> 
> Joel
> 
> On Fri, Jan 29, 2016 at 12:48 AM, Peter Hutterer 
> wrote:
> 
> > This button sends a release N, press N+1 on each press, cycling through the
> > three event codes supported. This causes a stuck button since the current
> > mode
> > is never released.
> >
> > Long-term this better served by a set of switches that toggle accordingly,
> > for
> > now disable the button codes.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=92127
> >
> > Signed-off-by: Peter Hutterer 
> > ---
> >  src/evdev.c| 36 +
> >  src/evdev.h|  1 +
> >  test/Makefile.am   |  1 +
> >  test/device.c  | 33 
> >  test/litest-device-cyborg-rat-5.c  | 71
> > ++
> >  test/litest.c  |  2 +
> >  test/litest.h  |  1 +
> >  udev/90-libinput-model-quirks.hwdb |  7 
> >  udev/90-libinput-model-quirks.rules.in |  4 ++
> >  9 files changed, 156 insertions(+)
> >  create mode 100644 test/litest-device-cyborg-rat-5.c
> >
> > diff --git a/src/evdev.c b/src/evdev.c
> > index 8f0a607..66673a8 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -1677,6 +1677,7 @@ evdev_read_model_flags(struct evdev_device *device)
> > { "LIBINPUT_MODEL_JUMPING_SEMI_MT",
> > EVDEV_MODEL_JUMPING_SEMI_MT },
> > { "LIBINPUT_MODEL_ELANTECH_TOUCHPAD",
> > EVDEV_MODEL_ELANTECH_TOUCHPAD },
> > { "LIBINPUT_MODEL_APPLE_INTERNAL_KEYBOARD",
> > EVDEV_MODEL_APPLE_INTERNAL_KEYBOARD },
> > +   { "LIBINPUT_MODEL_CYBORG_RAT", EVDEV_MODEL_CYBORG_RAT },
> > { NULL, EVDEV_MODEL_DEFAULT },
> > };
> > const struct model_map *m = model_map;
> > @@ -2249,6 +2250,39 @@ evdev_drain_fd(int fd)
> > }
> >  }
> >
> > +static inline void
> > +evdev_pre_configure_model_quirks(struct evdev_device *device)
> > +{
> > +   /* The Cyborg RAT has a mode button that cycles through event
> > codes.
> > +* On press, we get a release for the current mode and a press for
> > the
> > +* next mode:
> > +* E: 0.01 0004 0004 589833 # EV_MSC / MSC_SCAN
> >  589833
> > +* E: 0.01 0001 0118    # EV_KEY / (null)   0
> > +* E: 0.01 0004 0004 589834 # EV_MSC / MSC_SCAN
> >  589834
> > +* E: 0.01 0001 0119 0001   # EV_KEY / (null)   1
> > +* E: 0.01      #  SYN_REPORT (0)
> > -- +0ms
> > +* E: 0.705000 0004 0004 589834 # EV_MSC / MSC_SCAN
> >  589834
> > +* E: 0.705000 0001 0119    # EV_KEY / (null)   0
> > +* E: 0.705000 0004 0004 589835 # EV_MSC / MSC_SCAN
> >  589835
> > +* E: 0.705000 0001 011a 0001   # EV_KEY / (null)   1
> > +* E: 0.705000      #  SYN_REPORT (0)
> > -- +705ms
> > +* E: 1.496995 0004 0004 589833 # EV_MSC / MSC_SCAN
> >  589833
> > +* E: 1.496995 0001 0118 0001   # EV_KEY / (null)   1
> > +* E: 1.496995 0004 0004 589835 # EV_MSC / MSC_SCAN
> >  589835
> > +* E: 1.496995 0001 011a    # EV_KEY / (null)   0
> > +* E: 1.496995      #  SYN_REPORT (0)
> > -- +791ms
> > +*
> > +* https://bugs.freedesktop.org/show_bug.cgi?id=92127
> > +*
> > +* Disable the event codes to avoid stuck buttons.
> > +*/
> > +   if(device->model_flags & EVDEV_MODEL_CYBORG_RAT) {
> > +   libevdev_disable_event_code(device->evdev, EV_KEY, 0x118);
> > +   libevdev_disable_event_code(device->evdev, EV_KEY, 0x119);
> > +   libevdev_disable_event_code(device->evdev, EV_KEY, 0x11a);
> > +   }
> > +}
> > +
> >  struct evdev_device *
> >  evdev_device_create(struct libinput_seat *seat,
> >

Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jasper St. Pierre
On Wed, Feb 3, 2016 at 1:00 PM, Bill Spitzak  wrote:
>
>
> On Wed, Feb 3, 2016 at 11:36 AM, Jasper St. Pierre 
> wrote:
>>
>> set_parent was moved to xdg_toplevel. Perhaps that's a good idea,
>> perhaps it's not. This does mean that a tooltip's parent can never
>> change, and a popup's parent can never change. That can help for
>> getting grab semantics right, though it might be an idea if we say
>> that set_parent on a popup / tooltip simply emit error if a parent is
>> already set. I'm -1 to that, though.
>
>
> xdg_toplevel *must* support the ability to change the parent.

Somehow I get the feeling you're not even reading the words I'm
writing. xdg_toplevel still has a set_parent method, I said it was
moved *to* xdg_toplevel.

... snip rant-y dialogue ...

> The toolkits will hide this. I would agree that defining the delta to be
> from 0,0 for the initial attach, in cases where the position is usable on
> first attach, is a good idea.

Toolkits now have additional state they have to keep track of -- where
the compositor thinks their surface is, relative to themselves. As
mentioned, I'm +0 on the idea. I'll leave it up to Jonas, Guilio and
Mike -- the people actually writing the toolkits in question -- to
determine whether they want it.

-- 
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Moving bugs to Phabricator

2016-02-03 Thread Bryce Harrington
On Wed, Feb 03, 2016 at 12:25:26PM +, Daniel Stone wrote:
> Hi,
> 
> On 2 February 2016 at 20:28, Bill Spitzak  wrote:
> > Can you make a clone of the current Bugzilla state in Phabricator so that we
> > can see what it looks like?
> 
> Good idea! Here they are:
> https://phabricator.freedesktop.org/project/board/100/
> https://phabricator.freedesktop.org/project/board/101/
> https://phabricator.freedesktop.org/project/board/102/
> https://phabricator.freedesktop.org/project/board/103/
> 
> Please bear in mind these are REAL IMPORTED BUGS linked to REAL ACTUAL
> PEOPLE. Manipulating these tasks will lead to actual people being
> emailed, just as it would on Bugzilla. But please feel free to mess
> around with your own bugs, or create new ones and play with those.
> 
> The workboard columns aren't fixed in the current single-column
> format: depending on what works best, we could use any combination of
> one column per release, status-based columns (WIP / mid-bikeshed /
> etc), or anything really.

Does/can it have a tabulated view with various fields shown (like
whether patches are attached, number of comments, etc.)?

> Also, you can make more general (and more advanced) queries from
> https://phabricator.freedesktop.org/maniphest/.
> 
> Would be interested to hear some more feedback from people!

I'm with zmike that if the phabs have high barriers of access, I'm
uncertain it'll adequately replace bugzilla.

It might also be worthwhile to examine the email interface - in other
projects I've found phab a bit spammy with state change emails.

The command line interface is probably what I would end up using the
most though.  I know phab has one but haven't used it much.  I seem to
recall being frustrated by it but don't remember why; like it was
squashing my patches or something.  Maybe it can be tuned though.

Bryce

> Cheers,
> Daniel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jasper St. Pierre
set_parent was moved to xdg_toplevel. Perhaps that's a good idea,
perhaps it's not. This does mean that a tooltip's parent can never
change, and a popup's parent can never change. That can help for
getting grab semantics right, though it might be an idea if we say
that set_parent on a popup / tooltip simply emit error if a parent is
already set. I'm -1 to that, though.

Some think that wl_surface.attach's coordinates are confusing and
built poorly, and I am inclined to agree, however, when I wrote the
original get_xdg_popup, I simply forgot about them. It might be that
we do positioning on first-attach, and we say that the tooltip is
defined to be 0,0 relative to the surface. That is an idea, though I'm
+0 on it.

On Wed, Feb 3, 2016 at 11:08 AM, Bill Spitzak  wrote:
>
>
> On Wed, Feb 3, 2016 at 1:06 AM, Jasper St. Pierre 
> wrote:
>>
>> The existing surface. Perhaps this "get_" naming is a bit confusing,
>> and should be renamed to "create_tooltip" or "make_tooltip" or
>> similar. The intended client flow is:
>>
>> toplevel_wl_surface = wl_compositor.create_surface()
>> toplevel_xdg_surface = xdg_shell.get_xdg_surface(toplevel_wl_surface)
>> toplevel_xdg_toplevel = toplevel_xdg_surface.get_toplevel()
>>
>> tooltip_wl_surface = wl_compositor.create_surface()
>> tooltip_xdg_surface = xdg_shell.get_xdg_surface(tooltip_wl_surface)
>> tooltip_xdg_tooltip =
>> tooltip_xdg_surface.get_tooltip(toplevel_xdg_toplevel, 150, 150)
>>
>> Attach main buffers to toplevel_wl_surface and tooltip buffers to
>> tooltip_wl_surface. tooltip_xdg_tooltip is a tooltip surface attached
>> to the toplevel.
>
>
> But I thought there already was an api to set the parent of a xdg_surface!
>
> I do not think there should be redundancy in ways to get things done, it
> makes it painful for toolkit implementations. It looks to me that if a
> toolkit makes a tooltip the same class structure as a normal window, and the
> user calls the generic setParent on that class, the tooltip is going to have
> to lookup the setParent result just so it can pass it *again* in the
> get_tooltip request. Or I suppose you could use null to mean "leave the
> parent as-is".
>
> There already is relative positioning for child surfaces, that should be
> reused as well, rather than passing the xy here. In fact there is quite a
> lot of api for figuring out relative positioning and it is silly that this
> request makes all that useless by overwriting it. Again if you insist I
> guess you can use 0,0 as an indication of "leave it as-is".
>
>



-- 
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Moving bugs to Phabricator

2016-02-03 Thread Daniel Stone
Hi,

On 3 February 2016 at 15:56, Mike Blumenkrantz  wrote:
> It seems that opening any of these links requires a login. A valid use case 
> for bug trackers is something like:
>
> * user searches web for issue keywords (eg. "wayland has no ssd!")
> * results include link to bug
> * user views bug report
>
> Requiring a login will create more hassle and reduce the likelihood that 
> tickets will be seen.
>
> For reference, we switched to phabricator a few years ago for EFL and had 
> this exact issue. Some parts of the site can be made public, but the majority 
> of it seems to be locked down and only accessible to logged-in users.

Yeah totally, that is an obvious showstopper. The default policy for
bugs is indeed public, but apparently the three projects weren't.
These are fixed now - thanks for letting me know. So far as I know,
there's nothing other hokey preventing you from keeping things public,
apart from the lack of a default policy for wiki pages, which we're
carrying a local patch to fix whilst we wait for it to be upstreamed.

Cheers,
Daniel

> On Wed, 03 Feb 2016 12:25:26 +
> Daniel Stone  wrote:
>
>> Hi,
>>
>> On 2 February 2016 at 20:28, Bill Spitzak  wrote:
>> > Can you make a clone of the current Bugzilla state in Phabricator so that 
>> > we
>> > can see what it looks like?
>>
>> Good idea! Here they are:
>> https://phabricator.freedesktop.org/project/board/100/
>> https://phabricator.freedesktop.org/project/board/101/
>> https://phabricator.freedesktop.org/project/board/102/
>> https://phabricator.freedesktop.org/project/board/103/
>>
>> Please bear in mind these are REAL IMPORTED BUGS linked to REAL ACTUAL
>> PEOPLE. Manipulating these tasks will lead to actual people being
>> emailed, just as it would on Bugzilla. But please feel free to mess
>> around with your own bugs, or create new ones and play with those.
>>
>> The workboard columns aren't fixed in the current single-column
>> format: depending on what works best, we could use any combination of
>> one column per release, status-based columns (WIP / mid-bikeshed /
>> etc), or anything really.
>>
>> Also, you can make more general (and more advanced) queries from
>> https://phabricator.freedesktop.org/maniphest/.
>>
>> Would be interested to hear some more feedback from people!
>>
>> Cheers,
>> Daniel
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Moving bugs to Phabricator

2016-02-03 Thread Mike Blumenkrantz
Hi,

It seems that opening any of these links requires a login. A valid use case for 
bug trackers is something like:

* user searches web for issue keywords (eg. "wayland has no ssd!")
* results include link to bug
* user views bug report

Requiring a login will create more hassle and reduce the likelihood that 
tickets will be seen.

For reference, we switched to phabricator a few years ago for EFL and had this 
exact issue. Some parts of the site can be made public, but the majority of it 
seems to be locked down and only accessible to logged-in users.

On Wed, 03 Feb 2016 12:25:26 +
Daniel Stone  wrote:

> Hi,
> 
> On 2 February 2016 at 20:28, Bill Spitzak  wrote:
> > Can you make a clone of the current Bugzilla state in Phabricator so that we
> > can see what it looks like?  
> 
> Good idea! Here they are:
> https://phabricator.freedesktop.org/project/board/100/
> https://phabricator.freedesktop.org/project/board/101/
> https://phabricator.freedesktop.org/project/board/102/
> https://phabricator.freedesktop.org/project/board/103/
> 
> Please bear in mind these are REAL IMPORTED BUGS linked to REAL ACTUAL
> PEOPLE. Manipulating these tasks will lead to actual people being
> emailed, just as it would on Bugzilla. But please feel free to mess
> around with your own bugs, or create new ones and play with those.
> 
> The workboard columns aren't fixed in the current single-column
> format: depending on what works best, we could use any combination of
> one column per release, status-based columns (WIP / mid-bikeshed /
> etc), or anything really.
> 
> Also, you can make more general (and more advanced) queries from
> https://phabricator.freedesktop.org/maniphest/.
> 
> Would be interested to hear some more feedback from people!
> 
> Cheers,
> Daniel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter

2016-02-03 Thread Rui Matos
The last cursor frame we commited before the pointer left one of our
surfaces might not have been shown. In that case we'll have a cursor
surface frame callback pending which we need to clear so that we can
continue submitting new cursor frames.

Signed-off-by: Rui Matos 
---

On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen  wrote:
> Xwayland commits a wl_buffer to a cursor wl_surface with a frame
> callback, and the frame callback may never be emitted by the
> compositor, right?
>
> Is Xwayland waiting for any previous frame callback to be signalled
> before it commits a buffer or re-sets the cursor role on the
> wl_surface? Even if the commit and re-set is caused by wl_pointer.enter?
>
> Would it be a better fix to destroy any pending frame callback and
> commit and re-set the role unconditionally on wl_pointer.enter?

Yes, this seems like the proper fix indeed. Thanks,

Rui

 hw/xwayland/xwayland-input.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 61ca70b..f9e3255 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -267,6 +267,16 @@ pointer_handle_enter(void *data, struct wl_pointer 
*pointer,
 for (i = 0; i < dev->button->numButtons; i++)
 if (BitIsOn(dev->button->down, i))
 QueuePointerEvents(dev, ButtonRelease, i, 0, &mask);
+
+/* The last cursor frame we commited before the pointer left one
+ * of our surfaces might not have been shown. In that case we'll
+ * have a cursor surface frame callback pending which we need to
+ * clear so that we can continue submitting new cursor frames. */
+if (xwl_seat->cursor_frame_cb) {
+wl_callback_destroy(xwl_seat->cursor_frame_cb);
+xwl_seat->cursor_frame_cb = NULL;
+xwl_seat_set_cursor(xwl_seat);
+}
 }
 
 static void
-- 
2.5.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland-protocols] Add notification area protocol version 1

2016-02-03 Thread Quentin Glidic
From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---

I hope the descriptions in the protocol are clear enough.

I have working compositor implementations for Weston (as a plugin), Orbment 
(plugin) and Sway,
and a working client implementation in eventd.

I want to make very clear that this protocol is intended for non-DEs 
compositors.
DEs are expected to implement notifications as an internal detail (e.g. 
providing D-Bus APIs).

Another important point: this protocol is not using global coordinates.
The compositor defines an output and an area which notifications are restricted 
to.
The recommended default value is the "primary" output, minus the possible 
panels.

I do not expect much notification daemons to port to Wayland and this protocol, 
but if any does,
I would rather see them using an existing protocol than having to do the 
compositor implementation
over and over.
Of course if this protocol is rejected, I will maintain it along with my 
compositor implementation
with a different namespace.

 Makefile.am|   1 +
 unstable/notification-area/README  |   4 +
 .../notification-area-unstable-v1.xml  | 128 +
 3 files changed, 133 insertions(+)
 create mode 100644 unstable/notification-area/README
 create mode 100644 unstable/notification-area/notification-area-unstable-v1.xml

diff --git a/Makefile.am b/Makefile.am
index 582b3f2..032a422 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5,6 +5,7 @@ unstable_protocols =
\
unstable/text-input/text-input-unstable-v1.xml  
\
unstable/input-method/input-method-unstable-v1.xml  
\
unstable/xdg-shell/xdg-shell-unstable-v5.xml
\
+   unstable/notification-area/notification-area-unstable-v1.xml\
$(NULL)
 
 stable_protocols = 
\
diff --git a/unstable/notification-area/README 
b/unstable/notification-area/README
new file mode 100644
index 000..a77bc43
--- /dev/null
+++ b/unstable/notification-area/README
@@ -0,0 +1,4 @@
+Protocol for DE-agnostic notification daemons
+
+Maintainers:
+Quentin Glidic 
diff --git a/unstable/notification-area/notification-area-unstable-v1.xml 
b/unstable/notification-area/notification-area-unstable-v1.xml
new file mode 100644
index 000..370034b
--- /dev/null
+++ b/unstable/notification-area/notification-area-unstable-v1.xml
@@ -0,0 +1,128 @@
+
+
+
+   Copyright © 2011-2016 Quentin "Sardem FF7" Glidic
+
+   Permission to use, copy, modify, distribute, and sell this
+   software and its documentation for any purpose is hereby granted
+   without fee, provided that the above copyright notice appear in
+   all copies and that both that copyright notice and this permission
+   notice appear in supporting documentation, and that the name of
+   the copyright holders not be used in advertising or publicity
+   pertaining to distribution of the software without specific,
+   written prior permission.  The copyright holders make no
+   representations about the suitability of this software for any
+   purpose.  It is provided "as is" without express or implied
+   warranty.
+
+   THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+   SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+   FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+   SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
+   AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
+   ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
+   THIS SOFTWARE.
+
+
+
+   
+   The object is a singleton global.
+
+   This interface can only be bound once at the same time.
+   Any binding of this interface while already bound results
+   in a protocol error (bound).
+
+   This interface is intended for classic notification daemons which
+   display “bubbles”. These daemons are meant to be used in small
+   desktop environment or with highly-customizable compositors.
+   This interface must not be implemented where an integrated
+   notification system is desirable, like in complete desktop
+   environments as GNOME, KDE or EFL. Such an integrated
+
+   DE-independent daemons have an internal logic to place these
+   notifications on screen, which can be a really complex layout.
+   The compositor is in charge of providing a area for the
+   notification daemon to use. This is usually the screen area, minus
+   panels or other surfaces that should always be visible.
+   The compositor is also selecting t

Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Marek Chalupa



On 02/03/16 09:39, Jonas Ådahl wrote:

An xdg_tooltip is a new window type used to implement tooltip like
surfaces. See the interface documentation for details.

Signed-off-by: Jonas Ådahl 
---

Changes since v1:

Various wording changes as suggested by Mike.

Added missing version attribute and .


  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
  1 file changed, 41 insertions(+)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index f2eba64..a164bd6 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -168,6 +168,20 @@

  

+
+  
+   This creates an xdg_tooltip for the given xdg_surface and gives the
+   associated wl_surface the xdg_tooltip role.
+
+   See the documentation of xdg_tooltip for more details about what an
+   xdg_tooltip is and how it is used.
+  
+  
+  
+  
+  


Hi,

IMHO it'd be good describe the purpose of the coordinates, to avoid 
confusion and ensure that everybody use them in the same way.


Cheers,
Marek


+
+
  

The window geometry of a surface is its "visible bounds" from the
@@ -650,4 +664,31 @@



+  
+
+  This interface defines an xdg_tooltip role that provides functionality
+  related to tooltip like surfaces.
+
+  An xdg_tooltip is a temporary surface which is displayed over its parent
+  xdg_surface. The last-created xdg_tooltip surface will always be the
+  top-most child of the parent xdg_surface.
+
+  The parent surface must either have the surface role xdg_toplevel,
+  xdg_popup or xdg_tooltip.
+
+  An xdg_tooltip does not take an active grab while mapped. xdg_tooltip
+  surfaces are either directly unmapped by clients using the
+  xdg_tooltip.destroy request or indirectly unmapped when their parent
+  surface is unmapped.
+
+  An xdg_tooltip obeys normal input region semantics.
+
+
+
+  
+   Unmap the tooltip surface and destroy the object.
+  
+
+  
+
  


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston] compositor-x11: send pointer frame after axis event

2016-02-03 Thread Marek Chalupa
clients that implement pointer interface of version 5
wait for the frame event, so without it the scrolling
does not work (GTK+ clients do not scroll now for example).
Xcb axis events are discrete, so it's fine to send
frame after every single axis event

Signed-off-by: Marek Chalupa 
---
 src/compositor-x11.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index b70c119..13a5d73 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -1092,6 +1092,7 @@ x11_backend_deliver_button_event(struct x11_backend *b,
notify_axis(&b->core_seat,
weston_compositor_get_time(),
&weston_event);
+   notify_pointer_frame(&b->core_seat);
}
return;
case 5:
@@ -1104,6 +1105,7 @@ x11_backend_deliver_button_event(struct x11_backend *b,
notify_axis(&b->core_seat,
weston_compositor_get_time(),
&weston_event);
+   notify_pointer_frame(&b->core_seat);
}
return;
case 6:
@@ -1116,6 +1118,7 @@ x11_backend_deliver_button_event(struct x11_backend *b,
notify_axis(&b->core_seat,
weston_compositor_get_time(),
&weston_event);
+   notify_pointer_frame(&b->core_seat);
}
return;
case 7:
@@ -1128,6 +1131,7 @@ x11_backend_deliver_button_event(struct x11_backend *b,
notify_axis(&b->core_seat,
weston_compositor_get_time(),
&weston_event);
+   notify_pointer_frame(&b->core_seat);
}
return;
default:
-- 
2.5.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] ivi-layout: apply opacity to weston_view correctly

2016-02-03 Thread Pekka Paalanen
On Sat, 26 Dec 2015 23:52:51 +0900
Nobuhiko Tanibata  wrote:

> From: Nobuhiko Tanibata 
> 
> update_opacity is only called when a ivi-surface is visible. But the
> previous code also checks event masks redundantly. However if the event
> happens when ivi-surface is invisible, opacity is not calculated. This
> patch removes this redundant check to fix potential bug.
> 
> Signed-off-by: Nobuhiko Tanibata 
> 
> ---
>  ivi-shell/ivi-layout.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index b90a437..3de1405 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -325,12 +325,9 @@ update_opacity(struct ivi_layout_layer *ivilayer,
>   double layer_alpha = wl_fixed_to_double(ivilayer->prop.opacity);
>   double surf_alpha  = wl_fixed_to_double(ivisurf->prop.opacity);
>  
> - if ((ivilayer->event_mask & IVI_NOTIFICATION_OPACITY) ||
> - (ivisurf->event_mask  & IVI_NOTIFICATION_OPACITY)) {
> - tmpview = get_weston_view(ivisurf);
> - assert(tmpview != NULL);
> - tmpview->alpha = layer_alpha * surf_alpha;
> - }
> + tmpview = get_weston_view(ivisurf);
> + assert(tmpview != NULL);
> + tmpview->alpha = layer_alpha * surf_alpha;
>  }
>  
>  static void

Hi,

looks fine by me, and is a very low-risk fix, so pushed even when we
are at beta now:
   0cc4e98..90c2789  master -> master


Thanks,
pq


pgpH_l3a0f9xB.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Moving bugs to Phabricator

2016-02-03 Thread Daniel Stone
Hi,

On 2 February 2016 at 20:28, Bill Spitzak  wrote:
> Can you make a clone of the current Bugzilla state in Phabricator so that we
> can see what it looks like?

Good idea! Here they are:
https://phabricator.freedesktop.org/project/board/100/
https://phabricator.freedesktop.org/project/board/101/
https://phabricator.freedesktop.org/project/board/102/
https://phabricator.freedesktop.org/project/board/103/

Please bear in mind these are REAL IMPORTED BUGS linked to REAL ACTUAL
PEOPLE. Manipulating these tasks will lead to actual people being
emailed, just as it would on Bugzilla. But please feel free to mess
around with your own bugs, or create new ones and play with those.

The workboard columns aren't fixed in the current single-column
format: depending on what works best, we could use any combination of
one column per release, status-based columns (WIP / mid-bikeshed /
etc), or anything really.

Also, you can make more general (and more advanced) queries from
https://phabricator.freedesktop.org/maniphest/.

Would be interested to hear some more feedback from people!

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Pekka Paalanen
On Wed, 3 Feb 2016 10:52:07 +
Auke Booij  wrote:

> On 3 February 2016 at 09:34, Jonas Ådahl  wrote:
> > On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:  
> >> On Wed, 3 Feb 2016 09:56:20 +0900
> >> "Jaeyoon Jung"  wrote:
> >>  
> >> > > -Original Message-
> >> > > From: Derek Foreman [mailto:der...@osg.samsung.com]
> >> > > Sent: Wednesday, February 03, 2016 6:56 AM
> >> > > To: Pekka Paalanen; Jaeyoon Jung
> >> > > Cc: 'Jonas Ådahl'; wayland-devel@lists.freedesktop.org
> >> > > Subject: Re: Recursive dispatch (Re: [PATCH v2] server: Calculate 
> >> > > remaining
> >> > > data size after a closure is processed)
> >> > >
> >> > > On 02/02/16 06:57 AM, Pekka Paalanen wrote:  
> >> > > > On Tue, 12 Jan 2016 13:03:19 +0900 "Jaeyoon Jung"
> >> > > >  wrote:
> >> > > >  
> >> > > >>> -Original Message- From: wayland-devel
> >> > > >>> [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf
> >> > > >>> Of Jonas Adahl Sent: Tuesday, January 12, 2016 12:27 PM To:
> >> > > >>> Jaeyoon Jung Cc: wayland-devel@lists.freedesktop.org Subject:
> >> > > >>> Re: [PATCH v2] server: Calculate remaining data size after a
> >> > > >>> closure is processed
> >> > > >>>
> >> > > >>> On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:
> >> > > >>>  
> >> > >  When processing a closure, data in the connection can be
> >> > >  consumed again if the closure itself invokes extra event
> >> > >  dispatch. In that case the remaining data size is also
> >> > >  altered, so the variable len should be updated after the
> >> > >  closure is processed.
> >> > > 
> >> > >  Signed-off-by: Jaeyoon Jung   
> >> > > >>>
> >> > > >>> I don't like the name wl_connection_size
> >> > > >>> (wl_connection_pending_input() or something would be more
> >> > > >>> descriptive).  
> >> > > >>
> >> > > >> I just wanted to have a shorter name.
> >> > > >> wl_connection_pending_input() looks better to me as well, and
> >> > > >> I'll update the patch accordingly. Thanks.
> >> > > >>
> >> > > >>  
> >> > > >>> A good follow up would be a test case where this would
> >> > > >>> otherwise cause issues. I assume it would be triggered by a
> >> > > >>> wl_event_loop_dispatch() in a interface request handler?  
> >> > > >>
> >> > > >> Yes, exactly.  
> >> > > >
> >> > > > Hi,
> >> > > >
> >> > > > could you humor me and explain in what sort of situations
> >> > > > dispatching from within a request handler is useful?  
> >> >
> >> > Suppose a server has a single event queue for all kind of events
> >> > including wayland events. If the server wants to process more events
> >> > from the queue in the middle of the request handler, and there is
> >> > another wayland request arrived in the queue, it induces a recursive
> >> > dispatch. I'm not saying that this is a good practice though.  
> >>
> >> Right, this is a tautology: "the server wants to process more
> >> events in the middle of a request handler" is the very definition of
> >> recursive dispatch, not a justification for it.
> >>
> >>  
> >> > > > I thought recursive dispatch is a programming pattern that can
> >> > > > only lead to painful and erratic problems later. Don't we consider
> >> > > > this to be bad practice also in the client API?  
> >> > >
> >> > > Is that actually documented somewhere?  
> >>
> >> I don't think it is. It would be worth to make an official statement
> >> about it in the docs.
> >>
> >> To me it is common sense (or expert knowledge) about event loops in
> >> general.
> >>  
> >> > > > If you do it in the server, that is like: this message causes more
> >> > > > messages to be dispatched - how does that make sense? A server must
> >> > > > not block waiting for any client message.  
> >> > >
> >> > > I agree that a server blocking on a client message is a great way to
> >> > > create a terrible user experience - but I'm not certain it's
> >> > > libwayland's mandate to prevent it?  
> >>
> >> Libwayland could prevent recursive dispatch because it has not been
> >> intended to work. This patch proves that is actually was broken.
> >>
> >> Preventing recursive dispatch does not prevent writing a server that
> >> blocks waiting on a client message. You can very well do that without
> >> recursive dispatch, but it's just a little harder to write.
> >>  
> >> > > > Also, inspecting future messages before finishing the processing of
> >> > > > the current message is racy: the following messages may or may not
> >> > > > be in the receive buffer and nothing guarantees either way, you
> >> > > > just get lucky a lot of times. Wayland IPC does not guarantee more
> >> > > > than one message to be delivered at a time, this is why we have
> >> > > > things like wl_surface.commit request.  
> >> > >
> >> > > This is a really good point. :)
> >> > >  
> >> > > > If someone starts hardening libwayland against programmer errors,
> >> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
> >> > > 

Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Auke Booij
On 3 February 2016 at 09:34, Jonas Ådahl  wrote:
> On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:
>> On Wed, 3 Feb 2016 09:56:20 +0900
>> "Jaeyoon Jung"  wrote:
>>
>> > > -Original Message-
>> > > From: Derek Foreman [mailto:der...@osg.samsung.com]
>> > > Sent: Wednesday, February 03, 2016 6:56 AM
>> > > To: Pekka Paalanen; Jaeyoon Jung
>> > > Cc: 'Jonas Ådahl'; wayland-devel@lists.freedesktop.org
>> > > Subject: Re: Recursive dispatch (Re: [PATCH v2] server: Calculate 
>> > > remaining
>> > > data size after a closure is processed)
>> > >
>> > > On 02/02/16 06:57 AM, Pekka Paalanen wrote:
>> > > > On Tue, 12 Jan 2016 13:03:19 +0900 "Jaeyoon Jung"
>> > > >  wrote:
>> > > >
>> > > >>> -Original Message- From: wayland-devel
>> > > >>> [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf
>> > > >>> Of Jonas Adahl Sent: Tuesday, January 12, 2016 12:27 PM To:
>> > > >>> Jaeyoon Jung Cc: wayland-devel@lists.freedesktop.org Subject:
>> > > >>> Re: [PATCH v2] server: Calculate remaining data size after a
>> > > >>> closure is processed
>> > > >>>
>> > > >>> On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:
>> > > >>>
>> > >  When processing a closure, data in the connection can be
>> > >  consumed again if the closure itself invokes extra event
>> > >  dispatch. In that case the remaining data size is also
>> > >  altered, so the variable len should be updated after the
>> > >  closure is processed.
>> > > 
>> > >  Signed-off-by: Jaeyoon Jung 
>> > > >>>
>> > > >>> I don't like the name wl_connection_size
>> > > >>> (wl_connection_pending_input() or something would be more
>> > > >>> descriptive).
>> > > >>
>> > > >> I just wanted to have a shorter name.
>> > > >> wl_connection_pending_input() looks better to me as well, and
>> > > >> I'll update the patch accordingly. Thanks.
>> > > >>
>> > > >>
>> > > >>> A good follow up would be a test case where this would
>> > > >>> otherwise cause issues. I assume it would be triggered by a
>> > > >>> wl_event_loop_dispatch() in a interface request handler?
>> > > >>
>> > > >> Yes, exactly.
>> > > >
>> > > > Hi,
>> > > >
>> > > > could you humor me and explain in what sort of situations
>> > > > dispatching from within a request handler is useful?
>> >
>> > Suppose a server has a single event queue for all kind of events
>> > including wayland events. If the server wants to process more events
>> > from the queue in the middle of the request handler, and there is
>> > another wayland request arrived in the queue, it induces a recursive
>> > dispatch. I'm not saying that this is a good practice though.
>>
>> Right, this is a tautology: "the server wants to process more
>> events in the middle of a request handler" is the very definition of
>> recursive dispatch, not a justification for it.
>>
>>
>> > > > I thought recursive dispatch is a programming pattern that can
>> > > > only lead to painful and erratic problems later. Don't we consider
>> > > > this to be bad practice also in the client API?
>> > >
>> > > Is that actually documented somewhere?
>>
>> I don't think it is. It would be worth to make an official statement
>> about it in the docs.
>>
>> To me it is common sense (or expert knowledge) about event loops in
>> general.
>>
>> > > > If you do it in the server, that is like: this message causes more
>> > > > messages to be dispatched - how does that make sense? A server must
>> > > > not block waiting for any client message.
>> > >
>> > > I agree that a server blocking on a client message is a great way to
>> > > create a terrible user experience - but I'm not certain it's
>> > > libwayland's mandate to prevent it?
>>
>> Libwayland could prevent recursive dispatch because it has not been
>> intended to work. This patch proves that is actually was broken.
>>
>> Preventing recursive dispatch does not prevent writing a server that
>> blocks waiting on a client message. You can very well do that without
>> recursive dispatch, but it's just a little harder to write.
>>
>> > > > Also, inspecting future messages before finishing the processing of
>> > > > the current message is racy: the following messages may or may not
>> > > > be in the receive buffer and nothing guarantees either way, you
>> > > > just get lucky a lot of times. Wayland IPC does not guarantee more
>> > > > than one message to be delivered at a time, this is why we have
>> > > > things like wl_surface.commit request.
>> > >
>> > > This is a really good point. :)
>> > >
>> > > > If someone starts hardening libwayland against programmer errors,
>> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
>> > > > you tell me why we should support recursive dispatch?
>> > >
>> > > IMHO we should either do that immediately (before the 1.10 release) or
>> > > revert this patch now.  Not that anything is wrong with the patch - it
>> > > doesn't introduce bugs in libwayland...
>> > >
>> > > Previously recursive dispa

Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Jonas Ådahl
On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:
> On Wed, 3 Feb 2016 09:56:20 +0900
> "Jaeyoon Jung"  wrote:
> 
> > > -Original Message-
> > > From: Derek Foreman [mailto:der...@osg.samsung.com]
> > > Sent: Wednesday, February 03, 2016 6:56 AM
> > > To: Pekka Paalanen; Jaeyoon Jung
> > > Cc: 'Jonas Ådahl'; wayland-devel@lists.freedesktop.org
> > > Subject: Re: Recursive dispatch (Re: [PATCH v2] server: Calculate 
> > > remaining
> > > data size after a closure is processed)
> > > 
> > > On 02/02/16 06:57 AM, Pekka Paalanen wrote:  
> > > > On Tue, 12 Jan 2016 13:03:19 +0900 "Jaeyoon Jung"
> > > >  wrote:
> > > >  
> > > >>> -Original Message- From: wayland-devel
> > > >>> [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf
> > > >>> Of Jonas Adahl Sent: Tuesday, January 12, 2016 12:27 PM To:
> > > >>> Jaeyoon Jung Cc: wayland-devel@lists.freedesktop.org Subject:
> > > >>> Re: [PATCH v2] server: Calculate remaining data size after a
> > > >>> closure is processed
> > > >>>
> > > >>> On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:
> > > >>>  
> > >  When processing a closure, data in the connection can be
> > >  consumed again if the closure itself invokes extra event
> > >  dispatch. In that case the remaining data size is also
> > >  altered, so the variable len should be updated after the
> > >  closure is processed.
> > > 
> > >  Signed-off-by: Jaeyoon Jung   
> > > >>>
> > > >>> I don't like the name wl_connection_size
> > > >>> (wl_connection_pending_input() or something would be more
> > > >>> descriptive).  
> > > >>
> > > >> I just wanted to have a shorter name.
> > > >> wl_connection_pending_input() looks better to me as well, and
> > > >> I'll update the patch accordingly. Thanks.
> > > >>
> > > >>  
> > > >>> A good follow up would be a test case where this would
> > > >>> otherwise cause issues. I assume it would be triggered by a
> > > >>> wl_event_loop_dispatch() in a interface request handler?  
> > > >>
> > > >> Yes, exactly.  
> > > >
> > > > Hi,
> > > >
> > > > could you humor me and explain in what sort of situations
> > > > dispatching from within a request handler is useful?  
> > 
> > Suppose a server has a single event queue for all kind of events
> > including wayland events. If the server wants to process more events
> > from the queue in the middle of the request handler, and there is
> > another wayland request arrived in the queue, it induces a recursive
> > dispatch. I'm not saying that this is a good practice though.
> 
> Right, this is a tautology: "the server wants to process more
> events in the middle of a request handler" is the very definition of
> recursive dispatch, not a justification for it.
> 
> 
> > > > I thought recursive dispatch is a programming pattern that can
> > > > only lead to painful and erratic problems later. Don't we consider
> > > > this to be bad practice also in the client API?  
> > > 
> > > Is that actually documented somewhere?
> 
> I don't think it is. It would be worth to make an official statement
> about it in the docs.
> 
> To me it is common sense (or expert knowledge) about event loops in
> general.
> 
> > > > If you do it in the server, that is like: this message causes more
> > > > messages to be dispatched - how does that make sense? A server must
> > > > not block waiting for any client message.  
> > > 
> > > I agree that a server blocking on a client message is a great way to
> > > create a terrible user experience - but I'm not certain it's
> > > libwayland's mandate to prevent it?
> 
> Libwayland could prevent recursive dispatch because it has not been
> intended to work. This patch proves that is actually was broken.
> 
> Preventing recursive dispatch does not prevent writing a server that
> blocks waiting on a client message. You can very well do that without
> recursive dispatch, but it's just a little harder to write.
> 
> > > > Also, inspecting future messages before finishing the processing of
> > > > the current message is racy: the following messages may or may not
> > > > be in the receive buffer and nothing guarantees either way, you
> > > > just get lucky a lot of times. Wayland IPC does not guarantee more
> > > > than one message to be delivered at a time, this is why we have
> > > > things like wl_surface.commit request.  
> > > 
> > > This is a really good point. :)
> > >   
> > > > If someone starts hardening libwayland against programmer errors,
> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
> > > > you tell me why we should support recursive dispatch?  
> > > 
> > > IMHO we should either do that immediately (before the 1.10 release) or
> > > revert this patch now.  Not that anything is wrong with the patch - it
> > > doesn't introduce bugs in libwayland...
> > > 
> > > Previously recursive dispatch was impossible, with this patch it's
> > > possible.  The patch was landed to allow recursive dispatch to work

Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

2016-02-03 Thread Pekka Paalanen
On Wed, 3 Feb 2016 09:56:20 +0900
"Jaeyoon Jung"  wrote:

> > -Original Message-
> > From: Derek Foreman [mailto:der...@osg.samsung.com]
> > Sent: Wednesday, February 03, 2016 6:56 AM
> > To: Pekka Paalanen; Jaeyoon Jung
> > Cc: 'Jonas Ådahl'; wayland-devel@lists.freedesktop.org
> > Subject: Re: Recursive dispatch (Re: [PATCH v2] server: Calculate remaining
> > data size after a closure is processed)
> > 
> > On 02/02/16 06:57 AM, Pekka Paalanen wrote:  
> > > On Tue, 12 Jan 2016 13:03:19 +0900 "Jaeyoon Jung"
> > >  wrote:
> > >  
> > >>> -Original Message- From: wayland-devel
> > >>> [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf
> > >>> Of Jonas Adahl Sent: Tuesday, January 12, 2016 12:27 PM To:
> > >>> Jaeyoon Jung Cc: wayland-devel@lists.freedesktop.org Subject:
> > >>> Re: [PATCH v2] server: Calculate remaining data size after a
> > >>> closure is processed
> > >>>
> > >>> On Tue, Jan 12, 2016 at 08:22:56AM +0900, Jaeyoon Jung wrote:
> > >>>  
> >  When processing a closure, data in the connection can be
> >  consumed again if the closure itself invokes extra event
> >  dispatch. In that case the remaining data size is also
> >  altered, so the variable len should be updated after the
> >  closure is processed.
> > 
> >  Signed-off-by: Jaeyoon Jung   
> > >>>
> > >>> I don't like the name wl_connection_size
> > >>> (wl_connection_pending_input() or something would be more
> > >>> descriptive).  
> > >>
> > >> I just wanted to have a shorter name.
> > >> wl_connection_pending_input() looks better to me as well, and
> > >> I'll update the patch accordingly. Thanks.
> > >>
> > >>  
> > >>> A good follow up would be a test case where this would
> > >>> otherwise cause issues. I assume it would be triggered by a
> > >>> wl_event_loop_dispatch() in a interface request handler?  
> > >>
> > >> Yes, exactly.  
> > >
> > > Hi,
> > >
> > > could you humor me and explain in what sort of situations
> > > dispatching from within a request handler is useful?  
> 
> Suppose a server has a single event queue for all kind of events
> including wayland events. If the server wants to process more events
> from the queue in the middle of the request handler, and there is
> another wayland request arrived in the queue, it induces a recursive
> dispatch. I'm not saying that this is a good practice though.

Right, this is a tautology: "the server wants to process more
events in the middle of a request handler" is the very definition of
recursive dispatch, not a justification for it.


> > > I thought recursive dispatch is a programming pattern that can
> > > only lead to painful and erratic problems later. Don't we consider
> > > this to be bad practice also in the client API?  
> > 
> > Is that actually documented somewhere?

I don't think it is. It would be worth to make an official statement
about it in the docs.

To me it is common sense (or expert knowledge) about event loops in
general.

> > > If you do it in the server, that is like: this message causes more
> > > messages to be dispatched - how does that make sense? A server must
> > > not block waiting for any client message.  
> > 
> > I agree that a server blocking on a client message is a great way to
> > create a terrible user experience - but I'm not certain it's
> > libwayland's mandate to prevent it?

Libwayland could prevent recursive dispatch because it has not been
intended to work. This patch proves that is actually was broken.

Preventing recursive dispatch does not prevent writing a server that
blocks waiting on a client message. You can very well do that without
recursive dispatch, but it's just a little harder to write.

> > > Also, inspecting future messages before finishing the processing of
> > > the current message is racy: the following messages may or may not
> > > be in the receive buffer and nothing guarantees either way, you
> > > just get lucky a lot of times. Wayland IPC does not guarantee more
> > > than one message to be delivered at a time, this is why we have
> > > things like wl_surface.commit request.  
> > 
> > This is a really good point. :)
> >   
> > > If someone starts hardening libwayland against programmer errors,
> > > I'd expect one thing to be to abort() on recursive dispatch. Can
> > > you tell me why we should support recursive dispatch?  
> > 
> > IMHO we should either do that immediately (before the 1.10 release) or
> > revert this patch now.  Not that anything is wrong with the patch - it
> > doesn't introduce bugs in libwayland...
> > 
> > Previously recursive dispatch was impossible, with this patch it's
> > possible.  The patch was landed to allow recursive dispatch to work,
> > and it's obvious someone's about to start using it.  

You're right, we should make a statement about this. I'd be happy to
accept a doc patch to be included in 1.10 as AFAIK we have never
intended recursive dispatch to work reliably in either server or client
code. I do not conside

Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Giulio Camuffo
2016-02-03 11:07 GMT+02:00 Jonas Ådahl :
> On Wed, Feb 03, 2016 at 11:02:14AM +0200, Giulio Camuffo wrote:
>> 2016-02-03 11:00 GMT+02:00 Jasper St. Pierre :
>> > No? The parent is the surface the tooltip is laid on top of.
>>
>> So where do the contents of the tooltip come from?
>
> The tooltip is created from an xdg_surface. As of patch 1/3 of the
> series this patch replies to, each "role" (xdg_toplevel, xdg_popup and
> xdg_tooltip) is created from a xdg_surface object which contains the
> common functionality needed by all xdg_surface based roles.
>
> So its:
>
> Create a xdg_surface A from a wl_surface 1.
> Create a xdg_toplevel from a xdg_surface A.
>
> Create a xdg_surface B from a wl_surface 2.
> Create a xdg_tooltip from a xdg_surface B passing the xdg_surface A as
> the parent.
>
> The tooltip is backed by wl_surface 2, the toplevel is backed by
> wl_surface 1.

Right, sorry. I've been out of the loop for a bit and i forgot that
the requests have an implicit 'this' argument.

>
> Jonas
>
>>
>> >
>> > On Wed, Feb 3, 2016 at 12:59 AM, Giulio Camuffo  
>> > wrote:
>> >> 2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
>> >>> An xdg_tooltip is a new window type used to implement tooltip like
>> >>> surfaces. See the interface documentation for details.
>> >>>
>> >>> Signed-off-by: Jonas Ådahl 
>> >>> ---
>> >>>
>> >>> Changes since v1:
>> >>>
>> >>> Various wording changes as suggested by Mike.
>> >>>
>> >>> Added missing version attribute and .
>> >>>
>> >>>
>> >>>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
>> >>> 
>> >>>  1 file changed, 41 insertions(+)
>> >>>
>> >>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
>> >>> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >>> index f2eba64..a164bd6 100644
>> >>> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >>> @@ -168,6 +168,20 @@
>> >>>
>> >>>  
>> >>>
>> >>> +
>> >>> +  
>> >>> +   This creates an xdg_tooltip for the given xdg_surface and gives 
>> >>> the
>> >>> +   associated wl_surface the xdg_tooltip role.
>> >>> +
>> >>> +   See the documentation of xdg_tooltip for more details about what 
>> >>> an
>> >>> +   xdg_tooltip is and how it is used.
>> >>> +  
>> >>> +  
>> >>> +  
>> >>> +  
>> >>> +  
>> >>> +
>> >>
>> >> Hi, I have just a comment here:
>> >>
>> >> I understand that 'parent' here is the actual tooltip surface, right?
>> >> The description of the zxdg_tooltip_v6 interface below uses parent to
>> >> refer to another surface, and that's i suppose the surface set with
>> >> xdg_surface.set_parent(), right? I find that confusing, i would rename
>> >> 'parent' in get_tooltip to 'surface', and maybe clarify in the
>> >> description what is the actual parent.
>> >>
>> >>
>> >> Cheers,
>> >> Giulio
>> >>
>> >>
>> >>> +
>> >>>  
>> >>>
>> >>> The window geometry of a surface is its "visible bounds" from the
>> >>> @@ -650,4 +664,31 @@
>> >>>
>> >>>
>> >>>
>> >>> +  
>> >>> +
>> >>> +  This interface defines an xdg_tooltip role that provides 
>> >>> functionality
>> >>> +  related to tooltip like surfaces.
>> >>> +
>> >>> +  An xdg_tooltip is a temporary surface which is displayed over its 
>> >>> parent
>> >>> +  xdg_surface. The last-created xdg_tooltip surface will always be 
>> >>> the
>> >>> +  top-most child of the parent xdg_surface.
>> >>> +
>> >>> +  The parent surface must either have the surface role xdg_toplevel,
>> >>> +  xdg_popup or xdg_tooltip.
>> >>> +
>> >>> +  An xdg_tooltip does not take an active grab while mapped. 
>> >>> xdg_tooltip
>> >>> +  surfaces are either directly unmapped by clients using the
>> >>> +  xdg_tooltip.destroy request or indirectly unmapped when their 
>> >>> parent
>> >>> +  surface is unmapped.
>> >>> +
>> >>> +  An xdg_tooltip obeys normal input region semantics.
>> >>> +
>> >>> +
>> >>> +
>> >>> +  
>> >>> +   Unmap the tooltip surface and destroy the object.
>> >>> +  
>> >>> +
>> >>> +  
>> >>> +
>> >>>  
>> >>> --
>> >>> 2.4.3
>> >>>
>> >>> ___
>> >>> wayland-devel mailing list
>> >>> wayland-devel@lists.freedesktop.org
>> >>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>> >
>> >
>> >
>> > --
>> >   Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jonas Ådahl
On Wed, Feb 03, 2016 at 11:02:14AM +0200, Giulio Camuffo wrote:
> 2016-02-03 11:00 GMT+02:00 Jasper St. Pierre :
> > No? The parent is the surface the tooltip is laid on top of.
> 
> So where do the contents of the tooltip come from?

The tooltip is created from an xdg_surface. As of patch 1/3 of the
series this patch replies to, each "role" (xdg_toplevel, xdg_popup and
xdg_tooltip) is created from a xdg_surface object which contains the
common functionality needed by all xdg_surface based roles.

So its:

Create a xdg_surface A from a wl_surface 1.
Create a xdg_toplevel from a xdg_surface A.

Create a xdg_surface B from a wl_surface 2.
Create a xdg_tooltip from a xdg_surface B passing the xdg_surface A as
the parent.

The tooltip is backed by wl_surface 2, the toplevel is backed by
wl_surface 1.

Jonas

> 
> >
> > On Wed, Feb 3, 2016 at 12:59 AM, Giulio Camuffo  
> > wrote:
> >> 2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
> >>> An xdg_tooltip is a new window type used to implement tooltip like
> >>> surfaces. See the interface documentation for details.
> >>>
> >>> Signed-off-by: Jonas Ådahl 
> >>> ---
> >>>
> >>> Changes since v1:
> >>>
> >>> Various wording changes as suggested by Mike.
> >>>
> >>> Added missing version attribute and .
> >>>
> >>>
> >>>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
> >>> 
> >>>  1 file changed, 41 insertions(+)
> >>>
> >>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> >>> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> >>> index f2eba64..a164bd6 100644
> >>> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> >>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> >>> @@ -168,6 +168,20 @@
> >>>
> >>>  
> >>>
> >>> +
> >>> +  
> >>> +   This creates an xdg_tooltip for the given xdg_surface and gives 
> >>> the
> >>> +   associated wl_surface the xdg_tooltip role.
> >>> +
> >>> +   See the documentation of xdg_tooltip for more details about what 
> >>> an
> >>> +   xdg_tooltip is and how it is used.
> >>> +  
> >>> +  
> >>> +  
> >>> +  
> >>> +  
> >>> +
> >>
> >> Hi, I have just a comment here:
> >>
> >> I understand that 'parent' here is the actual tooltip surface, right?
> >> The description of the zxdg_tooltip_v6 interface below uses parent to
> >> refer to another surface, and that's i suppose the surface set with
> >> xdg_surface.set_parent(), right? I find that confusing, i would rename
> >> 'parent' in get_tooltip to 'surface', and maybe clarify in the
> >> description what is the actual parent.
> >>
> >>
> >> Cheers,
> >> Giulio
> >>
> >>
> >>> +
> >>>  
> >>>
> >>> The window geometry of a surface is its "visible bounds" from the
> >>> @@ -650,4 +664,31 @@
> >>>
> >>>
> >>>
> >>> +  
> >>> +
> >>> +  This interface defines an xdg_tooltip role that provides 
> >>> functionality
> >>> +  related to tooltip like surfaces.
> >>> +
> >>> +  An xdg_tooltip is a temporary surface which is displayed over its 
> >>> parent
> >>> +  xdg_surface. The last-created xdg_tooltip surface will always be 
> >>> the
> >>> +  top-most child of the parent xdg_surface.
> >>> +
> >>> +  The parent surface must either have the surface role xdg_toplevel,
> >>> +  xdg_popup or xdg_tooltip.
> >>> +
> >>> +  An xdg_tooltip does not take an active grab while mapped. 
> >>> xdg_tooltip
> >>> +  surfaces are either directly unmapped by clients using the
> >>> +  xdg_tooltip.destroy request or indirectly unmapped when their 
> >>> parent
> >>> +  surface is unmapped.
> >>> +
> >>> +  An xdg_tooltip obeys normal input region semantics.
> >>> +
> >>> +
> >>> +
> >>> +  
> >>> +   Unmap the tooltip surface and destroy the object.
> >>> +  
> >>> +
> >>> +  
> >>> +
> >>>  
> >>> --
> >>> 2.4.3
> >>>
> >>> ___
> >>> wayland-devel mailing list
> >>> wayland-devel@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> >
> >
> > --
> >   Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jasper St. Pierre
The existing surface. Perhaps this "get_" naming is a bit confusing,
and should be renamed to "create_tooltip" or "make_tooltip" or
similar. The intended client flow is:

toplevel_wl_surface = wl_compositor.create_surface()
toplevel_xdg_surface = xdg_shell.get_xdg_surface(toplevel_wl_surface)
toplevel_xdg_toplevel = toplevel_xdg_surface.get_toplevel()

tooltip_wl_surface = wl_compositor.create_surface()
tooltip_xdg_surface = xdg_shell.get_xdg_surface(tooltip_wl_surface)
tooltip_xdg_tooltip =
tooltip_xdg_surface.get_tooltip(toplevel_xdg_toplevel, 150, 150)

Attach main buffers to toplevel_wl_surface and tooltip buffers to
tooltip_wl_surface. tooltip_xdg_tooltip is a tooltip surface attached
to the toplevel.

On Wed, Feb 3, 2016 at 1:02 AM, Giulio Camuffo  wrote:
> 2016-02-03 11:00 GMT+02:00 Jasper St. Pierre :
>> No? The parent is the surface the tooltip is laid on top of.
>
> So where do the contents of the tooltip come from?
>
>>
>> On Wed, Feb 3, 2016 at 12:59 AM, Giulio Camuffo  
>> wrote:
>>> 2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
 An xdg_tooltip is a new window type used to implement tooltip like
 surfaces. See the interface documentation for details.

 Signed-off-by: Jonas Ådahl 
 ---

 Changes since v1:

 Various wording changes as suggested by Mike.

 Added missing version attribute and .


  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
 
  1 file changed, 41 insertions(+)

 diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
 b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
 index f2eba64..a164bd6 100644
 --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
 +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
 @@ -168,6 +168,20 @@

  

 +
 +  
 +   This creates an xdg_tooltip for the given xdg_surface and gives the
 +   associated wl_surface the xdg_tooltip role.
 +
 +   See the documentation of xdg_tooltip for more details about what an
 +   xdg_tooltip is and how it is used.
 +  
 +  
 +  
 +  
 +  
 +
>>>
>>> Hi, I have just a comment here:
>>>
>>> I understand that 'parent' here is the actual tooltip surface, right?
>>> The description of the zxdg_tooltip_v6 interface below uses parent to
>>> refer to another surface, and that's i suppose the surface set with
>>> xdg_surface.set_parent(), right? I find that confusing, i would rename
>>> 'parent' in get_tooltip to 'surface', and maybe clarify in the
>>> description what is the actual parent.
>>>
>>>
>>> Cheers,
>>> Giulio
>>>
>>>
 +
  

 The window geometry of a surface is its "visible bounds" from the
 @@ -650,4 +664,31 @@



 +  
 +
 +  This interface defines an xdg_tooltip role that provides 
 functionality
 +  related to tooltip like surfaces.
 +
 +  An xdg_tooltip is a temporary surface which is displayed over its 
 parent
 +  xdg_surface. The last-created xdg_tooltip surface will always be the
 +  top-most child of the parent xdg_surface.
 +
 +  The parent surface must either have the surface role xdg_toplevel,
 +  xdg_popup or xdg_tooltip.
 +
 +  An xdg_tooltip does not take an active grab while mapped. 
 xdg_tooltip
 +  surfaces are either directly unmapped by clients using the
 +  xdg_tooltip.destroy request or indirectly unmapped when their parent
 +  surface is unmapped.
 +
 +  An xdg_tooltip obeys normal input region semantics.
 +
 +
 +
 +  
 +   Unmap the tooltip surface and destroy the object.
 +  
 +
 +  
 +
  
 --
 2.4.3

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>>
>>
>> --
>>   Jasper



-- 
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Giulio Camuffo
2016-02-03 11:00 GMT+02:00 Jasper St. Pierre :
> No? The parent is the surface the tooltip is laid on top of.

So where do the contents of the tooltip come from?

>
> On Wed, Feb 3, 2016 at 12:59 AM, Giulio Camuffo  
> wrote:
>> 2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
>>> An xdg_tooltip is a new window type used to implement tooltip like
>>> surfaces. See the interface documentation for details.
>>>
>>> Signed-off-by: Jonas Ådahl 
>>> ---
>>>
>>> Changes since v1:
>>>
>>> Various wording changes as suggested by Mike.
>>>
>>> Added missing version attribute and .
>>>
>>>
>>>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
>>> 
>>>  1 file changed, 41 insertions(+)
>>>
>>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
>>> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>>> index f2eba64..a164bd6 100644
>>> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>>> @@ -168,6 +168,20 @@
>>>
>>>  
>>>
>>> +
>>> +  
>>> +   This creates an xdg_tooltip for the given xdg_surface and gives the
>>> +   associated wl_surface the xdg_tooltip role.
>>> +
>>> +   See the documentation of xdg_tooltip for more details about what an
>>> +   xdg_tooltip is and how it is used.
>>> +  
>>> +  
>>> +  
>>> +  
>>> +  
>>> +
>>
>> Hi, I have just a comment here:
>>
>> I understand that 'parent' here is the actual tooltip surface, right?
>> The description of the zxdg_tooltip_v6 interface below uses parent to
>> refer to another surface, and that's i suppose the surface set with
>> xdg_surface.set_parent(), right? I find that confusing, i would rename
>> 'parent' in get_tooltip to 'surface', and maybe clarify in the
>> description what is the actual parent.
>>
>>
>> Cheers,
>> Giulio
>>
>>
>>> +
>>>  
>>>
>>> The window geometry of a surface is its "visible bounds" from the
>>> @@ -650,4 +664,31 @@
>>>
>>>
>>>
>>> +  
>>> +
>>> +  This interface defines an xdg_tooltip role that provides 
>>> functionality
>>> +  related to tooltip like surfaces.
>>> +
>>> +  An xdg_tooltip is a temporary surface which is displayed over its 
>>> parent
>>> +  xdg_surface. The last-created xdg_tooltip surface will always be the
>>> +  top-most child of the parent xdg_surface.
>>> +
>>> +  The parent surface must either have the surface role xdg_toplevel,
>>> +  xdg_popup or xdg_tooltip.
>>> +
>>> +  An xdg_tooltip does not take an active grab while mapped. xdg_tooltip
>>> +  surfaces are either directly unmapped by clients using the
>>> +  xdg_tooltip.destroy request or indirectly unmapped when their parent
>>> +  surface is unmapped.
>>> +
>>> +  An xdg_tooltip obeys normal input region semantics.
>>> +
>>> +
>>> +
>>> +  
>>> +   Unmap the tooltip surface and destroy the object.
>>> +  
>>> +
>>> +  
>>> +
>>>  
>>> --
>>> 2.4.3
>>>
>>> ___
>>> wayland-devel mailing list
>>> wayland-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
>
> --
>   Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jasper St. Pierre
No? The parent is the surface the tooltip is laid on top of.

On Wed, Feb 3, 2016 at 12:59 AM, Giulio Camuffo  wrote:
> 2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
>> An xdg_tooltip is a new window type used to implement tooltip like
>> surfaces. See the interface documentation for details.
>>
>> Signed-off-by: Jonas Ådahl 
>> ---
>>
>> Changes since v1:
>>
>> Various wording changes as suggested by Mike.
>>
>> Added missing version attribute and .
>>
>>
>>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
>> 
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
>> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> index f2eba64..a164bd6 100644
>> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> @@ -168,6 +168,20 @@
>>
>>  
>>
>> +
>> +  
>> +   This creates an xdg_tooltip for the given xdg_surface and gives the
>> +   associated wl_surface the xdg_tooltip role.
>> +
>> +   See the documentation of xdg_tooltip for more details about what an
>> +   xdg_tooltip is and how it is used.
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>
> Hi, I have just a comment here:
>
> I understand that 'parent' here is the actual tooltip surface, right?
> The description of the zxdg_tooltip_v6 interface below uses parent to
> refer to another surface, and that's i suppose the surface set with
> xdg_surface.set_parent(), right? I find that confusing, i would rename
> 'parent' in get_tooltip to 'surface', and maybe clarify in the
> description what is the actual parent.
>
>
> Cheers,
> Giulio
>
>
>> +
>>  
>>
>> The window geometry of a surface is its "visible bounds" from the
>> @@ -650,4 +664,31 @@
>>
>>
>>
>> +  
>> +
>> +  This interface defines an xdg_tooltip role that provides functionality
>> +  related to tooltip like surfaces.
>> +
>> +  An xdg_tooltip is a temporary surface which is displayed over its 
>> parent
>> +  xdg_surface. The last-created xdg_tooltip surface will always be the
>> +  top-most child of the parent xdg_surface.
>> +
>> +  The parent surface must either have the surface role xdg_toplevel,
>> +  xdg_popup or xdg_tooltip.
>> +
>> +  An xdg_tooltip does not take an active grab while mapped. xdg_tooltip
>> +  surfaces are either directly unmapped by clients using the
>> +  xdg_tooltip.destroy request or indirectly unmapped when their parent
>> +  surface is unmapped.
>> +
>> +  An xdg_tooltip obeys normal input region semantics.
>> +
>> +
>> +
>> +  
>> +   Unmap the tooltip surface and destroy the object.
>> +  
>> +
>> +  
>> +
>>  
>> --
>> 2.4.3
>>
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel



-- 
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Giulio Camuffo
2016-02-03 10:39 GMT+02:00 Jonas Ådahl :
> An xdg_tooltip is a new window type used to implement tooltip like
> surfaces. See the interface documentation for details.
>
> Signed-off-by: Jonas Ådahl 
> ---
>
> Changes since v1:
>
> Various wording changes as suggested by Mike.
>
> Added missing version attribute and .
>
>
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
> 
>  1 file changed, 41 insertions(+)
>
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index f2eba64..a164bd6 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -168,6 +168,20 @@
>
>  
>
> +
> +  
> +   This creates an xdg_tooltip for the given xdg_surface and gives the
> +   associated wl_surface the xdg_tooltip role.
> +
> +   See the documentation of xdg_tooltip for more details about what an
> +   xdg_tooltip is and how it is used.
> +  
> +  
> +  
> +  
> +  
> +

Hi, I have just a comment here:

I understand that 'parent' here is the actual tooltip surface, right?
The description of the zxdg_tooltip_v6 interface below uses parent to
refer to another surface, and that's i suppose the surface set with
xdg_surface.set_parent(), right? I find that confusing, i would rename
'parent' in get_tooltip to 'surface', and maybe clarify in the
description what is the actual parent.


Cheers,
Giulio


> +
>  
>
> The window geometry of a surface is its "visible bounds" from the
> @@ -650,4 +664,31 @@
>
>
>
> +  
> +
> +  This interface defines an xdg_tooltip role that provides functionality
> +  related to tooltip like surfaces.
> +
> +  An xdg_tooltip is a temporary surface which is displayed over its 
> parent
> +  xdg_surface. The last-created xdg_tooltip surface will always be the
> +  top-most child of the parent xdg_surface.
> +
> +  The parent surface must either have the surface role xdg_toplevel,
> +  xdg_popup or xdg_tooltip.
> +
> +  An xdg_tooltip does not take an active grab while mapped. xdg_tooltip
> +  surfaces are either directly unmapped by clients using the
> +  xdg_tooltip.destroy request or indirectly unmapped when their parent
> +  surface is unmapped.
> +
> +  An xdg_tooltip obeys normal input region semantics.
> +
> +
> +
> +  
> +   Unmap the tooltip surface and destroy the object.
> +  
> +
> +  
> +
>  
> --
> 2.4.3
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland-protocols v2] xdg-shell: Introduce xdg_tooltip

2016-02-03 Thread Jonas Ådahl
An xdg_tooltip is a new window type used to implement tooltip like
surfaces. See the interface documentation for details.

Signed-off-by: Jonas Ådahl 
---

Changes since v1:

Various wording changes as suggested by Mike.

Added missing version attribute and .


 unstable/xdg-shell/xdg-shell-unstable-v6.xml | 41 
 1 file changed, 41 insertions(+)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index f2eba64..a164bd6 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -168,6 +168,20 @@
   
 
 
+
+  
+   This creates an xdg_tooltip for the given xdg_surface and gives the
+   associated wl_surface the xdg_tooltip role.
+
+   See the documentation of xdg_tooltip for more details about what an
+   xdg_tooltip is and how it is used.
+  
+  
+  
+  
+  
+
+
 
   
The window geometry of a surface is its "visible bounds" from the
@@ -650,4 +664,31 @@
 
   
 
+  
+
+  This interface defines an xdg_tooltip role that provides functionality
+  related to tooltip like surfaces.
+
+  An xdg_tooltip is a temporary surface which is displayed over its parent
+  xdg_surface. The last-created xdg_tooltip surface will always be the
+  top-most child of the parent xdg_surface.
+
+  The parent surface must either have the surface role xdg_toplevel,
+  xdg_popup or xdg_tooltip.
+
+  An xdg_tooltip does not take an active grab while mapped. xdg_tooltip
+  surfaces are either directly unmapped by clients using the
+  xdg_tooltip.destroy request or indirectly unmapped when their parent
+  surface is unmapped.
+
+  An xdg_tooltip obeys normal input region semantics.
+
+
+
+  
+   Unmap the tooltip surface and destroy the object.
+  
+
+  
+
 
-- 
2.4.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland-protocols v2] xdg-shell: Turn xdg_surface into a generic base interface

2016-02-03 Thread Jonas Ådahl
Split out toplevel window like requests and events into a new interface
called xdg_toplevel, and turn xdg_surface into a generic base interface
which others extends.

xdg_popup is changed to extend the xdg_surface.

The configure event in xdg_surface was split up making
xdg_surface.configure an event only carrying the serial number, while a
new xdg_toplevel.configure event carries the other data previously sent
via xdg_surface.configure. xdg_toplevel.configure is made to extend,
via the latch-state mechanism, xdg_surface.configure and depends on
that event to synchronize state.

Other future xdg_surface based extensions are meant to also extend
xdg_surface.configure for relevant window type dependend state
synchronization.

Signed-off-by: Jonas Ådahl 
Signed-off-by: Mike Blumenkrantz 
---

Changes since v1:

Moved role assignment requirements to the xdg_surface interface.

Added a requirement that a role must always be assigned before any other
requests (such as set_window_geometry) is made.

Changed various paragraphs according to Mike's suggestions. One suggestion
(xdg_toplevel.configure) was tweaked a bit.

Removed left-over get_xdg_popup request and references.

Changed xdg_toplevel.set_parent to take another xdg_toplevel instead of
xdg_surface.



 unstable/xdg-shell/xdg-shell-unstable-v6.xml | 261 ---
 1 file changed, 156 insertions(+), 105 deletions(-)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index 2b028c0..3fc7d42 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -54,11 +54,9 @@
 
 
   
-   This creates an xdg_surface for the given surface and gives it the
-   xdg_surface role. A wl_surface can only be given an xdg_surface role
-   once. If get_xdg_surface is called with a wl_surface that already has
-   an active xdg_surface associated with it, or if it had any other role,
-   an error is raised.
+   This creates an xdg_surface for the given surface. While xdg_surface
+   itself is not a role, the corresponding surface may only be assigned
+   a role extending xdg_surface, such as xdg_toplevel or xdg_popup.
 
See the documentation of xdg_surface for more details about what an
xdg_surface is and how it is used.
@@ -67,29 +65,6 @@
   
 
 
-
-  
-   This creates an xdg_popup for the given surface and gives it the
-   xdg_popup role. A wl_surface can only be given an xdg_popup role
-   once. If get_xdg_popup is called with a wl_surface that already has
-   an active xdg_popup associated with it, or if it had any other role,
-   an error is raised.
-
-   This request must be used in response to some sort of user action
-   like a button press, key press, or touch down event.
-
-   See the documentation of xdg_popup for more details about what an
-   xdg_popup is and how it is used.
-  
-  
-  
-  
-  
-  
-  
-  
-
-
 
   
The ping event asks the client if it's still alive. Pass the
@@ -117,13 +92,27 @@
   
 
   
-
+
   An interface that may be implemented by a wl_surface, for
   implementations that provide a desktop-style user interface.
 
-  It provides requests to treat surfaces like windows, allowing to set
-  properties like maximized, fullscreen, minimized, and to move and resize
-  them, and associate metadata like title and app id.
+  It provides a base set of functionality required to construct user
+  interface elements requiring management by the compositor, such as
+  toplevel windows, menus, etc. The types of functionality is split into 
xdg
+  surface roles.
+
+  Creating an xdg_surface does not set the role for a wl_surface. In order
+  to map an xdg_surface, create a role-specific object using, e.g.,
+  get_toplevel, get_popup. The wl_surface for any given xdg_surface can
+  have at most one role.
+
+  If an xdg_surface is created from a wl_surface which already has a role,
+  an error will be raised. When assigning a role to a xdg_surface, if the
+  wl_surface it was created from previously had a different role from the
+  one being assigned, an error will be raised.
+
+  A role must be assigned before any other requests are made to the
+  xdg_surface object.
 
   The client must call wl_surface.commit on the corresponding wl_surface
   for the xdg_surface state to take effect.
@@ -137,8 +126,134 @@
   committed both an xdg_surface state and a buffer.
 
 
+
+  
+  
+
+
 
   
+   Destroy the xdg_surface object. An xdg_surface can only be destroyed
+   after its role object has been destroyed.
+  
+
+
+
+  
+   This creates an xdg_toplevel object for the given xdg_surface and gives
+   the associated wl_surface the xdg_toplevel

Re: [PATCH] xwayland: Only request cursor frame events if the surface is visible

2016-02-03 Thread Pekka Paalanen
On Tue,  2 Feb 2016 21:06:33 +0100
Rui Matos  wrote:

> If the wayland compositor hides our cursor surface (e.g. because the
> pointer moved over a different wayland client) before our last
> submitted buffer gets a chance to be displayed, no frame event will be
> sent and thus we end up in a state where we'll never do any more
> cursor updates since we never clear cursor_frame_cb.
> 
> Signed-off-by: Rui Matos 
> ---
> 
> If you have seen stuck cursors with xwayland windows lately this is
> probably why.
> 
>  hw/xwayland/xwayland-cursor.c |  6 --
>  hw/xwayland/xwayland-input.c  | 30 ++
>  hw/xwayland/xwayland.h|  1 +
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index 76729db..a6ee78f 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -140,8 +140,10 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
>xwl_seat->x_cursor->bits->width,
>xwl_seat->x_cursor->bits->height);
>  
> -xwl_seat->cursor_frame_cb = wl_surface_frame(xwl_seat->cursor);
> -wl_callback_add_listener(xwl_seat->cursor_frame_cb, &frame_listener, 
> xwl_seat);
> +if (xwl_seat->cursor_visible) {
> +xwl_seat->cursor_frame_cb = wl_surface_frame(xwl_seat->cursor);
> +wl_callback_add_listener(xwl_seat->cursor_frame_cb, &frame_listener, 
> xwl_seat);
> +}
>  
>  wl_surface_commit(xwl_seat->cursor);
>  }
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 61ca70b..bdf1fbc 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -781,6 +781,34 @@ static const struct wl_seat_listener seat_listener = {
>  };
>  
>  static void
> +cursor_surface_handle_enter(void *data, struct wl_surface *surface,
> +struct wl_output *output)
> +{
> +struct xwl_seat *xwl_seat = data;
> +xwl_seat->cursor_visible = TRUE;
> +if (xwl_seat->cursor_needs_update) {
> +xwl_seat->cursor_needs_update = FALSE;
> +xwl_seat_set_cursor(xwl_seat);
> +}
> +}
> +
> +static void
> +cursor_surface_handle_leave(void *data, struct wl_surface *surface,
> +struct wl_output *output)
> +{
> +struct xwl_seat *xwl_seat = data;
> +xwl_seat->cursor_visible = FALSE;
> +if (xwl_seat->cursor_frame_cb)
> +wl_callback_destroy(xwl_seat->cursor_frame_cb);
> +xwl_seat->cursor_frame_cb = NULL;
> +}
> +
> +static const struct wl_surface_listener cursor_surface_listener = {
> +cursor_surface_handle_enter,
> +cursor_surface_handle_leave
> +};
> +
> +static void
>  create_input_device(struct xwl_screen *xwl_screen, uint32_t id, uint32_t 
> version)
>  {
>  struct xwl_seat *xwl_seat;
> @@ -800,6 +828,8 @@ create_input_device(struct xwl_screen *xwl_screen, 
> uint32_t id, uint32_t version
>  xwl_seat->id = id;
>  
>  xwl_seat->cursor = wl_compositor_create_surface(xwl_screen->compositor);
> +wl_surface_add_listener(xwl_seat->cursor, &cursor_surface_listener, 
> xwl_seat);
> +
>  wl_seat_add_listener(xwl_seat->seat, &seat_listener, xwl_seat);
>  wl_array_init(&xwl_seat->keys);
>  
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index a7d7119..a0af07a 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -131,6 +131,7 @@ struct xwl_seat {
>  struct wl_surface *cursor;
>  struct wl_callback *cursor_frame_cb;
>  Bool cursor_needs_update;
> +Bool cursor_visible;
>  
>  struct xorg_list touches;
>  

Hi,

I'm not completely convinced that using the
wl_surface.enter/leave(output) events is totally correct for
determining whether you will get a frame callback or not. I do not
recall anything in the protocol implying or guaranteering this.

Compositors are encouraged to stop issuing frame callbacks or to reduce
their rate if the wl_surface is not visible in a normal way. This can
also happen when the surface is mapped on an output but completely
obscured by other surfaces, though I'm not sure if compositors today
implement that. Stopping frame callbacks does not imply a
wl_surface.leave.

Of course, getting a cursor obscured like that can usually happen only
by another cursor, so it's very hard to trigger, but not impossible.

However, when the cursor surface becomes visible again, I'd expect all
pending frame callbacks to be emitted. So why does the surface not
become visible? I suppose the compositor is expecting the cursor
role to be re-set on the surface, otherwise it won't get mapped again.

How does the original problem of a stuck cursor happen?

Xwayland commits a wl_buffer to a cursor wl_surface with a frame
callback, and the frame callback may never be emitted by the
compositor, right?

Is Xwayland waiting for any previous frame callback to be signalled
before it commits a buffer or re-sets the cursor role on th

Re: [PATCH v2 libinput] touchpad: drop motion hysteresis by default

2016-02-03 Thread Hans de Goede

Hi,

On 02-02-16 01:16, Peter Hutterer wrote:

Some older touchpad devices jitter a fair bit when a finger is resting on the
touchpad. That's why the hysteresis was introduced in the synaptics driver
back in 2011. However, the default value of the hysteresis in the synaptics
driver ended up being 0, even though the code looks like it's using a fraction
of the touchpad diagonal. When the hysteresis code was ported to libinput it
was eventually set to 0.5mm.

Turns out this is still too high and tiny finger motions are either
nonreactive or quite jumpy, making it hard to select small targets. Drop the
default hysteresis by reducing its margin to 0, but leave it in place for
those devices where we need them (e.g. the cyapa touchpads).

https://bugs.freedesktop.org/show_bug.cgi?id=93503

Signed-off-by: Peter Hutterer 
---
Changes to v1:
- add exception for the cyapa touchpads

I'm purposely only including that one model because we know that it needs
the hysteresis. Over time, there will be other devices that need it, but
let's fix those up as we find them rather than erring on the side of
including a bunch of devices that may not need it, the improvements to slow
motion are too good to ignore


Ok, lets say how this one plays out:

Reviewed-by: Hans de Goede 

Regards,

Hans






  src/evdev-mt-touchpad.c| 23 ++-
  src/evdev.c|  1 +
  src/evdev.h|  1 +
  udev/90-libinput-model-quirks.hwdb |  3 +++
  4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index f249116..0f72807 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1951,13 +1951,29 @@ tp_init_default_resolution(struct tp_dispatch *tp,
return 0;
  }

+static inline void
+tp_init_hysteresis(struct tp_dispatch *tp)
+{
+   int res_x, res_y;
+
+   res_x = tp->device->abs.absinfo_x->resolution;
+   res_y = tp->device->abs.absinfo_y->resolution;
+
+   if (tp->device->model_flags & EVDEV_MODEL_CYAPA) {
+   tp->hysteresis_margin.x = res_x/2;
+   tp->hysteresis_margin.y = res_y/2;
+   } else {
+   tp->hysteresis_margin.x = 0;
+   tp->hysteresis_margin.y = 0;
+   }
+}
+
  static int
  tp_init(struct tp_dispatch *tp,
struct evdev_device *device)
  {
int width, height;
double diagonal;
-   int res_x, res_y;

tp->base.interface = &tp_interface;
tp->device = device;
@@ -1971,8 +1987,6 @@ tp_init(struct tp_dispatch *tp,
if (tp_init_slots(tp, device) != 0)
return -1;

-   res_x = tp->device->abs.absinfo_x->resolution;
-   res_y = tp->device->abs.absinfo_y->resolution;
width = device->abs.dimensions.x;
height = device->abs.dimensions.y;
diagonal = sqrt(width*width + height*height);
@@ -1981,8 +1995,7 @@ tp_init(struct tp_dispatch *tp,
   EV_ABS,
   ABS_MT_DISTANCE);

-   tp->hysteresis_margin.x = res_x/2;
-   tp->hysteresis_margin.y = res_y/2;
+   tp_init_hysteresis(tp);

if (tp_init_accel(tp, diagonal) != 0)
return -1;
diff --git a/src/evdev.c b/src/evdev.c
index 66673a8..473ff63 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1678,6 +1678,7 @@ evdev_read_model_flags(struct evdev_device *device)
{ "LIBINPUT_MODEL_ELANTECH_TOUCHPAD", 
EVDEV_MODEL_ELANTECH_TOUCHPAD },
{ "LIBINPUT_MODEL_APPLE_INTERNAL_KEYBOARD", 
EVDEV_MODEL_APPLE_INTERNAL_KEYBOARD },
{ "LIBINPUT_MODEL_CYBORG_RAT", EVDEV_MODEL_CYBORG_RAT },
+   { "LIBINPUT_MODEL_CYAPA", EVDEV_MODEL_CYAPA },
{ NULL, EVDEV_MODEL_DEFAULT },
};
const struct model_map *m = model_map;
diff --git a/src/evdev.h b/src/evdev.h
index 8b567a8..b164af8 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -111,6 +111,7 @@ enum evdev_device_model {
EVDEV_MODEL_LENOVO_X220_TOUCHPAD_FW81 = (1 << 12),
EVDEV_MODEL_APPLE_INTERNAL_KEYBOARD = (1 << 13),
EVDEV_MODEL_CYBORG_RAT = (1 << 14),
+   EVDEV_MODEL_CYAPA = (1 << 15),
  };

  struct mt_slot {
diff --git a/udev/90-libinput-model-quirks.hwdb 
b/udev/90-libinput-model-quirks.hwdb
index fa668d6..f23a7f9 100644
--- a/udev/90-libinput-model-quirks.hwdb
+++ b/udev/90-libinput-model-quirks.hwdb
@@ -73,6 +73,9 @@ libinput:name:Cypress APA Trackpad 
(cyapa):dmi:*svn*SAMSUNG*:pn*Lumpy*
  libinput:name:Atmel maXTouch Touchpad:dmi:*svn*GOOGLE*:pn*Samus*
   LIBINPUT_MODEL_CHROMEBOOK=1

+libinput:name:Cypress APA Trackpad (cyapa):dmi:*
+ LIBINPUT_MODEL_CYAPA=1
+
  ##
  # LENOVO
  ##


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/w