Re: [PATCH 1/8] protocol: add presentation extension v4

2014-09-17 Thread Pekka Paalanen
On Tue, 16 Sep 2014 14:16:37 -0700
Bryce Harrington br...@osg.samsung.com wrote:

 On Tue, Sep 16, 2014 at 10:34:29AM +0300, Pekka Paalanen wrote:
  On Mon, 15 Sep 2014 23:12:24 -0700
  Bryce Harrington br...@osg.samsung.com wrote:
  
   Mostly just some minor grammar improvement suggestions...
  
  Appreciated. :-)
  
   On Mon, Sep 15, 2014 at 04:16:40PM -0400, Louis-Francis Ratté-Boulianne 
   wrote:
   snip
diff --git a/Makefile.am b/Makefile.am
diff --git a/protocol/presentation_timing.xml 
b/protocol/presentation_timing.xml
new file mode 100644
index 000..8aadff0
--- /dev/null
+++ b/protocol/presentation_timing.xml
@@ -0,0 +1,247 @@
+?xml version=1.0 encoding=UTF-8?
+protocol name=presentation_timing
+!-- wrap:70 --
+
+
+request name=feedback
+  description summary=request presentation feedback information
+With this request, presentation feedback will be provided for
+the current content submission of the given surface. A new
+presentation_feedback object is created, and that object will
+deliver the information once. The object is tied to this
+content submission only. Multiple presentation_feedback
+objects may be created for the same submission, and they will
+all return the same information.
   
   The language here is a bit clunky.  Here's my try:
   
   In response to this request, a new presentation_feedback object is
   created for the given surface's current content submission.  The object
   contains a static snapshot of the information.  Each
   request against the same submission will create a new
   presentation_feedback object, but they will all have the same
   information.
   
   Not sure that's any better!  But maybe gives some wording ideas.
  
  Yeah, not sure...
  
  How about:
  
  Request presentation feedback for the current content submission on
  the given surface. This creates a new presentation_feedback object,
  which will deliver the feedback information once. If multiple
  presentation_feedback objects are created for the same submission, they
  will all deliver the same information.
 
 There we go, that sounds better.
  
   Also, it sounds like the client will need to free this object itself?
   Either way, it might be worth stating explicitly here.
  
  Clients will always need to free all objects themselves, that's part of
  the Wayland C bindings. No need to add it here, as it applies to
  everything.
 
 Alright, then in that case I noticed at least one other spot where the
 management responsibility was documented in this patch, and so by this
 design convention could be dropped.

Did you intend to point it out?

We have two different cases here though. One is when the protocol
object gets destroyed; after that it is invalid to try to send events
or requests on/with it. The other is when the C object is freed. These
are distinct cases, and one does not exactly imply the other.

If there is a destroy request marked as destructor in the protocol,
then freeing the C object automatically destroys the protocol object so
that both sides will know about it. This is the normal case and does
not need explicit documentation, IMO.

If there is no destructor protocol for destroy, freeing the C object
does not exactly destroy the protocol object; it will continue to exist
in the server, and the client will just ignore events coming through
it. In this case, the protocol designer must have guaranteed a way for
the protocol object to get destroyed in the server (e.g. the done
event for wl_callback).

If the protocol specifies, that an event destroys the protocol object,
the protocol must also guarantee that the client cannot race and send
requests on the protocol object after the server destroyed it. An
obvious way to do that is to not have any requests in the interface.

  Also the destructor protocol existing is a clue, that getting an event
  won't destroy the protocol object.
  
  That's actually something we could change. Remove the destructor
  protocol, and specify that 'presented' and 'discarded' events destroy
  the protocol object. Presentation_feedback being a pure feedback
  interface, I cannot imagine ever adding any requests to it. The only use
  case for destructor protocol would be to cancel the feedback, which
  probably is not common, and would still be possible anyway.
  


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


Re: Stabilizing wl_scaler protocol extension

2014-09-17 Thread Pekka Paalanen
On Tue, 16 Sep 2014 15:33:19 +0100
Steven Newbury st...@snewbury.org.uk wrote:

 On Tue, 2014-09-16 at 14:51 +0300, Pekka Paalanen wrote:
  On Tue, 16 Sep 2014 13:26:12 +0200
  Alexander Preisinger preisin...@gmail.com wrote:
  
   Hi pq,
   
   I use it in my wayland-next branch (for unstable wayland stuff) of 
   the mpv
   player: http://mpv.io/
   In this commit:
 
   https://github.com/mpv-player/mpv/commit/77cc885b44a9e95e5c3c9ae4961b9958ff5cf643
  
  Good to know, thanks.
  
   I only just now realized that I should just use set_destination 
   for my use
   case.
   So setting the destination separately is definitely a use case and 
   I think
   the set request is redundant.
  
  True, but I'm worried how many upset people there will be if I break
  the protocol during the migration by removing or renaming something. 
  :-)
  There should be no-one as it's all experimental still, but...
  
   So far I really like the scaler extensions. But the scaling 
   quality has
   lots of room for improvement.
   I thought about improving the scaling quality, but didn't had the
   opportunity to look into it.
  
  You mean in the Weston implementation? Yeah, that could very well 
  be. I
  think fixing that would come after
  https://bugs.freedesktop.org/show_bug.cgi?id=83895
  as it should make detecting the overall scaling factor a lot easier.
  
  However, I'm more interested in the protocol aspect right now, and
  there the scaling quality cannot be specified IMHO. We have to leave
  room for hardware overlays doing the scaling in unknown ways.
  
  
 
 Wouldn't it be useful if the protocol could have a method of 
 presenting available scaling methods to the application so that the 
 user could configure the preferred trade-off of performance vs quality?

We cannot enumerate the available scaling methods in the protocol, not
statically in the spec, and not even dynamically (e.g. as arbitrary
strings) at runtime. The compositor might not have any clue what
scaling method a hardware scaler uses.

If we did know, and we exposed that to clients, and clients wanted to
use that, the compositor could not present the client's window at all,
because it is not guaranteed that the hardware scaler is always
available and usable (e.g. a partially occluded window may be
impossible to put on a hardware overlay). Vice versa, if the client
chooses some common scaling method, the compositor just cannot use any
hardware scaler, because the scaling method might not be the one.

We could do it as a hint instead of a requirement, but I'm not sure
there is much benefit in that, considering we do have options for the
user already:

The application has two choices for the user to choose between: use
compositor scaling, or don't use and do scaling itself.

The compositor can also have preferences, like a tick box for use
dedicated hardware video scaling or alike, vs. always use OpenGL for
scaling.


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


[PATCH libinput 2/3] litest: Add litest_assert_scroll() helper function

2014-09-17 Thread Hans de Goede
Make check_2fg_scroll functionality available outside of touchpad.c ,
no functional changes.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 test/litest.c   | 39 +++
 test/litest.h   |  1 +
 test/touchpad.c | 49 -
 3 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index cf7e692..e5092b8 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -1084,3 +1084,42 @@ litest_assert_button_event(struct libinput *li, unsigned 
int button,
 state);
libinput_event_destroy(event);
 }
+
+void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir)
+{
+   struct libinput_event *event, *next_event;
+   struct libinput_event_pointer *ptrev;
+
+   event = libinput_get_event(li);
+   next_event = libinput_get_event(li);
+   ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */
+
+   while (event) {
+   ck_assert_int_eq(libinput_event_get_type(event),
+LIBINPUT_EVENT_POINTER_AXIS);
+   ptrev = libinput_event_get_pointer_event(event);
+   ck_assert(ptrev != NULL);
+   ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis);
+
+   if (next_event) {
+   /* Normal scroll event, check dir */
+   if (dir  0) {
+   ck_assert_int_ge(
+   
libinput_event_pointer_get_axis_value(ptrev),
+   dir);
+   } else {
+   ck_assert_int_le(
+   
libinput_event_pointer_get_axis_value(ptrev),
+   dir);
+   }
+   } else {
+   /* Last scroll event, must be 0 */
+   ck_assert_int_eq(
+   libinput_event_pointer_get_axis_value(ptrev),
+   0);
+   }
+   libinput_event_destroy(event);
+   event = next_event;
+   next_event = libinput_get_event(li);
+   }
+}
diff --git a/test/litest.h b/test/litest.h
index dd1386c..fdf815f 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -147,6 +147,7 @@ void litest_assert_empty_queue(struct libinput *li);
 void litest_assert_button_event(struct libinput *li,
unsigned int button,
enum libinput_button_state state);
+void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir);
 
 struct libevdev_uinput * litest_create_uinput_device(const char *name,
 struct input_id *id,
diff --git a/test/touchpad.c b/test/touchpad.c
index 7ff3d14..522cee6 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -1346,47 +1346,6 @@ test_2fg_scroll(struct litest_device *dev, double dx, 
double dy, int sleep)
libinput_dispatch(li);
 }
 
-static void
-check_2fg_scroll(struct litest_device *dev, unsigned int axis, int dir)
-{
-   struct libinput *li = dev-libinput;
-   struct libinput_event *event, *next_event;
-   struct libinput_event_pointer *ptrev;
-
-   event = libinput_get_event(li);
-   next_event = libinput_get_event(li);
-   ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */
-
-   while (event) {
-   ck_assert_int_eq(libinput_event_get_type(event),
-LIBINPUT_EVENT_POINTER_AXIS);
-   ptrev = libinput_event_get_pointer_event(event);
-   ck_assert(ptrev != NULL);
-   ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis);
-
-   if (next_event) {
-   /* Normal scroll event, check dir */
-   if (dir  0) {
-   ck_assert_int_ge(
-   
libinput_event_pointer_get_axis_value(ptrev),
-   dir);
-   } else {
-   ck_assert_int_le(
-   
libinput_event_pointer_get_axis_value(ptrev),
-   dir);
-   }
-   } else {
-   /* Last scroll event, must be 0 */
-   ck_assert_int_eq(
-   libinput_event_pointer_get_axis_value(ptrev),
-   0);
-   }
-   libinput_event_destroy(event);
-   event = next_event;
-   next_event = libinput_get_event(li);
-   }
-}
-
 START_TEST(touchpad_2fg_scroll)
 {
struct litest_device *dev = litest_current_device();
@@ -1395,13 +1354,13 @@ START_TEST(touchpad_2fg_scroll)

[PATCH libinput 1/3] touchpad: Enlarge topbutton area a bit while the touchpad is disabled

2014-09-17 Thread Hans de Goede
Make it easier to hit the topbutton area when the touchpad is disabled,
normally we don't want to make the topbutton area too big, so as to not
interfere with normal touchpad operation, but when disabled we have no such
worries.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 src/evdev-mt-touchpad-buttons.c | 19 ---
 src/evdev-mt-touchpad.c |  4 
 src/evdev-mt-touchpad.h |  4 +++-
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index 865346b..f0800d7 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -496,7 +496,8 @@ tp_release_all_buttons(struct tp_dispatch *tp,
 
 void
 tp_init_softbuttons(struct tp_dispatch *tp,
-   struct evdev_device *device)
+   struct evdev_device *device,
+   double topbutton_size_mult)
 {
int width, height;
const struct input_absinfo *absinfo_x, *absinfo_y;
@@ -524,14 +525,18 @@ tp_init_softbuttons(struct tp_dispatch *tp,
tp-buttons.bottom_area.rightbutton_left_edge = width/2 + xoffset;
 
if (tp-buttons.has_topbuttons) {
-   /* T440s has the top button line 5mm from the top,
-  event analysis has shown events to start down to ~10mm
-  from the top - which maps to 15% */
+   /* T440s has the top button line 5mm from the top, event
+  analysis has shown events to start down to ~10mm from the
+  top - which maps to 15%.  We allow the caller to enlarge the
+  area using a multiplier for the touchpad disabled case. */
+   double topsize_mm = 10 * topbutton_size_mult;
+   double topsize_pct = .15 * topbutton_size_mult;
+
if (yres  1) {
tp-buttons.top_area.bottom_edge =
-   yoffset + 10 * yres;
+   yoffset + topsize_mm * yres;
} else {
-   tp-buttons.top_area.bottom_edge = height * .15 + 
yoffset;
+   tp-buttons.top_area.bottom_edge = height * topsize_pct 
+ yoffset;
}
tp-buttons.top_area.rightbutton_left_edge = width * .58 + 
xoffset;
tp-buttons.top_area.leftbutton_right_edge = width * .42 + 
xoffset;
@@ -581,7 +586,7 @@ tp_init_buttons(struct tp_dispatch *tp,
tp-buttons.use_clickfinger = true;
 
if (tp-buttons.is_clickpad  !tp-buttons.use_clickfinger) {
-   tp_init_softbuttons(tp, device);
+   tp_init_softbuttons(tp, device, 1.0);
} else {
tp-buttons.bottom_area.top_edge = INT_MAX;
tp-buttons.top_area.bottom_edge = INT_MIN;
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 9e568ad..1c32cc6 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -645,6 +645,8 @@ tp_suspend(struct tp_dispatch *tp, struct evdev_device 
*device)
 */
if (tp-buttons.has_topbuttons) {
evdev_notify_suspended_device(device);
+   /* Enlarge topbutton area while suspended */
+   tp_init_softbuttons(tp, device, 1.5);
} else {
evdev_device_suspend(device);
}
@@ -656,6 +658,8 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device 
*device)
if (tp-buttons.has_topbuttons) {
/* tap state-machine is offline while suspended, reset state */
tp_clear_state(tp, device);
+   /* restore original topbutton area size */
+   tp_init_softbuttons(tp, device, 1.0);
evdev_notify_resumed_device(device);
} else {
evdev_device_resume(device);
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 15cca76..e0c8c47 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -245,7 +245,9 @@ int
 tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device);
 
 void
-tp_init_softbuttons(struct tp_dispatch *tp, struct evdev_device *device);
+tp_init_softbuttons(struct tp_dispatch *tp,
+   struct evdev_device *device,
+   double topbutton_size_mult);
 
 void
 tp_destroy_buttons(struct tp_dispatch *tp);
-- 
2.1.0

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


[PATCH weston 0/3] Pointer locking revisited

2014-09-17 Thread Jonas Ådahl
Hi,

This short series introduces a pointer lock interface based on the
previous proposal[0] by Kristian Høgsberg from last year. The first patch
adds a protocol more or less equal to Kristians proposal, but moved out
of wl_seat into its own global object (currently _wl_pointer_lock), with
some concerns raised in the original thread addressed. The implementation
in weston is new. There was no point trying to rebase the previous
implementation due to the architectural changes done since that patch was
written.

The second patch introduces a client side API in toytoolkit, and the
forth makes use of that API in resizor, in the same way as in the
previous proposal, to resize its window.

Note that the reason for having the underscore prefix is to have the
possibility to push a stable variant to the wayland repo without
breaking any clients implementing the experimental version. The
intention is that _wl_pointer_lock ever would be considered an
official interface, it would be renamed to wl_pointer_lock.

In short, the interface works by having the client create a separate
wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus
until wl_pointer.release is requested and the surface gets deactivated.
If the locked pointer object is still not released when a surface
is activated again, the pointer will be implicitly locked to the special
wl_pointer object again.

Limitations with this approach include that it would, as far as I know,
not be possible to fully implement pointer warping support in Xwayland
only using this protocol. However, not sure if support for legacy
concepts is within the scope of an interface like this. Maybe Xwayland
needs some private way to communicate (privileged wl_xwayland interface?)
for these type of things? Maybe it could be emulated by locking if
warping is done with some heuristic for releasing the lock, but have not
tried to implement that.

We wouldn't be able to implement API of certain toolkits (for example
SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the
first (global positions etc) and the use case is most likely to lock the
pointer and get relative motion events, and there is API available in
both those toolkits for that. Pointer confinement (absolute motion
events, but not letting the pointer leave the surface) is another
functionality that wouldn't be possible. It could be emulated by having
the client lock the pointer and draw the pointer itself given the
relative motion events, or simply added to this protocol in some way
(wl_pointer_lock.confine_pointer?).

In the previous thread, surface transformations came up as something that
needed to be thought through. In the updated protocol of this series, I
clarified that relative motion events are as if the surface has not been,
transformed. The reasoning behind this is more or less how typical use
case looks like: 1:st person 3D games. I would expect that the viewport
moves the same even if the window happen to have been transformed for
example zoomed or rotated.

Following the trend of keeping track of these kind of stuff, I filed[1] a
bug on BZ as well.

Thoughts? Is this the right way to go?


Jonas

[0] 
http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html
[1] https://bugs.freedesktop.org/show_bug.cgi?id=84014

Jonas Ådahl (3):
  Introduce pointer lock interface
  clients: Introduce pointer lock API to toytoolkit
  resizor: Make resizor also use pointer locks for resizing

 Makefile.am   |  11 +-
 clients/resizor.c |  62 +++
 clients/window.c  | 177 +
 clients/window.h  |  42 +++
 desktop-shell/exposay.c   |   2 +-
 desktop-shell/shell.c |  10 +-
 protocol/pointer-lock.xml |  85 ++
 src/compositor.c  |   4 +
 src/compositor.h  |  21 +++-
 src/input.c   | 277 +-
 10 files changed, 678 insertions(+), 13 deletions(-)
 create mode 100644 protocol/pointer-lock.xml

-- 
1.8.5.1

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


[PATCH weston 1/3] Introduce pointer lock interface

2014-09-17 Thread Jonas Ådahl
Introduce a pointer lock interface and implementation. The interface
consists of a global currently called _wl_pointer_lock. It is prefixed
with an underscore (_) in order to not conflict with a potential
official protocol in Wayland, and if moving it there, the prefixed
should be removed.

The protocol works by exposing the global _wl_pointer_lock to the
client, and when the client wants to lock the pointer, it creates a new
wl_pointer object using the 'lock_pointer' request. A more detailed
description of the protocol seen in pointer-lock.xml file.

The interface is based on the W3C pointer lock interface [0].

[0] http://www.w3.org/TR/pointerlock/

Signed-off-by: Jonas Ådahl jad...@gmail.com
---

The part of the implementation I'm not very happy with is the internal
API change: the extra argument in the pointer lock motion callback used
to determine if the motion event was a result of a relative motion or an
absolute motion. In this implementation absolute events are simply
ignored as they don't tend to behave in any sensible way. One could
generate relative motion events from absolute ones as done from a
touchpad, but one would need some kind of threshold as there are no
touch down or touch up events.

With backends where relative and absolute events are indistinguishable
(X11, Wayland) is tricker, as there is no way to know whether the event
is suitable or not. One could also just let the client deal with it
making detached absolute motions just result in very long vectors.


Jonas

 Makefile.am   |   7 +-
 desktop-shell/exposay.c   |   2 +-
 desktop-shell/shell.c |  10 +-
 protocol/pointer-lock.xml |  85 ++
 src/compositor.c  |   4 +
 src/compositor.h  |  21 +++-
 src/input.c   | 277 +-
 7 files changed, 394 insertions(+), 12 deletions(-)
 create mode 100644 protocol/pointer-lock.xml

diff --git a/Makefile.am b/Makefile.am
index b2d6893..ed74983 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -79,7 +79,9 @@ nodist_weston_SOURCES =   
\
protocol/workspaces-protocol.c  \
protocol/workspaces-server-protocol.h   \
protocol/scaler-protocol.c  \
-   protocol/scaler-server-protocol.h
+   protocol/scaler-server-protocol.h   \
+   protocol/pointer-lock-protocol.c\
+   protocol/pointer-lock-server-protocol.h
 
 BUILT_SOURCES += $(nodist_weston_SOURCES)
 
@@ -987,7 +989,8 @@ EXTRA_DIST +=   \
protocol/wayland-test.xml   \
protocol/xdg-shell.xml  \
protocol/fullscreen-shell.xml   \
-   protocol/scaler.xml
+   protocol/scaler.xml \
+   protocol/pointer-lock.xml
 
 man_MANS = weston.1 weston.ini.5
 
diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
index 4b65cbd..d6ec6a1 100644
--- a/desktop-shell/exposay.c
+++ b/desktop-shell/exposay.c
@@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab)
 
 static void
 exposay_motion(struct weston_pointer_grab *grab, uint32_t time,
-  wl_fixed_t x, wl_fixed_t y)
+  wl_fixed_t x, wl_fixed_t y, int rel)
 {
struct desktop_shell *shell =
container_of(grab, struct desktop_shell, exposay.grab_ptr);
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 3a5a702..35b05d0 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int 
*cx, int *cy)
 
 static void
 move_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
-wl_fixed_t x, wl_fixed_t y)
+wl_fixed_t x, wl_fixed_t y, int rel)
 {
struct weston_move_grab *move = (struct weston_move_grab *) grab;
struct weston_pointer *pointer = grab-pointer;
@@ -1772,7 +1772,7 @@ struct weston_resize_grab {
 
 static void
 resize_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
-  wl_fixed_t x, wl_fixed_t y)
+  wl_fixed_t x, wl_fixed_t y, int rel)
 {
struct weston_resize_grab *resize = (struct weston_resize_grab *) grab;
struct weston_pointer *pointer = grab-pointer;
@@ -1981,7 +1981,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base)
 
 static void
 busy_cursor_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
-   wl_fixed_t x, wl_fixed_t y)
+   wl_fixed_t x, wl_fixed_t y, int rel)
 {
weston_pointer_move(grab-pointer, x, y);
 }
@@ -3063,7 +3063,7 @@ popup_grab_focus(struct weston_pointer_grab *grab)
 
 static void
 popup_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
- wl_fixed_t x, wl_fixed_t y)
+ wl_fixed_t x, wl_fixed_t y, int rel)
 {
struct weston_pointer *pointer = grab-pointer;

[PATCH weston 2/3] clients: Introduce pointer lock API to toytoolkit

2014-09-17 Thread Jonas Ådahl
Clients can lock the pointer surface wide, and receive relative pointer
events while the pointer is locked.

Signed-off-by: Jonas Ådahl jad...@gmail.com
---
 Makefile.am  |   4 +-
 clients/window.c | 177 +++
 clients/window.h |  42 +
 3 files changed, 222 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index ed74983..cf4bf60 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -448,7 +448,9 @@ nodist_libtoytoolkit_la_SOURCES =   \
protocol/workspaces-protocol.c  \
protocol/workspaces-client-protocol.h   \
protocol/xdg-shell-protocol.c   \
-   protocol/xdg-shell-client-protocol.h
+   protocol/xdg-shell-client-protocol.h\
+   protocol/pointer-lock-protocol.c\
+   protocol/pointer-lock-client-protocol.h
 
 BUILT_SOURCES += $(nodist_libtoytoolkit_la_SOURCES)
 
diff --git a/clients/window.c b/clients/window.c
index e44d65c..d67039b 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -68,6 +68,7 @@ typedef void *EGLContext;
 #include xdg-shell-client-protocol.h
 #include text-cursor-position-client-protocol.h
 #include workspaces-client-protocol.h
+#include pointer-lock-client-protocol.h
 #include ../shared/os-compatibility.h
 
 #include window.h
@@ -91,6 +92,7 @@ struct display {
struct text_cursor_position *text_cursor_position;
struct workspace_manager *workspace_manager;
struct xdg_shell *xdg_shell;
+   struct _wl_pointer_lock *pointer_lock;
EGLDisplay dpy;
EGLConfig argb_config;
EGLContext argb_ctx;
@@ -242,6 +244,12 @@ struct window {
window_output_handler_t output_handler;
window_state_changed_handler_t state_changed_handler;
 
+   pointer_lock_enter_handler_t pointer_lock_enter_handler;
+   pointer_lock_leave_handler_t pointer_lock_leave_handler;
+   pointer_lock_motion_handler_t pointer_lock_motion_handler;
+   pointer_lock_button_handler_t pointer_lock_button_handler;
+   pointer_lock_axis_handler_t pointer_lock_axis_handler;
+
struct surface *main_surface;
struct xdg_surface *xdg_surface;
struct xdg_popup *xdg_popup;
@@ -254,6 +262,8 @@ struct window {
/* struct surface::link, contains also main_surface */
struct wl_list subsurface_list;
 
+   struct wl_pointer *locked_pointer;
+
void *user_data;
struct wl_list link;
 };
@@ -4384,6 +4394,169 @@ window_damage(struct window *window, int32_t x, int32_t 
y,
wl_surface_damage(window-main_surface-surface, x, y, width, height);
 }
 
+void
+window_set_pointer_lock_enter_handler(struct window *window,
+ pointer_lock_enter_handler_t handler)
+{
+   window-pointer_lock_enter_handler = handler;
+}
+
+void
+window_set_pointer_lock_leave_handler(struct window *window,
+ pointer_lock_leave_handler_t handler)
+{
+   window-pointer_lock_leave_handler = handler;
+}
+
+void
+window_set_pointer_lock_motion_handler(struct window *window,
+  pointer_lock_motion_handler_t handler)
+{
+   window-pointer_lock_motion_handler = handler;
+}
+
+void
+window_set_pointer_lock_button_handler(struct window *window,
+  pointer_lock_button_handler_t handler)
+{
+   window-pointer_lock_button_handler = handler;
+}
+
+void
+window_set_pointer_lock_axis_handler(struct window *window,
+pointer_lock_axis_handler_t handler)
+{
+   window-pointer_lock_axis_handler = handler;
+}
+
+static void
+locked_pointer_handle_enter(void *data, struct wl_pointer *pointer,
+   uint32_t serial, struct wl_surface *surface,
+   wl_fixed_t dx, wl_fixed_t dy)
+{
+   struct input *input = data;
+   struct window *window;
+
+   window = wl_surface_get_user_data(surface);
+   if (window-pointer_lock_enter_handler) {
+   window-pointer_lock_enter_handler(window, input,
+  wl_fixed_to_double(dx),
+  wl_fixed_to_double(dy),
+  window-user_data);
+   }
+
+   input-display-serial = serial;
+   input-pointer_enter_serial = serial;
+   input-pointer_focus = window;
+}
+
+static void
+locked_pointer_handle_leave(void *data, struct wl_pointer *pointer,
+   uint32_t serial, struct wl_surface *surface)
+{
+   struct input *input = data;
+   struct window *window;
+
+   window = wl_surface_get_user_data(surface);
+   if (window-pointer_lock_leave_handler) {
+   window-pointer_lock_leave_handler(window, input,
+  window-user_data);
+   

[PATCH weston 3/3] resizor: Make resizor also use pointer locks for resizing

2014-09-17 Thread Jonas Ådahl
Signed-off-by: Jonas Ådahl jad...@gmail.com
---
 clients/resizor.c | 62 +++
 1 file changed, 62 insertions(+)

diff --git a/clients/resizor.c b/clients/resizor.c
index 19c6eeb..66e1c2f 100644
--- a/clients/resizor.c
+++ b/clients/resizor.c
@@ -219,13 +219,70 @@ show_menu(struct resizor *resizor, struct input *input, 
uint32_t time)
 }
 
 static void
+pointer_lock_handle_motion(struct window *window,
+  struct input *input,
+  uint32_t time,
+  float dx,
+  float dy,
+  void *data)
+{
+   struct resizor *resizor = data;
+
+   resizor-width.current += dx;
+   resizor-width.previous = resizor-width.current;
+   resizor-width.target = resizor-width.current;
+
+   resizor-height.current += dy;
+   resizor-height.previous = resizor-height.current;
+   resizor-height.target = resizor-height.current;
+
+   widget_schedule_resize(resizor-widget,
+  resizor-width.current,
+  resizor-height.current);
+}
+
+static void
+pointer_lock_handle_button(struct window *window,
+  struct input *input,
+  uint32_t time,
+  uint32_t button,
+  uint32_t state,
+  void *data)
+{
+   struct resizor *resizor = data;
+
+   if (state != WL_POINTER_BUTTON_STATE_PRESSED)
+   return;
+
+   window_unlock_pointer(resizor-window);
+}
+
+static void
 button_handler(struct widget *widget,
   struct input *input, uint32_t time,
   uint32_t button, enum wl_pointer_button_state state, void *data)
 {
struct resizor *resizor = data;
+   struct rectangle allocation;
 
switch (button) {
+   case BTN_LEFT:
+   if (state != WL_POINTER_BUTTON_STATE_PRESSED)
+   break;
+
+   window_get_allocation(resizor-window, allocation);
+
+   resizor-width.current = allocation.width;
+   resizor-width.previous = allocation.width;
+   resizor-width.target = allocation.width;
+
+   resizor-height.current = allocation.height;
+   resizor-height.previous = allocation.height;
+   resizor-height.target = allocation.height;
+
+   window_lock_pointer(resizor-window, input);
+   break;
+
case BTN_RIGHT:
if (state == WL_POINTER_BUTTON_STATE_PRESSED)
show_menu(resizor, input, time);
@@ -250,6 +307,11 @@ resizor_create(struct display *display)
window_set_keyboard_focus_handler(resizor-window,
  keyboard_focus_handler);
 
+   window_set_pointer_lock_motion_handler(resizor-window,
+  pointer_lock_handle_motion);
+   window_set_pointer_lock_button_handler(resizor-window,
+  pointer_lock_handle_button);
+
widget_set_button_handler(resizor-widget, button_handler);
 
resizor-height.previous = 400;
-- 
1.8.5.1

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


Re: [PATCH weston 0/3] Pointer locking revisited

2014-09-17 Thread Axel Davy

On 17/09/2014 21:39, Jonas Ådahl wrote :

Hi,

This short series introduces a pointer lock interface based on the
previous proposal[0] by Kristian Høgsberg from last year. The first patch
adds a protocol more or less equal to Kristians proposal, but moved out
of wl_seat into its own global object (currently _wl_pointer_lock), with
some concerns raised in the original thread addressed. The implementation
in weston is new. There was no point trying to rebase the previous
implementation due to the architectural changes done since that patch was
written.

The second patch introduces a client side API in toytoolkit, and the
forth makes use of that API in resizor, in the same way as in the
previous proposal, to resize its window.

Note that the reason for having the underscore prefix is to have the
possibility to push a stable variant to the wayland repo without
breaking any clients implementing the experimental version. The
intention is that _wl_pointer_lock ever would be considered an
official interface, it would be renamed to wl_pointer_lock.

In short, the interface works by having the client create a separate
wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus
until wl_pointer.release is requested and the surface gets deactivated.
If the locked pointer object is still not released when a surface
is activated again, the pointer will be implicitly locked to the special
wl_pointer object again.

Limitations with this approach include that it would, as far as I know,
not be possible to fully implement pointer warping support in Xwayland
only using this protocol. However, not sure if support for legacy
concepts is within the scope of an interface like this. Maybe Xwayland
needs some private way to communicate (privileged wl_xwayland interface?)
for these type of things? Maybe it could be emulated by locking if
warping is done with some heuristic for releasing the lock, but have not
tried to implement that.

We wouldn't be able to implement API of certain toolkits (for example
SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the
first (global positions etc) and the use case is most likely to lock the
pointer and get relative motion events, and there is API available in
both those toolkits for that. Pointer confinement (absolute motion
events, but not letting the pointer leave the surface) is another
functionality that wouldn't be possible. It could be emulated by having
the client lock the pointer and draw the pointer itself given the
relative motion events, or simply added to this protocol in some way
(wl_pointer_lock.confine_pointer?).

In the previous thread, surface transformations came up as something that
needed to be thought through. In the updated protocol of this series, I
clarified that relative motion events are as if the surface has not been,
transformed. The reasoning behind this is more or less how typical use
case looks like: 1:st person 3D games. I would expect that the viewport
moves the same even if the window happen to have been transformed for
example zoomed or rotated.

Following the trend of keeping track of these kind of stuff, I filed[1] a
bug on BZ as well.

Thoughts? Is this the right way to go?


Jonas

[0] 
http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html
[1] https://bugs.freedesktop.org/show_bug.cgi?id=84014

Jonas Ådahl (3):
   Introduce pointer lock interface
   clients: Introduce pointer lock API to toytoolkit
   resizor: Make resizor also use pointer locks for resizing

  Makefile.am   |  11 +-
  clients/resizor.c |  62 +++
  clients/window.c  | 177 +
  clients/window.h  |  42 +++
  desktop-shell/exposay.c   |   2 +-
  desktop-shell/shell.c |  10 +-
  protocol/pointer-lock.xml |  85 ++
  src/compositor.c  |   4 +
  src/compositor.h  |  21 +++-
  src/input.c   | 277 +-
  10 files changed, 678 insertions(+), 13 deletions(-)
  create mode 100644 protocol/pointer-lock.xml


Thanks for getting into this.

Could you clarify where the cursor should be during the lock,
and what should be the cursor position when a lock is ended ?

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


Re: [PATCH weston 1/3] Introduce pointer lock interface

2014-09-17 Thread Giulio Camuffo
I haven't looked at the implementation yet, just at the protocol, but
isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a
problem? Objects should have a unique factory interface, or else the
version of the interface can't be determined.


--
Giulio


2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com:
 Introduce a pointer lock interface and implementation. The interface
 consists of a global currently called _wl_pointer_lock. It is prefixed
 with an underscore (_) in order to not conflict with a potential
 official protocol in Wayland, and if moving it there, the prefixed
 should be removed.

 The protocol works by exposing the global _wl_pointer_lock to the
 client, and when the client wants to lock the pointer, it creates a new
 wl_pointer object using the 'lock_pointer' request. A more detailed
 description of the protocol seen in pointer-lock.xml file.

 The interface is based on the W3C pointer lock interface [0].

 [0] http://www.w3.org/TR/pointerlock/

 Signed-off-by: Jonas Ådahl jad...@gmail.com
 ---

 The part of the implementation I'm not very happy with is the internal
 API change: the extra argument in the pointer lock motion callback used
 to determine if the motion event was a result of a relative motion or an
 absolute motion. In this implementation absolute events are simply
 ignored as they don't tend to behave in any sensible way. One could
 generate relative motion events from absolute ones as done from a
 touchpad, but one would need some kind of threshold as there are no
 touch down or touch up events.

 With backends where relative and absolute events are indistinguishable
 (X11, Wayland) is tricker, as there is no way to know whether the event
 is suitable or not. One could also just let the client deal with it
 making detached absolute motions just result in very long vectors.


 Jonas

  Makefile.am   |   7 +-
  desktop-shell/exposay.c   |   2 +-
  desktop-shell/shell.c |  10 +-
  protocol/pointer-lock.xml |  85 ++
  src/compositor.c  |   4 +
  src/compositor.h  |  21 +++-
  src/input.c   | 277 
 +-
  7 files changed, 394 insertions(+), 12 deletions(-)
  create mode 100644 protocol/pointer-lock.xml

 diff --git a/Makefile.am b/Makefile.am
 index b2d6893..ed74983 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -79,7 +79,9 @@ nodist_weston_SOURCES = 
   \
 protocol/workspaces-protocol.c  \
 protocol/workspaces-server-protocol.h   \
 protocol/scaler-protocol.c  \
 -   protocol/scaler-server-protocol.h
 +   protocol/scaler-server-protocol.h   \
 +   protocol/pointer-lock-protocol.c\
 +   protocol/pointer-lock-server-protocol.h

  BUILT_SOURCES += $(nodist_weston_SOURCES)

 @@ -987,7 +989,8 @@ EXTRA_DIST +=   \
 protocol/wayland-test.xml   \
 protocol/xdg-shell.xml  \
 protocol/fullscreen-shell.xml   \
 -   protocol/scaler.xml
 +   protocol/scaler.xml \
 +   protocol/pointer-lock.xml

  man_MANS = weston.1 weston.ini.5

 diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
 index 4b65cbd..d6ec6a1 100644
 --- a/desktop-shell/exposay.c
 +++ b/desktop-shell/exposay.c
 @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab)

  static void
  exposay_motion(struct weston_pointer_grab *grab, uint32_t time,
 -  wl_fixed_t x, wl_fixed_t y)
 +  wl_fixed_t x, wl_fixed_t y, int rel)
  {
 struct desktop_shell *shell =
 container_of(grab, struct desktop_shell, exposay.grab_ptr);
 diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
 index 3a5a702..35b05d0 100644
 --- a/desktop-shell/shell.c
 +++ b/desktop-shell/shell.c
 @@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int 
 *cx, int *cy)

  static void
  move_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
 -wl_fixed_t x, wl_fixed_t y)
 +wl_fixed_t x, wl_fixed_t y, int rel)
  {
 struct weston_move_grab *move = (struct weston_move_grab *) grab;
 struct weston_pointer *pointer = grab-pointer;
 @@ -1772,7 +1772,7 @@ struct weston_resize_grab {

  static void
  resize_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
 -  wl_fixed_t x, wl_fixed_t y)
 +  wl_fixed_t x, wl_fixed_t y, int rel)
  {
 struct weston_resize_grab *resize = (struct weston_resize_grab *) 
 grab;
 struct weston_pointer *pointer = grab-pointer;
 @@ -1981,7 +1981,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base)

  static void
  busy_cursor_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
 -   wl_fixed_t x, wl_fixed_t y)
 +

Re: [PATCH weston 1/3] Introduce pointer lock interface

2014-09-17 Thread Jonas Ådahl
On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo wrote:
 I haven't looked at the implementation yet, just at the protocol, but
 isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a
 problem? Objects should have a unique factory interface, or else the
 version of the interface can't be determined.

Is it really? In the implementation below, the wl_pointer object gets
the same version as the wl_pointer object that is locked. It is also
specified in the last paragraph of _wl_pointer_lock.lock_pointer.

Also, this would not be the first interface that creates objects that
other interface do as well. For example wl_shm_pool.create_buffer and
wl_drm.create_buffer both create wl_buffer's.


Jonas

 
 
 --
 Giulio
 
 
 2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com:
  Introduce a pointer lock interface and implementation. The interface
  consists of a global currently called _wl_pointer_lock. It is prefixed
  with an underscore (_) in order to not conflict with a potential
  official protocol in Wayland, and if moving it there, the prefixed
  should be removed.
 
  The protocol works by exposing the global _wl_pointer_lock to the
  client, and when the client wants to lock the pointer, it creates a new
  wl_pointer object using the 'lock_pointer' request. A more detailed
  description of the protocol seen in pointer-lock.xml file.
 
  The interface is based on the W3C pointer lock interface [0].
 
  [0] http://www.w3.org/TR/pointerlock/
 
  Signed-off-by: Jonas Ådahl jad...@gmail.com
  ---
 
  The part of the implementation I'm not very happy with is the internal
  API change: the extra argument in the pointer lock motion callback used
  to determine if the motion event was a result of a relative motion or an
  absolute motion. In this implementation absolute events are simply
  ignored as they don't tend to behave in any sensible way. One could
  generate relative motion events from absolute ones as done from a
  touchpad, but one would need some kind of threshold as there are no
  touch down or touch up events.
 
  With backends where relative and absolute events are indistinguishable
  (X11, Wayland) is tricker, as there is no way to know whether the event
  is suitable or not. One could also just let the client deal with it
  making detached absolute motions just result in very long vectors.
 
 
  Jonas
 
   Makefile.am   |   7 +-
   desktop-shell/exposay.c   |   2 +-
   desktop-shell/shell.c |  10 +-
   protocol/pointer-lock.xml |  85 ++
   src/compositor.c  |   4 +
   src/compositor.h  |  21 +++-
   src/input.c   | 277 
  +-
   7 files changed, 394 insertions(+), 12 deletions(-)
   create mode 100644 protocol/pointer-lock.xml
 
  diff --git a/Makefile.am b/Makefile.am
  index b2d6893..ed74983 100644
  --- a/Makefile.am
  +++ b/Makefile.am
  @@ -79,7 +79,9 @@ nodist_weston_SOURCES =   
  \
  protocol/workspaces-protocol.c  \
  protocol/workspaces-server-protocol.h   \
  protocol/scaler-protocol.c  \
  -   protocol/scaler-server-protocol.h
  +   protocol/scaler-server-protocol.h   \
  +   protocol/pointer-lock-protocol.c\
  +   protocol/pointer-lock-server-protocol.h
 
   BUILT_SOURCES += $(nodist_weston_SOURCES)
 
  @@ -987,7 +989,8 @@ EXTRA_DIST +=   \
  protocol/wayland-test.xml   \
  protocol/xdg-shell.xml  \
  protocol/fullscreen-shell.xml   \
  -   protocol/scaler.xml
  +   protocol/scaler.xml \
  +   protocol/pointer-lock.xml
 
   man_MANS = weston.1 weston.ini.5
 
  diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
  index 4b65cbd..d6ec6a1 100644
  --- a/desktop-shell/exposay.c
  +++ b/desktop-shell/exposay.c
  @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab)
 
   static void
   exposay_motion(struct weston_pointer_grab *grab, uint32_t time,
  -  wl_fixed_t x, wl_fixed_t y)
  +  wl_fixed_t x, wl_fixed_t y, int rel)
   {
  struct desktop_shell *shell =
  container_of(grab, struct desktop_shell, exposay.grab_ptr);
  diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
  index 3a5a702..35b05d0 100644
  --- a/desktop-shell/shell.c
  +++ b/desktop-shell/shell.c
  @@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int 
  *cx, int *cy)
 
   static void
   move_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
  -wl_fixed_t x, wl_fixed_t y)
  +wl_fixed_t x, wl_fixed_t y, int rel)
   {
  struct weston_move_grab *move = (struct weston_move_grab *) grab;
  struct weston_pointer *pointer = grab-pointer;
  @@ -1772,7 +1772,7 @@ struct 

Re: [PATCH weston 0/3] Pointer locking revisited

2014-09-17 Thread Jonas Ådahl
On Wed, Sep 17, 2014 at 09:55:36PM +0200, Axel Davy wrote:
 On 17/09/2014 21:39, Jonas Ådahl wrote :
 Hi,
 
 This short series introduces a pointer lock interface based on the
 previous proposal[0] by Kristian Høgsberg from last year. The first patch
 adds a protocol more or less equal to Kristians proposal, but moved out
 of wl_seat into its own global object (currently _wl_pointer_lock), with
 some concerns raised in the original thread addressed. The implementation
 in weston is new. There was no point trying to rebase the previous
 implementation due to the architectural changes done since that patch was
 written.
 
 The second patch introduces a client side API in toytoolkit, and the
 forth makes use of that API in resizor, in the same way as in the
 previous proposal, to resize its window.
 
 Note that the reason for having the underscore prefix is to have the
 possibility to push a stable variant to the wayland repo without
 breaking any clients implementing the experimental version. The
 intention is that _wl_pointer_lock ever would be considered an
 official interface, it would be renamed to wl_pointer_lock.
 
 In short, the interface works by having the client create a separate
 wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus
 until wl_pointer.release is requested and the surface gets deactivated.
 If the locked pointer object is still not released when a surface
 is activated again, the pointer will be implicitly locked to the special
 wl_pointer object again.
 
 Limitations with this approach include that it would, as far as I know,
 not be possible to fully implement pointer warping support in Xwayland
 only using this protocol. However, not sure if support for legacy
 concepts is within the scope of an interface like this. Maybe Xwayland
 needs some private way to communicate (privileged wl_xwayland interface?)
 for these type of things? Maybe it could be emulated by locking if
 warping is done with some heuristic for releasing the lock, but have not
 tried to implement that.
 
 We wouldn't be able to implement API of certain toolkits (for example
 SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the
 first (global positions etc) and the use case is most likely to lock the
 pointer and get relative motion events, and there is API available in
 both those toolkits for that. Pointer confinement (absolute motion
 events, but not letting the pointer leave the surface) is another
 functionality that wouldn't be possible. It could be emulated by having
 the client lock the pointer and draw the pointer itself given the
 relative motion events, or simply added to this protocol in some way
 (wl_pointer_lock.confine_pointer?).
 
 In the previous thread, surface transformations came up as something that
 needed to be thought through. In the updated protocol of this series, I
 clarified that relative motion events are as if the surface has not been,
 transformed. The reasoning behind this is more or less how typical use
 case looks like: 1:st person 3D games. I would expect that the viewport
 moves the same even if the window happen to have been transformed for
 example zoomed or rotated.
 
 Following the trend of keeping track of these kind of stuff, I filed[1] a
 bug on BZ as well.
 
 Thoughts? Is this the right way to go?
 
 
 Jonas
 
 [0] 
 http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html
 [1] https://bugs.freedesktop.org/show_bug.cgi?id=84014
 
 Jonas Ådahl (3):
Introduce pointer lock interface
clients: Introduce pointer lock API to toytoolkit
resizor: Make resizor also use pointer locks for resizing
 
   Makefile.am   |  11 +-
   clients/resizor.c |  62 +++
   clients/window.c  | 177 +
   clients/window.h  |  42 +++
   desktop-shell/exposay.c   |   2 +-
   desktop-shell/shell.c |  10 +-
   protocol/pointer-lock.xml |  85 ++
   src/compositor.c  |   4 +
   src/compositor.h  |  21 +++-
   src/input.c   | 277 
  +-
   10 files changed, 678 insertions(+), 13 deletions(-)
   create mode 100644 protocol/pointer-lock.xml
 
 Thanks for getting into this.
 
 Could you clarify where the cursor should be during the lock,
 and what should be the cursor position when a lock is ended ?

Sure. Can add a clause saying that the the position of the actual pointer
will stay where it was before the lock was initiated (be it by
lock_pointer or surface activation). Regarding the cursor, hiding it
during it being locked probably makes most sense.


Jonas

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


Re: [PATCH weston 1/3] Introduce pointer lock interface

2014-09-17 Thread Jonas Ådahl
On Wed, Sep 17, 2014 at 11:16:06PM +0300, Giulio Camuffo wrote:
 2014-09-17 23:11 GMT+03:00 Jonas Ådahl jad...@gmail.com:
  On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo wrote:
  I haven't looked at the implementation yet, just at the protocol, but
  isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a
  problem? Objects should have a unique factory interface, or else the
  version of the interface can't be determined.
 
  Is it really? In the implementation below, the wl_pointer object gets
  the same version as the wl_pointer object that is locked. It is also
  specified in the last paragraph of _wl_pointer_lock.lock_pointer.
 
 Mmh, then maybe it is fine. I'm not convinced actually, but I'm too
 tired now. :)
 
 
 
  Also, this would not be the first interface that creates objects that
  other interface do as well. For example wl_shm_pool.create_buffer and
  wl_drm.create_buffer both create wl_buffer's.
 
 Yes, indeed wl_buffer and wl_callback version can't be increased, afaik.

Then, another example, (of a non-final protoc though):
wl_input_method_context.grab_keyboard vs wl_seat.get_keyboard, both
creating wl_keyboard objects.


Jonas

 
 
 --
 Giulio
 
 
 
 
  Jonas
 
 
 
  --
  Giulio
 
 
  2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com:
   Introduce a pointer lock interface and implementation. The interface
   consists of a global currently called _wl_pointer_lock. It is prefixed
   with an underscore (_) in order to not conflict with a potential
   official protocol in Wayland, and if moving it there, the prefixed
   should be removed.
  
   The protocol works by exposing the global _wl_pointer_lock to the
   client, and when the client wants to lock the pointer, it creates a new
   wl_pointer object using the 'lock_pointer' request. A more detailed
   description of the protocol seen in pointer-lock.xml file.
  
   The interface is based on the W3C pointer lock interface [0].
  
   [0] http://www.w3.org/TR/pointerlock/
  
   Signed-off-by: Jonas Ådahl jad...@gmail.com
   ---
  
   The part of the implementation I'm not very happy with is the internal
   API change: the extra argument in the pointer lock motion callback used
   to determine if the motion event was a result of a relative motion or an
   absolute motion. In this implementation absolute events are simply
   ignored as they don't tend to behave in any sensible way. One could
   generate relative motion events from absolute ones as done from a
   touchpad, but one would need some kind of threshold as there are no
   touch down or touch up events.
  
   With backends where relative and absolute events are indistinguishable
   (X11, Wayland) is tricker, as there is no way to know whether the event
   is suitable or not. One could also just let the client deal with it
   making detached absolute motions just result in very long vectors.
  
  
   Jonas
  
Makefile.am   |   7 +-
desktop-shell/exposay.c   |   2 +-
desktop-shell/shell.c |  10 +-
protocol/pointer-lock.xml |  85 ++
src/compositor.c  |   4 +
src/compositor.h  |  21 +++-
src/input.c   | 277 
   +-
7 files changed, 394 insertions(+), 12 deletions(-)
create mode 100644 protocol/pointer-lock.xml
  
   diff --git a/Makefile.am b/Makefile.am
   index b2d6893..ed74983 100644
   --- a/Makefile.am
   +++ b/Makefile.am
   @@ -79,7 +79,9 @@ nodist_weston_SOURCES =
  \
   protocol/workspaces-protocol.c  \
   protocol/workspaces-server-protocol.h   \
   protocol/scaler-protocol.c  \
   -   protocol/scaler-server-protocol.h
   +   protocol/scaler-server-protocol.h   \
   +   protocol/pointer-lock-protocol.c\
   +   protocol/pointer-lock-server-protocol.h
  
BUILT_SOURCES += $(nodist_weston_SOURCES)
  
   @@ -987,7 +989,8 @@ EXTRA_DIST +=   \
   protocol/wayland-test.xml   \
   protocol/xdg-shell.xml  \
   protocol/fullscreen-shell.xml   \
   -   protocol/scaler.xml
   +   protocol/scaler.xml \
   +   protocol/pointer-lock.xml
  
man_MANS = weston.1 weston.ini.5
  
   diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
   index 4b65cbd..d6ec6a1 100644
   --- a/desktop-shell/exposay.c
   +++ b/desktop-shell/exposay.c
   @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab)
  
static void
exposay_motion(struct weston_pointer_grab *grab, uint32_t time,
   -  wl_fixed_t x, wl_fixed_t y)
   +  wl_fixed_t x, wl_fixed_t y, int rel)
{
   struct desktop_shell *shell =
   container_of(grab, struct desktop_shell, 
   exposay.grab_ptr);
   diff --git 

Re: [PATCH libinput 1/3] touchpad: Enlarge topbutton area a bit while the touchpad is disabled

2014-09-17 Thread Peter Hutterer
On Wed, Sep 17, 2014 at 03:35:30PM +0200, Hans de Goede wrote:
 Make it easier to hit the topbutton area when the touchpad is disabled,
 normally we don't want to make the topbutton area too big, so as to not
 interfere with normal touchpad operation, but when disabled we have no such
 worries.

even though it may not seem like the scalable solution, I'd really prefer
this to be two functions, expand and normal or so. We don't need a
generic multiplier here (we don't plan to add more than 2 sizes) and
something like tp_expand_softbuttons(tp, device) or 
tp_set_softbutton_size_large(tp, device) is easier to comprehend IMO
than adding the raw numbers as multipliers of some magic size.

Plus, we may end up having some magic number being the best size 
that's not a straightforward multiplier in which case the multiplier just
adds to the confusion. So let's just use two sizes, normal and large and go
with those.

Cheers,
   Peter

 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  src/evdev-mt-touchpad-buttons.c | 19 ---
  src/evdev-mt-touchpad.c |  4 
  src/evdev-mt-touchpad.h |  4 +++-
  3 files changed, 19 insertions(+), 8 deletions(-)
 
 diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
 index 865346b..f0800d7 100644
 --- a/src/evdev-mt-touchpad-buttons.c
 +++ b/src/evdev-mt-touchpad-buttons.c
 @@ -496,7 +496,8 @@ tp_release_all_buttons(struct tp_dispatch *tp,
  
  void
  tp_init_softbuttons(struct tp_dispatch *tp,
 - struct evdev_device *device)
 + struct evdev_device *device,
 + double topbutton_size_mult)
  {
   int width, height;
   const struct input_absinfo *absinfo_x, *absinfo_y;
 @@ -524,14 +525,18 @@ tp_init_softbuttons(struct tp_dispatch *tp,
   tp-buttons.bottom_area.rightbutton_left_edge = width/2 + xoffset;
  
   if (tp-buttons.has_topbuttons) {
 - /* T440s has the top button line 5mm from the top,
 -event analysis has shown events to start down to ~10mm
 -from the top - which maps to 15% */
 + /* T440s has the top button line 5mm from the top, event
 +analysis has shown events to start down to ~10mm from the
 +top - which maps to 15%.  We allow the caller to enlarge the
 +area using a multiplier for the touchpad disabled case. */
 + double topsize_mm = 10 * topbutton_size_mult;
 + double topsize_pct = .15 * topbutton_size_mult;
 +
   if (yres  1) {
   tp-buttons.top_area.bottom_edge =
 - yoffset + 10 * yres;
 + yoffset + topsize_mm * yres;
   } else {
 - tp-buttons.top_area.bottom_edge = height * .15 + 
 yoffset;
 + tp-buttons.top_area.bottom_edge = height * topsize_pct 
 + yoffset;
   }
   tp-buttons.top_area.rightbutton_left_edge = width * .58 + 
 xoffset;
   tp-buttons.top_area.leftbutton_right_edge = width * .42 + 
 xoffset;
 @@ -581,7 +586,7 @@ tp_init_buttons(struct tp_dispatch *tp,
   tp-buttons.use_clickfinger = true;
  
   if (tp-buttons.is_clickpad  !tp-buttons.use_clickfinger) {
 - tp_init_softbuttons(tp, device);
 + tp_init_softbuttons(tp, device, 1.0);
   } else {
   tp-buttons.bottom_area.top_edge = INT_MAX;
   tp-buttons.top_area.bottom_edge = INT_MIN;
 diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
 index 9e568ad..1c32cc6 100644
 --- a/src/evdev-mt-touchpad.c
 +++ b/src/evdev-mt-touchpad.c
 @@ -645,6 +645,8 @@ tp_suspend(struct tp_dispatch *tp, struct evdev_device 
 *device)
*/
   if (tp-buttons.has_topbuttons) {
   evdev_notify_suspended_device(device);
 + /* Enlarge topbutton area while suspended */
 + tp_init_softbuttons(tp, device, 1.5);
   } else {
   evdev_device_suspend(device);
   }
 @@ -656,6 +658,8 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device 
 *device)
   if (tp-buttons.has_topbuttons) {
   /* tap state-machine is offline while suspended, reset state */
   tp_clear_state(tp, device);
 + /* restore original topbutton area size */
 + tp_init_softbuttons(tp, device, 1.0);
   evdev_notify_resumed_device(device);
   } else {
   evdev_device_resume(device);
 diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
 index 15cca76..e0c8c47 100644
 --- a/src/evdev-mt-touchpad.h
 +++ b/src/evdev-mt-touchpad.h
 @@ -245,7 +245,9 @@ int
  tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device);
  
  void
 -tp_init_softbuttons(struct tp_dispatch *tp, struct evdev_device *device);
 +tp_init_softbuttons(struct tp_dispatch *tp,
 + struct evdev_device *device,
 + double 

Re: [PATCH libinput 3/3] test: Add trackpoint middlebutton scrolling tests

2014-09-17 Thread Peter Hutterer
On Wed, Sep 17, 2014 at 03:35:32PM +0200, Hans de Goede wrote:
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  test/Makefile.am |   5 +++
  test/litest-trackpoint.c |   2 +-
  test/litest.h|   1 +
  test/trackpoint.c| 100 
 +++
  4 files changed, 107 insertions(+), 1 deletion(-)
  create mode 100644 test/trackpoint.c
 
 diff --git a/test/Makefile.am b/test/Makefile.am
 index 86859d8..6a68982 100644
 --- a/test/Makefile.am
 +++ b/test/Makefile.am
 @@ -34,6 +34,7 @@ run_tests = \
   test-touch \
   test-log \
   test-touchpad \
 + test-trackpoint \
   test-misc \
   test-keyboard \
   test-device
 @@ -72,6 +73,10 @@ test_touchpad_SOURCES = touchpad.c
  test_touchpad_LDADD = $(TEST_LIBS)
  test_touchpad_LDFLAGS = -no-install
  
 +test_trackpoint_SOURCES = trackpoint.c
 +test_trackpoint_LDADD = $(TEST_LIBS)
 +test_trackpoint_LDFLAGS = -no-install
 +
  test_misc_SOURCES = misc.c
  test_misc_LDADD = $(TEST_LIBS)
  test_misc_LDFLAGS = -no-install
 diff --git a/test/litest-trackpoint.c b/test/litest-trackpoint.c
 index 40b9ed0..01ad34e 100644
 --- a/test/litest-trackpoint.c
 +++ b/test/litest-trackpoint.c
 @@ -56,7 +56,7 @@ static int events[] = {
  
  struct litest_test_device litest_trackpoint_device = {
   .type = LITEST_TRACKPOINT,
 - .features = LITEST_POINTER | LITEST_BUTTON,
 + .features = LITEST_POINTER | LITEST_BUTTON | LITEST_POINTINGSTICK,
   .shortname = trackpoint,
   .setup = litest_trackpoint_setup,
   .interface = interface,
 diff --git a/test/litest.h b/test/litest.h
 index fdf815f..fca6acb 100644
 --- a/test/litest.h
 +++ b/test/litest.h
 @@ -61,6 +61,7 @@ enum litest_device_feature {
   LITEST_APPLE_CLICKPAD = 1  8,
   LITEST_TOPBUTTONPAD = 1  9,
   LITEST_SEMI_MT = 1  10,
 + LITEST_POINTINGSTICK = 1  11,
  };
  
  struct litest_device {
 diff --git a/test/trackpoint.c b/test/trackpoint.c
 new file mode 100644
 index 000..038185b
 --- /dev/null
 +++ b/test/trackpoint.c
 @@ -0,0 +1,100 @@
 +/*
 + * Copyright © 2014 Red Hat, Inc.
 + *
 + * 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.
 + */
 +
 +#include config.h
 +
 +#include check.h
 +#include errno.h
 +#include fcntl.h
 +#include libinput.h
 +#include unistd.h
 +
 +#include libinput-util.h
 +#include litest.h
 +
 +START_TEST(trackpoint_middlebutton)
 +{
 + struct litest_device *dev = litest_current_device();
 + struct libinput *li = dev-libinput;
 +
 + litest_drain_events(li);
 +
 + /* A quick middle button click should get reported normally */
 + litest_button_click(dev, BTN_MIDDLE, 1);
 + litest_button_click(dev, BTN_MIDDLE, 0);
 +
 + litest_assert_button_event(li, BTN_MIDDLE, 1);
 + litest_assert_button_event(li, BTN_MIDDLE, 0);
 +
 + litest_assert_empty_queue(li);
 +}
 +END_TEST

add a test for a middlebutton timeout without trackstick motion please.

 +
 +static void
 +test_2fg_scroll(struct litest_device *dev, double dx, double dy)
 +{
 + struct libinput *li = dev-libinput;
 +
 + litest_button_click(dev, BTN_MIDDLE, 1);
 +
 + libinput_dispatch(li);
 + msleep(300);
 + libinput_dispatch(li);
 +
 + litest_event(dev, EV_REL, REL_X, dx);
 + litest_event(dev, EV_REL, REL_Y, dy);
 + litest_event(dev, EV_SYN, SYN_REPORT, 0);
 +
 + litest_button_click(dev, BTN_MIDDLE, 0);
 +
 + libinput_dispatch(li);
 +}
 +
 +START_TEST(trackpoint_scroll)
 +{
 + struct litest_device *dev = litest_current_device();
 + struct libinput *li = dev-libinput;
 +
 + litest_drain_events(li);
 +
 + test_2fg_scroll(dev, 1, 6);
 + litest_assert_scroll(li, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, 6);
 + test_2fg_scroll(dev, 1, -7);
 + litest_assert_scroll(li, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, -7);
 + 

Re: [PATCH libinput 2/3] litest: Add litest_assert_scroll() helper function

2014-09-17 Thread Peter Hutterer
On Wed, Sep 17, 2014 at 03:35:31PM +0200, Hans de Goede wrote:
 Make check_2fg_scroll functionality available outside of touchpad.c ,
 no functional changes.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  test/litest.c   | 39 +++
  test/litest.h   |  1 +
  test/touchpad.c | 49 -
  3 files changed, 44 insertions(+), 45 deletions(-)
 
 diff --git a/test/litest.c b/test/litest.c
 index cf7e692..e5092b8 100644
 --- a/test/litest.c
 +++ b/test/litest.c
 @@ -1084,3 +1084,42 @@ litest_assert_button_event(struct libinput *li, 
 unsigned int button,
state);
   libinput_event_destroy(event);
  }
 +
 +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir)
 +{
 + struct libinput_event *event, *next_event;
 + struct libinput_event_pointer *ptrev;
 +
 + event = libinput_get_event(li);
 + next_event = libinput_get_event(li);
 + ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */

ck_assert_notnull(), not that it makes much difference.

 +
 + while (event) {
 + ck_assert_int_eq(libinput_event_get_type(event),
 +  LIBINPUT_EVENT_POINTER_AXIS);
 + ptrev = libinput_event_get_pointer_event(event);
 + ck_assert(ptrev != NULL);
 + ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis);
 +
 + if (next_event) {
 + /* Normal scroll event, check dir */
 + if (dir  0) {
 + ck_assert_int_ge(
 + 
 libinput_event_pointer_get_axis_value(ptrev),
 + dir);

if you use a temporary var for the axis values we can skip the
indentation/linewidth dance.

Ok, and now that I wrote this I see that's just moving the code around. so
Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
for this one and I'll push the updates on top of it.

Cheers,
   Peter



 + } else {
 + ck_assert_int_le(
 + 
 libinput_event_pointer_get_axis_value(ptrev),
 + dir);
 + }
 + } else {
 + /* Last scroll event, must be 0 */
 + ck_assert_int_eq(
 + libinput_event_pointer_get_axis_value(ptrev),
 + 0);
 + }
 + libinput_event_destroy(event);
 + event = next_event;
 + next_event = libinput_get_event(li);
 + }
 +}
 diff --git a/test/litest.h b/test/litest.h
 index dd1386c..fdf815f 100644
 --- a/test/litest.h
 +++ b/test/litest.h
 @@ -147,6 +147,7 @@ void litest_assert_empty_queue(struct libinput *li);
  void litest_assert_button_event(struct libinput *li,
   unsigned int button,
   enum libinput_button_state state);
 +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir);
  
  struct libevdev_uinput * litest_create_uinput_device(const char *name,
struct input_id *id,
 diff --git a/test/touchpad.c b/test/touchpad.c
 index 7ff3d14..522cee6 100644
 --- a/test/touchpad.c
 +++ b/test/touchpad.c
 @@ -1346,47 +1346,6 @@ test_2fg_scroll(struct litest_device *dev, double dx, 
 double dy, int sleep)
   libinput_dispatch(li);
  }
  
 -static void
 -check_2fg_scroll(struct litest_device *dev, unsigned int axis, int dir)
 -{
 - struct libinput *li = dev-libinput;
 - struct libinput_event *event, *next_event;
 - struct libinput_event_pointer *ptrev;
 -
 - event = libinput_get_event(li);
 - next_event = libinput_get_event(li);
 - ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */
 -
 - while (event) {
 - ck_assert_int_eq(libinput_event_get_type(event),
 -  LIBINPUT_EVENT_POINTER_AXIS);
 - ptrev = libinput_event_get_pointer_event(event);
 - ck_assert(ptrev != NULL);
 - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis);
 -
 - if (next_event) {
 - /* Normal scroll event, check dir */
 - if (dir  0) {
 - ck_assert_int_ge(
 - 
 libinput_event_pointer_get_axis_value(ptrev),
 - dir);
 - } else {
 - ck_assert_int_le(
 - 
 libinput_event_pointer_get_axis_value(ptrev),
 - dir);
 - }
 - } else {
 - /* Last scroll event, must be 0 */
 - ck_assert_int_eq(
 - 

Re: [PATCH libinput 2/8] evdev: Add middle button scrolling for trackpoints

2014-09-17 Thread Peter Hutterer
On Tue, Sep 16, 2014 at 04:22:36PM +0200, Hans de Goede wrote:
 Most trackpoint users want to be able to scroll using the trackpoint with
 the middle button pressed, add support for this.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
 ---
  include/linux/input.h|  1 +
  src/evdev.c  | 56 
 
  src/evdev.h  |  5 +
  test/litest-trackpoint.c |  2 ++
  test/pointer.c   |  4 +++-
  5 files changed, 67 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/input.h b/include/linux/input.h
 index aa98ce7..39b550b 100644
 --- a/include/linux/input.h
 +++ b/include/linux/input.h
 @@ -163,6 +163,7 @@ struct input_keymap_entry {
  #define INPUT_PROP_BUTTONPAD 0x02/* has button(s) under pad */
  #define INPUT_PROP_SEMI_MT   0x03/* touch rectangle only */
  #define INPUT_PROP_TOPBUTTONPAD  0x04/* softbuttons at top 
 of pad */
 +#define INPUT_PROP_POINTING_STICK0x05/* is a pointing stick */
  
  #define INPUT_PROP_MAX   0x1f
  #define INPUT_PROP_CNT   (INPUT_PROP_MAX + 1)
 diff --git a/src/evdev.c b/src/evdev.c
 index 45020ba..85e4d71 100644
 --- a/src/evdev.c
 +++ b/src/evdev.c
 @@ -41,6 +41,7 @@
  #include libinput-private.h
  
  #define DEFAULT_AXIS_STEP_DISTANCE 10
 +#define DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT 200
  
  enum evdev_key_type {
   EVDEV_KEY_TYPE_NONE,
 @@ -203,6 +204,15 @@ evdev_flush_pending_event(struct evdev_device *device, 
 uint64_t time)
   device-rel.dx = 0;
   device-rel.dy = 0;
  
 + /* Use unaccelerated deltas for pointing stick scroll */
 + if (device-scroll.has_middle_button_scroll 
 + hw_is_key_down(device, BTN_MIDDLE)) {
 + if (device-scroll.middle_button_scroll_active)
 + evdev_post_scroll(device, time,
 +   motion.dx, motion.dy);
 + break;
 + }
 +

Just to verify: the behaviour here is that the scrolling does not activate
until after the timeout, regardless of the number of motion events during
the timeout period. Is this intentional, or do we need a threshold here?

Cheers,
   Peter


   /* Apply pointer acceleration. */
   filter_dispatch(device-pointer.filter, motion, device, time);
  
 @@ -346,6 +356,37 @@ get_key_type(uint16_t code)
  }
  
  static void
 +evdev_middle_button_scroll_timeout(uint64_t time, void *data)
 +{
 + struct evdev_device *device = data;
 +
 + device-scroll.middle_button_scroll_active = true;
 +}
 +
 +static void
 +evdev_middle_button_scroll_button(struct evdev_device *device,
 +  uint64_t time, int is_press)
 +{
 + if (is_press) {
 + libinput_timer_set(device-scroll.timer,
 + time + DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT);
 + } else {
 + libinput_timer_cancel(device-scroll.timer);
 + if (device-scroll.middle_button_scroll_active) {
 + evdev_stop_scroll(device, time);
 + device-scroll.middle_button_scroll_active = false;
 + } else {
 + /* If the button is released quickly enough emit the
 +  * button press/release events. */
 + evdev_pointer_notify_button(device, time, BTN_MIDDLE,
 + LIBINPUT_BUTTON_STATE_PRESSED);
 + evdev_pointer_notify_button(device, time, BTN_MIDDLE,
 + LIBINPUT_BUTTON_STATE_RELEASED);
 + }
 + }
 +}
 +
 +static void
  evdev_process_touch_button(struct evdev_device *device,
  uint64_t time, int value)
  {
 @@ -405,6 +446,12 @@ evdev_process_key(struct evdev_device *device,
  LIBINPUT_KEY_STATE_RELEASED);
   break;
   case EVDEV_KEY_TYPE_BUTTON:
 + if (device-scroll.has_middle_button_scroll 
 + e-code == BTN_MIDDLE) {
 + evdev_middle_button_scroll_button(device, time,
 +   e-value);
 + break;
 + }
   evdev_pointer_notify_button(
   device,
   time,
 @@ -946,6 +993,15 @@ evdev_configure_device(struct evdev_device *device)
   device-mt.slot = active_slot;
   }
   }
 +
 + if (libevdev_has_property(evdev, INPUT_PROP_POINTING_STICK)) {
 + libinput_timer_init(device-scroll.timer,
 + device-base.seat-libinput,
 + evdev_middle_button_scroll_timeout,
 + device);
 + 

Re: [PATCH libinput 5/8] touchpad: Route top softbuttons through the trackstick if we've one

2014-09-17 Thread Peter Hutterer
On Tue, Sep 16, 2014 at 04:22:39PM +0200, Hans de Goede wrote:
 The touchpad top softbuttons such as found on the Lenove T440 are intended for
 use with the trackstick. Route their events through the trackstick, so that
 they can be used for e.g. middle button scrolling with the trackstick.
 
 Note that sending top button events to a disabled trackpoint makes no sense
 (and will mess up internal state). Likely a user with a disabled trackpoint
 will still expect the top buttons to work, so rather than not sending events
 in that case, simply treat a suspendeded trackpoint as not being there, and
 send the events directly from the touchpad device.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com

Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
but see the notes below

 ---
  doc/touchpad-softbutton-state-machine.svg | 200 
 +-
  src/evdev-mt-touchpad-buttons.c   |  45 +--
  src/evdev-mt-touchpad.c   |  17 ++-
  src/evdev-mt-touchpad.h   |   1 +
  4 files changed, 163 insertions(+), 100 deletions(-)
 
 diff --git a/doc/touchpad-softbutton-state-machine.svg 
 b/doc/touchpad-softbutton-state-machine.svg
 index 1d569bf..ffa17a2 100644
 --- a/doc/touchpad-softbutton-state-machine.svg
 +++ b/doc/touchpad-softbutton-state-machine.svg
[...]
 diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
 index 02d3205..0fdabde 100644
 --- a/src/evdev-mt-touchpad-buttons.c
 +++ b/src/evdev-mt-touchpad-buttons.c
 @@ -675,16 +675,40 @@ tp_post_physical_buttons(struct tp_dispatch *tp, 
 uint64_t time)
   return 0;
  }
  
 +static void
 +tp_notify_softbutton(struct tp_dispatch *tp,
 +  uint64_t time,
 +  uint32_t button,
 +  uint32_t is_top,

just because I prefer it: I replaced this with is_topbutton, and
active_is_top with active_is_topbutton (simple sed, both times)

 +  enum libinput_button_state state)
 +{
 + /* If we've a trackpoint, send top buttons through the trackpoint */
 + if (is_top  tp-buttons.trackpoint) {
 + struct evdev_dispatch *dispatch = 
 tp-buttons.trackpoint-dispatch;
 + struct input_event event;
 +
 + event.type = EV_KEY,
 + event.code = button,

I'm going to assume you meant ; here instead of ',' so I've replaced those
after applying.

 + event.value = (state == LIBINPUT_BUTTON_STATE_PRESSED) ? 1 : 0;

for correctness, we should set the time here too, I've added

+   event.time.tv_sec = time/1000;
+   event.time.tv_usec = (time % 1000) * 1000;


patch coming up, just double-check it please.

Cheers,
   Peter

 + dispatch-interface-process(dispatch, tp-buttons.trackpoint,
 +  event, time);
 + return;
 + }
 +
 + evdev_pointer_notify_button(tp-device, time, button, state);
 +}
 +
  static int
  tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time)
  {
 - uint32_t current, old, button;
 + uint32_t current, old, button, is_top;
   enum libinput_button_state state;
   enum { AREA = 0x01, LEFT = 0x02, MIDDLE = 0x04, RIGHT = 0x08 };
  
   current = tp-buttons.state;
   old = tp-buttons.old_state;
   button = 0;
 + is_top = 0;
  
   if (!tp-buttons.click_pending  current == old)
   return 0;
 @@ -697,15 +721,18 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, 
 uint64_t time)
   case BUTTON_EVENT_IN_AREA:
   button |= AREA;
   break;
 - case BUTTON_EVENT_IN_BOTTOM_L:
   case BUTTON_EVENT_IN_TOP_L:
 + is_top = 1;
 + case BUTTON_EVENT_IN_BOTTOM_L:
   button |= LEFT;
   break;
   case BUTTON_EVENT_IN_TOP_M:
 + is_top = 1;
   button |= MIDDLE;
   break;
 - case BUTTON_EVENT_IN_BOTTOM_R:
   case BUTTON_EVENT_IN_TOP_R:
 + is_top = 1;
 + case BUTTON_EVENT_IN_BOTTOM_R:
   button |= RIGHT;
   break;
   default:
 @@ -727,21 +754,21 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, 
 uint64_t time)
   button = BTN_LEFT;
  
   tp-buttons.active = button;
 + tp-buttons.active_is_top = is_top;
   state = LIBINPUT_BUTTON_STATE_PRESSED;
   } else {
   button = tp-buttons.active;
 + is_top = tp-buttons.active_is_top;
   tp-buttons.active = 0;
 + tp-buttons.active_is_top = 0;
   state = LIBINPUT_BUTTON_STATE_RELEASED;
   }
  
  

[PATCH v2 libinput] touchpad: Route top softbuttons through the trackstick if we've one

2014-09-17 Thread Peter Hutterer
From: Hans de Goede hdego...@redhat.com

The touchpad top softbuttons such as found on the Lenove T440 are intended for
use with the trackstick. Route their events through the trackstick, so that
they can be used for e.g. middle button scrolling with the trackstick.

Note that sending top button events to a disabled trackpoint makes no sense
(and will mess up internal state). Likely a user with a disabled trackpoint
will still expect the top buttons to work, so rather than not sending events
in that case, simply treat a suspendeded trackpoint as not being there, and
send the events directly from the touchpad device.

Signed-off-by: Hans de Goede hdego...@redhat.com
Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
Changes to v1:
- rename is_top and active_is_top to *is_topbutton for better clarity
- add timestamp to struct input_event generation 
- replace two commas with semicolons (typos)
- change active_is_topbutton to a bool (from unsigned int)

 doc/touchpad-softbutton-state-machine.svg | 198 +-
 src/evdev-mt-touchpad-buttons.c   |  47 +--
 src/evdev-mt-touchpad.c   |  17 ++-
 src/evdev-mt-touchpad.h   |   1 +
 4 files changed, 164 insertions(+), 99 deletions(-)

diff --git a/doc/touchpad-softbutton-state-machine.svg 
b/doc/touchpad-softbutton-state-machine.svg
index 1d569bf..ffa17a2 100644
--- a/doc/touchpad-softbutton-state-machine.svg
+++ b/doc/touchpad-softbutton-state-machine.svg
@@ -1,5 +1,5 @@
 ?xml version=1.0?
-svg xmlns=http://www.w3.org/2000/svg; 
xmlns:xlink=http://www.w3.org/1999/xlink; width=2190px height=1624px 
version=1.1
+svg xmlns=http://www.w3.org/2000/svg; 
xmlns:xlink=http://www.w3.org/1999/xlink; width=2180px height=1624px 
version=1.1
   defs/
   g transform=translate(0.5,0.5)
 path d=M 862 441 L 894 441 fill=none stroke=#00 
stroke-miterlimit=10 pointer-events=none/
@@ -68,13 +68,13 @@
 path d=M 712 441 L 668 441 Q 658 441 658 451 L 658 762 fill=none 
stroke=#00 stroke-miterlimit=10 pointer-events=none/
 path d=M 786 62 L 786 81 Q 786 91 786 101 L 786 114 fill=none 
stroke=#00 stroke-miterlimit=10 pointer-events=none/
 path d=M 786 120 L 783 113 L 786 114 L 790 113 Z fill=#00 
stroke=#00 stroke-miterlimit=10 pointer-events=none/
-rect x=1753 y=472 width=120 height=40 rx=16 ry=16 
fill=#c0 stroke=#ff pointer-events=none/
+rect x=1753 y=412 width=120 height=40 rx=16 ry=16 
fill=#c0 stroke=#ff pointer-events=none/
 g fill=#00 font-family=Helvetica text-anchor=middle 
font-size=12px
-  text x=1813 y=489Check state of/text
-  text x=1813 y=503all touches/text
+  text x=1813 y=429Check state of/text
+  text x=1813 y=443all touches/text
 /g
-path d=M 1813 512 L 1813 518 Q 1813 524 1813 529 L 1813 534 fill=none 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
-path d=M 1809 526 L 1813 535 L 1818 526 fill=none stroke=#ff 
stroke-miterlimit=10 pointer-events=none/
+path d=M 1813 452 L 1813 458 Q 1813 465 1813 470 L 1813 475 fill=none 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
+path d=M 1809 467 L 1813 476 L 1818 467 fill=none stroke=#ff 
stroke-miterlimit=10 pointer-events=none/
 ellipse cx=1813 cy=72 rx=11 ry=11 fill=#00 stroke=#ff 
pointer-events=none/
 path d=M 1813 87 L 1813 115 Q 1813 125 1813 135 L 1813 160 fill=none 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
 path d=M 1809 152 L 1813 161 L 1818 152 fill=none stroke=#ff 
stroke-miterlimit=10 pointer-events=none/
@@ -82,88 +82,89 @@
 g fill=#00 font-family=Helvetica text-anchor=middle 
font-size=12px
   text x=1813 y=41tp_post_softbutton_buttons()/text
 /g
-path d=M 1813 212 L 1908 267 L 1813 322 L 1718 267 Z fill=#c0 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
+path d=M 1813 192 L 1908 247 L 1813 302 L 1718 247 Z fill=#c0 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
 g fill=#00 font-family=Helvetica text-anchor=middle 
font-size=12px
-  text x=1813 y=264!buttons.click_pend/text
-  text x=1813 y=278amp;amp; current == old/text
+  text x=1813 y=244!buttons.click_pend/text
+  text x=1813 y=258amp;amp; current == old/text
 /g
-path d=M 1908 267 L 1968 267 Q 1978 267 1988 267 L 2056 267 fill=none 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
-path d=M 2048 272 L 2057 267 L 2048 263 fill=none stroke=#ff 
stroke-miterlimit=10 pointer-events=none/
+path d=M 1908 247 L 1968 247 Q 1978 247 1988 247 L 2056 247 fill=none 
stroke=#ff stroke-miterlimit=10 pointer-events=none/
+path d=M 2048 252 L 2057 247 L 2048 243 fill=none stroke=#ff 
stroke-miterlimit=10 pointer-events=none/
 g fill=#00 font-family=Helvetica font-size=11px
-  rect fill=#ff stroke=none x=1998 y=252 width=18 
height=14 

[PATCH libinput 4/7] touchpad: move updating of buttons.old_state to the process function

2014-09-17 Thread Peter Hutterer
Update the old_state once we've posted the respective buttons rather than
waiting for post_process to kick in. This allows calling our function multiple
times without double-posting the events.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/evdev-mt-touchpad-buttons.c | 7 +++
 src/evdev-mt-touchpad.c | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index fae6a13..2d4b2b9 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -640,6 +640,9 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, 
uint64_t time)
button,
state);
}
+
+   tp-buttons.old_state = tp-buttons.state;
+
return 1;
 }
 
@@ -672,6 +675,8 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t 
time)
old = 1;
}
 
+   tp-buttons.old_state = tp-buttons.state;
+
return 0;
 }
 
@@ -775,6 +780,8 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t 
time)
if (button)
tp_notify_softbutton(tp, time, button, is_top, state);
 
+   tp-buttons.old_state = tp-buttons.state;
+
return 1;
 }
 
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 26517b9..f9efc5f 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -460,7 +460,6 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t time)
}
 
tp-old_nfingers_down = tp-nfingers_down;
-   tp-buttons.old_state = tp-buttons.state;
 
tp-queued = TOUCHPAD_EVENT_NONE;
 }
-- 
1.9.3

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


[PATCH libinput 0/7] Handle jumping cursors

2014-09-17 Thread Peter Hutterer

When a finger leaves the touchpad at the same time as another finger sets
down, a touchpad may not notice the change in fingers but rather think the
touchpoint moved to the new position. Bug 76722 has an event sequence but it
really boils down to just a move of a touch, identifiable only by the
distance moved.

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

I've done some measurements on a T440 and a x220 and the most one can
reasonably move on those two is under 20mm. Use the script at the url below
if you want to verify yourself.
https://github.com/whot/input-data-analysis/tree/master/touchpad-max-delta

The idea of the fix was to consider a jumping touch a changing touch, i.e.
end it and begin it at the new position. This also requires the new touch to
be tap-capable, recognised by the softbutton code, etc.

Handling this is a bit tricky because we handle the state of the touchpad at
EV_SYN time, so we can't easily split event handling. Since we don't know
which touchpoint moves (could be the second one, in theory) we'd have to do
double-processing of the state otherwise, i.e. pull all events up to the
EV_SYN, check each touchpoint for jumps, release those, process the state,
then re-process the events again. The approach I've picked here is to handle
the state twice (rather than the events), ignoring certain things the first
time and other things the second time.

Benjamin is currently looking into a (hopefully simpler) kernel fix so maybe
we won't need this patchset anyway, but I prefer it to be archived on the
list either way. All that aside, patches 1-3 are useful anyway.

All available on 
https://github.com/whot/libinput/tree/wip/jumping-cursor

Cheers,
  Peter

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


[PATCH libinput 5/7] touchpad: only send button events if we have button events queued

2014-09-17 Thread Peter Hutterer
Even if the button state differs, only send events if we claim we have a
button press or release queued up. Otherwise, skip over the event.
This cannot happen in the current code, it's prep-work for an upcoming patch.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/evdev-mt-touchpad-buttons.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index 2d4b2b9..b0a89ae 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -618,7 +618,7 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, 
uint64_t time)
if (current == old)
return 0;
 
-   if (current) {
+   if (current  (tp-queued  TOUCHPAD_EVENT_BUTTON_PRESS)) {
switch (tp-nfingers_down) {
case 1: button = BTN_LEFT; break;
case 2: button = BTN_RIGHT; break;
@@ -628,11 +628,12 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, 
uint64_t time)
}
tp-buttons.active = button;
state = LIBINPUT_BUTTON_STATE_PRESSED;
-   } else {
+   } else if (!current  (tp-queued  TOUCHPAD_EVENT_BUTTON_RELEASE)) {
button = tp-buttons.active;
tp-buttons.active = 0;
state = LIBINPUT_BUTTON_STATE_RELEASED;
-   }
+   } else
+   return 0;
 
if (button) {
evdev_pointer_notify_button(tp-device,
@@ -649,34 +650,45 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, 
uint64_t time)
 static int
 tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time)
 {
-   uint32_t current, old, button;
+   uint32_t current, old, button, mask;
 
current = tp-buttons.state;
old = tp-buttons.old_state;
button = BTN_LEFT;
+   mask = 0x1;
 
while (current || old) {
enum libinput_button_state state;
 
if ((current  0x1) ^ (old  0x1)) {
-   if (!!(current  0x1))
+   bool skip = false;
+   if (!!(current  0x1) 
+   tp-queued  TOUCHPAD_EVENT_BUTTON_PRESS)
state = LIBINPUT_BUTTON_STATE_PRESSED;
-   else
+   else if (!(current  0x1) 
+tp-queued  TOUCHPAD_EVENT_BUTTON_RELEASE)
state = LIBINPUT_BUTTON_STATE_RELEASED;
+   else
+   skip = true;
 
-   evdev_pointer_notify_button(tp-device,
-   time,
-   button,
-   state);
+   if (!skip) {
+   evdev_pointer_notify_button(tp-device,
+   time,
+   button,
+   state);
+   if (state == LIBINPUT_BUTTON_STATE_PRESSED)
+   tp-buttons.old_state |= mask;
+   else
+   tp-buttons.old_state = ~mask;
+   }
}
 
button++;
current = 1;
old = 1;
+   mask = 1;
}
 
-   tp-buttons.old_state = tp-buttons.state;
-
return 0;
 }
 
-- 
1.9.3

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


[PATCH libinput 2/7] test: add helper functions for the two timers we care about

2014-09-17 Thread Peter Hutterer
Rather than a random msleep() with a comment, use a helper function that
describes what we're waiting for. Also makes changing the timeouts easier in
the future.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/litest.c   | 12 
 test/litest.h   |  3 +++
 test/touchpad.c | 42 +-
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index cf7e692..2b356be 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -1084,3 +1084,15 @@ litest_assert_button_event(struct libinput *li, unsigned 
int button,
 state);
libinput_event_destroy(event);
 }
+
+void
+litest_timeout_tap(void)
+{
+   msleep(200);
+}
+
+void
+litest_timeout_softbuttons(void)
+{
+   msleep(300);
+}
diff --git a/test/litest.h b/test/litest.h
index dd1386c..40d4df9 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -156,6 +156,9 @@ struct libevdev_uinput * 
litest_create_uinput_abs_device(const char *name,
 const struct 
input_absinfo *abs,
 ...);
 
+void litest_timeout_tap(void);
+void litest_timeout_softbuttons(void);
+
 #ifndef ck_assert_notnull
 #define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL)
 #endif
diff --git a/test/touchpad.c b/test/touchpad.c
index a6e2dd7..ea02f47 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -107,7 +107,7 @@ START_TEST(touchpad_1fg_tap)
 
litest_assert_button_event(li, BTN_LEFT,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_LEFT,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -162,7 +162,7 @@ START_TEST(touchpad_1fg_tap_n_drag)
 
ck_assert_int_eq(libinput_next_event_type(li), LIBINPUT_EVENT_NONE);
 
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
 
litest_assert_button_event(li, BTN_LEFT,
   LIBINPUT_BUTTON_STATE_RELEASED);
@@ -219,7 +219,7 @@ START_TEST(touchpad_2fg_tap)
 
litest_assert_button_event(li, BTN_RIGHT,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_RIGHT,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -246,7 +246,7 @@ START_TEST(touchpad_2fg_tap_inverted)
 
litest_assert_button_event(li, BTN_RIGHT,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_RIGHT,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -274,7 +274,7 @@ START_TEST(touchpad_1fg_tap_click)
litest_event(dev, EV_KEY, BTN_LEFT, 0);
litest_event(dev, EV_SYN, SYN_REPORT, 0);
libinput_dispatch(li);
-   msleep(200);
+   litest_timeout_tap();
 
libinput_dispatch(li);
 
@@ -423,7 +423,7 @@ START_TEST(touchpad_no_2fg_tap_after_timeout)
 */
litest_touch_down(dev, 0, 50, 50);
libinput_dispatch(dev-libinput);
-   msleep(300);
+   litest_timeout_tap();
libinput_dispatch(dev-libinput);
litest_drain_events(dev-libinput);
 
@@ -577,7 +577,7 @@ START_TEST(touchpad_3fg_tap)
 
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -613,7 +613,7 @@ START_TEST(touchpad_3fg_tap_btntool)
 
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -648,7 +648,7 @@ START_TEST(touchpad_3fg_tap_btntool_inverted)
 
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_PRESSED);
-   msleep(300); /* tap-n-drag timeout */
+   litest_timeout_tap();
litest_assert_button_event(li, BTN_MIDDLE,
   LIBINPUT_BUTTON_STATE_RELEASED);
 
@@ -750,7 +750,7 @@ START_TEST(clickpad_1fg_tap_click)
litest_event(dev, EV_SYN, SYN_REPORT, 0);
litest_touch_up(dev, 0);
libinput_dispatch(li);
-   msleep(200);
+   litest_timeout_tap();
 
libinput_dispatch(li);
 
@@ -999,7 +999,7 @@ START_TEST(clickpad_softbutton_left_1st_fg_move)
/* move out of the area, 

[PATCH libinput 3/7] test: add litest_push/pop_event_frame() helpers

2014-09-17 Thread Peter Hutterer
For some tests we need to string multiple event sequences together into one
event frame. Use a push/pop frame approach that stops litest from sending any
EV_SYN/SYN_REPORT events, so we can merge two touches together by e.g.

litest_push_event_frame(d);
litest_touch_down(d, 0, 10, 10);
litest_touch_down(d, 1, 20, 50);
litest_pop_event_frame(d);

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/litest.c | 22 +-
 test/litest.h |  5 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/test/litest.c b/test/litest.c
index 2b356be..eed41ba 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -634,7 +634,12 @@ void
 litest_event(struct litest_device *d, unsigned int type,
 unsigned int code, int value)
 {
-   int ret = libevdev_uinput_write_event(d-uinput, type, code, value);
+   int ret;
+
+   if (d-skip_ev_syn  type == EV_SYN  code == SYN_REPORT)
+   return;
+
+   ret = libevdev_uinput_write_event(d-uinput, type, code, value);
ck_assert_int_eq(ret, 0);
 }
 
@@ -1096,3 +1101,18 @@ litest_timeout_softbuttons(void)
 {
msleep(300);
 }
+
+void
+litest_push_event_frame(struct litest_device *dev)
+{
+   assert(!dev-skip_ev_syn);
+   dev-skip_ev_syn = true;
+}
+
+void
+litest_pop_event_frame(struct litest_device *dev)
+{
+   assert(dev-skip_ev_syn);
+   dev-skip_ev_syn = false;
+   litest_event(dev, EV_SYN, SYN_REPORT, 0);
+}
diff --git a/test/litest.h b/test/litest.h
index 40d4df9..540e07b 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -72,6 +72,8 @@ struct litest_device {
struct litest_device_interface *interface;
 
int ntouches_down;
+   bool skip_ev_syn;
+
void *private; /* device-specific data */
 };
 
@@ -159,6 +161,9 @@ struct libevdev_uinput * 
litest_create_uinput_abs_device(const char *name,
 void litest_timeout_tap(void);
 void litest_timeout_softbuttons(void);
 
+void litest_push_event_frame(struct litest_device *dev);
+void litest_pop_event_frame(struct litest_device *dev);
+
 #ifndef ck_assert_notnull
 #define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL)
 #endif
-- 
1.9.3

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


[PATCH libinput 7/7] test: add some tests for jumping fingers on touchpads

2014-09-17 Thread Peter Hutterer
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/touchpad.c | 259 
 1 file changed, 259 insertions(+)

diff --git a/test/touchpad.c b/test/touchpad.c
index ea02f47..37df992 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -1623,6 +1623,255 @@ 
START_TEST(touchpad_palm_detect_no_palm_moving_into_edges)
 }
 END_TEST
 
+START_TEST(touchpad_jump_finger_up)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_drain_events(li);
+
+   /* no motion event caused by jump*/
+   litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1);
+   litest_touch_up(dev, 0);
+   litest_assert_empty_queue(li);
+}
+END_TEST
+
+START_TEST(touchpad_jump_finger_up_2fg_scroll)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_drain_events(li);
+
+   litest_push_event_frame(dev);
+   litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1);
+   litest_touch_down(dev, 1, 50, 50);
+   litest_pop_event_frame(dev);
+
+   litest_assert_empty_queue(li);
+
+   /* Now move the first finger, expect scroll */
+   litest_touch_move_to(dev, 0, 90, 30, 20, 40, 10);
+   litest_wait_for_event_of_type(li, LIBINPUT_EVENT_POINTER_AXIS, -1);
+}
+END_TEST
+
+START_TEST(touchpad_jump_finger_up_2fg_tap)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+
+   libinput_device_config_tap_set_enabled(dev-libinput_device,
+  LIBINPUT_CONFIG_TAP_ENABLED);
+
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_drain_events(li);
+
+   litest_push_event_frame(dev);
+   litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1);
+   litest_touch_down(dev, 1, 50, 50);
+   litest_pop_event_frame(dev);
+
+   litest_assert_empty_queue(li);
+
+   /* expect two-finger tap */
+
+   litest_touch_up(dev, 0);
+   litest_touch_up(dev, 1);
+
+   litest_assert_button_event(li,
+  BTN_RIGHT,
+  LIBINPUT_BUTTON_STATE_PRESSED);
+   litest_assert_button_event(li,
+  BTN_RIGHT,
+  LIBINPUT_BUTTON_STATE_RELEASED);
+
+}
+END_TEST
+
+START_TEST(touchpad_jump_finger_up_release)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_button_click(dev, BTN_LEFT, 1);
+   litest_drain_events(li);
+
+   /* send BTN_LEFT release on touch jump */
+   litest_push_event_frame(dev);
+   litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1);
+   litest_event(dev, EV_KEY, BTN_LEFT, 0);
+   litest_pop_event_frame(dev);
+   litest_touch_up(dev, 0);
+
+   litest_assert_button_event(li,
+  BTN_LEFT,
+  LIBINPUT_BUTTON_STATE_RELEASED);
+   litest_assert_empty_queue(li);
+}
+END_TEST
+
+START_TEST(touchpad_jump_finger_up_press)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_drain_events(li);
+
+   /* send BTN_LEFT press on touch jump */
+   litest_push_event_frame(dev);
+   litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1);
+   litest_event(dev, EV_KEY, BTN_LEFT, 1);
+   litest_pop_event_frame(dev);
+   litest_touch_up(dev, 0);
+
+   litest_assert_button_event(li,
+  BTN_LEFT,
+  LIBINPUT_BUTTON_STATE_PRESSED);
+   litest_assert_empty_queue(li);
+}
+END_TEST
+
+START_TEST(touchpad_jump_finger_up_tap)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev-libinput;
+   struct libinput_event *event;
+   struct libinput_event_pointer *ptrev;
+
+   libinput_device_config_tap_set_enabled(dev-libinput_device,
+  LIBINPUT_CONFIG_TAP_ENABLED);
+   litest_drain_events(li);
+   litest_touch_down(dev, 0, 20, 30);
+   litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10);
+   litest_drain_events(li);
+
+   /* jumping finger counts as new finger, so 

[PATCH libinput 6/7] touchpad: detect coordinate jumps on touchpads

2014-09-17 Thread Peter Hutterer
Such jumps are usually consequences of the touchpad firmware failing to
notice that a different finger is in fact touching the touchpad.
This happens frequently when a finger is lifted at the same time as a
second finger is set down. Instead of a ABS_MT_TRACKING_ID  of -1
followed by a new touchpoing we get a large movement from the previous
position to the next position. This leads to a cursor jump on the
screen.

This patch adds detection for these jumps. Since the new finger is a new
touchpoint, all the usual rules for a new touch point should apply
(softbutton detection, tapping, etc.).

Thus, if we find one or more touchpoints to have jumped, we
mark the touchpoint as jumped and end it. Then we process the current
state, releasing all buttons where appropriate.
Then we re-process the state, marking all jumped and ended touchpoints
as TOUCH_BEGIN (and pressing buttons where appropriate).

The jump threshold is 20mm on devices with resolution, or a
quarter of the diagonal for devices without a resolution.

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

Based on a patch by Alexander E. Patrakov patra...@gmail.com
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 src/evdev-mt-touchpad-buttons.c |  17 +-
 src/evdev-mt-touchpad.c | 123 ++--
 src/evdev-mt-touchpad.h |   7 ++-
 3 files changed, 138 insertions(+), 9 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index b0a89ae..11b309b 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -416,7 +416,9 @@ tp_button_handle_event(struct tp_dispatch *tp,
 }
 
 int
-tp_button_handle_state(struct tp_dispatch *tp, uint64_t time)
+tp_button_handle_state(struct tp_dispatch *tp,
+  uint64_t time,
+  bool ignore_button_press)
 {
struct tp_touch *t;
 
@@ -440,9 +442,11 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t 
time)
else
tp_button_handle_event(tp, t, 
BUTTON_EVENT_IN_AREA, time);
}
+
if (tp-queued  TOUCHPAD_EVENT_BUTTON_RELEASE)
tp_button_handle_event(tp, t, BUTTON_EVENT_RELEASE, 
time);
-   if (tp-queued  TOUCHPAD_EVENT_BUTTON_PRESS)
+   if (!ignore_button_press 
+   tp-queued  TOUCHPAD_EVENT_BUTTON_PRESS)
tp_button_handle_event(tp, t, BUTTON_EVENT_PRESS, time);
}
 
@@ -662,6 +666,14 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t 
time)
 
if ((current  0x1) ^ (old  0x1)) {
bool skip = false;
+
+   /* we may have button state changes but only process
+  them if the matching tp-queued mask is set.
+
+  If a touch jump happens, the various post functions
+  are called separately with the queued mask manually
+  adjusted and we post release events before
+  press events */
if (!!(current  0x1) 
tp-queued  TOUCHPAD_EVENT_BUTTON_PRESS)
state = LIBINPUT_BUTTON_STATE_PRESSED;
@@ -682,7 +694,6 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t 
time)
tp-buttons.old_state = ~mask;
}
}
-
button++;
current = 1;
old = 1;
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index f9efc5f..426c22b 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -160,6 +160,67 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, 
uint64_t time)
tp-queued |= TOUCHPAD_EVENT_MOTION;
 }
 
+static inline void
+tp_disrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+   if (t-state == TOUCH_NONE)
+   return;
+
+   tp_end_touch(tp, t, time);
+   t-has_jumped = true;
+}
+
+static inline void
+tp_undisrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+   if (!t-has_jumped)
+   return;
+
+   t-has_jumped = false;
+   tp_begin_touch(tp, t, time);
+}
+
+static inline int
+tp_detect_jumps(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+   struct tp_motion *oldpos;
+   double dx, dy;
+
+   /* Detect cursor jumps. The sampling rate of touchpads limits what
+* we can detect, if one finger leaves touch and another sets down
+* quickly enough, the firmware may not notice the touch up/down
+* event but assume the finger moved.
+* See https://bugs.freedesktop.org/show_bug.cgi?id=76722
+*
+* We detect the jump here, mark the touch as jumped so we can end
+* it and then continue with the current touch as