Re: Dual display in clone mode or extended mode with Weston

[Konstantin Kharlamov]

On 21.06.2018 21:44, Matheus Santana wrote:



On Thu, Jun 21, 2018 at 5:11 AM, Pekka Paalanen > wrote:


On Wed, 13 Jun 2018 17:40:59 +0530
Ashvini Deshmukh mailto:ashvini2...@gmail.com>> wrote:

> Hello All,
> 
> I have read queries for dual display on freedesktop.org 
> 
> I need your help in the same context.
> 
> Currently we are creating one application to support multiple displays with

> Wayland.

Hi,

you are developing an application, ok. I assume that means a Wayland
client specifically.

> We are unaware that one compositor will be sufficient for dual display.

Sorry, I don't understand. Are you asking whether one Wayland
compositor could driver multiple displays? Yes, they can in general.
Capabilities will vary between different compositor implementations.

> We need to know about how virtual framebuffer is created per display.

As an application developer, why would you care about that? That is a
compositor internal implementation detail.

Why virtual? Real outputs do not have virtual framebuffers, they have
real framebuffers as far as the compositor is concerned.

> As DRM supports only one compositor,
> How to display same content on second display OR can we have different 
user
> events on second display monitor.

What do you mean?

If a Wayland compositor supports and has been configured to show the
same content on multiple displays, then it will do that. From a Wayland
client perspective, there is nothing you need to do to have your
window show up on cloned displays compared to a single display case.

By user events, do you mean input events?

It is certainly possible to write a compositor that dedicates one set
of input devices for one display and another set of input devices for
the other display.

Applications are expected to support multiple wl_seat globals (similar
to multi-pointer X in essence). There is nothing else they would need
to specifically support for a compositor that had multiple outputs,
cloned outputs, or divided input devices in any arbitrary way.

I did not understand your requirements well enough to say how well
Weston would work for you.

For example, Weston currently does not support multiple KMS devices,
but it does support multiple displays on a single KMS device. Support
for multiple KMS devices is desired in Weston though, so maybe it will
in the future.


I'm curious about what you specifically mean by KMS device. Each 
graphics card?


I've just tried Weston with two displays and it worked out on the fly 
("extended mode").



Weston's clone mode is currently limited to sharing a CRTC between all
displays, assuming someone reviews the final patch needed to configure
it. Support for this configuration does not seem to be common among PC
graphics hardware, embedded boards may have better chances.


What does CRTC stand for?


In this context, AFAIR, "CRTC" means a device that scans out an image 
from a buffer to a display. The acronym is historical, stands for 
"cathode ray tube controller".

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

Re: [PATCH libinput 2/4] touchpad: reset the wobble detection for non-pointer events

[Konstantin Kharlamov]

On 30.04.2018 08:33, Peter Hutterer wrote:

If we get an event other than a motion event we're not wobbling so we need to
reset and restart.

Signed-off-by: Peter Hutterer 


IMO this shouldn't matter, because if a user managed to consciously 
trigger an event other than movement, it would definitely take time more 
than the threshold in milliseconds which is used for detection.


On the other hand I imagine a broken touchpad could send a spurious 
event which would break a detection possibility because we reset.



---
  src/evdev-mt-touchpad.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index dc2ed8dc..704d238a 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -155,9 +155,14 @@ tp_detect_wobbling(struct tp_dispatch *tp,
int dx, dy;
uint64_t dtime;
  
-	if (!(tp->queued & TOUCHPAD_EVENT_MOTION) || tp->hysteresis.enabled)

+   if (tp->hysteresis.enabled)
return;
  
+	if (!(tp->queued & TOUCHPAD_EVENT_MOTION)) {

+   t->hysteresis.x_motion_history = 0;
+   return;
+   }
+
if (t->last_point.x == 0) { /* first invocation */
dx = 0;
dy = 0;


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

Re: [PATCH v4 libinput 3/3] touchpad: add wobbling detection

[Konstantin Kharlamov]

Looks fine to me. The series is

Reviewed-by: Konstantin Kharlamov <hi-an...@yandex.ru>

On 01.03.2018 09:39, Peter Hutterer wrote:

From: Konstantin Kharlamov <hi-an...@yandex.ru>

The details are explained in comment in the code. That aside, I shall
mention the check is so light, that it shouldn't influence CPU
performance even a bit, and can blindly be kept always enabled.

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

v2: rename the function, change comment style, and add calculation of
x_diff (the one that was used is an absolute coordinate).

FWIW, the algorithm don't care that now added "prev_x" is unintialized,
because that happening means the time threshold won't get satisfied
either. It's not like I can't default-init it — it's just that asking
oneself a question "what default value should it have" results in "none".

v3:
* style fixes (empty line after a block, declaration at top)
* s/x_diff/dx
* s/x_motion_in_threashold/x_motion_history
* compare with r_l_r instead of &
* store whole point instead of just x
* move x_motion_history and prev_coords into tp_touch
* move everything around tp_detect_wobbling() call into the function,
use goto for that purpose.
* per request: add a comparison for when previous coordinates don't
actually have the prev. coordinates yet.
* increased timeout from 20ms to 40ms just in case — it's still out of
human ability anyway, and in fact can be increased even further
* ignore Y-only changes, again just in case — it could happen that Y and
X events would be sent separately, and break the heuristic.

v4:
* some more style fixes

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
Changes to v3:
- more style fixes
- ajudsted for patch 1/3

  src/evdev-mt-touchpad.c | 58 -
  src/evdev-mt-touchpad.h |  2 ++
  2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 0c3e6a2b..cec4ba34 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -135,6 +135,61 @@ tp_motion_history_push(struct tp_touch *t)
t->history.index = motion_index;
  }
  
+/* Idea: if we got a tuple of *very* quick moves like {Left, Right,

+ * Left}, or {Right, Left, Right}, it means touchpad jitters since no
+ * human can move like that within thresholds.
+ *
+ * We encode left moves as zeroes, and right as ones. We also drop
+ * the array to all zeroes when contraints are not satisfied. Then we
+ * search for the pattern {1,0,1}. It can't match {Left, Right, Left},
+ * but it does match {Left, Right, Left, Right}, so it's okay.
+ *
+ * This only looks at x changes, y changes are ignored.
+ */
+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp,
+  struct tp_touch *t,
+  uint64_t time)
+{
+   int dx, dy;
+   uint64_t dtime;
+
+   if (!(tp->queued & TOUCHPAD_EVENT_MOTION) || tp->hysteresis.enabled)
+   return;
+
+   if (t->last_point.x == 0) { /* first invocation */
+   dx = 0;
+   dy = 0;
+   } else {
+   dx = t->last_point.x - t->point.x;
+   dy = t->last_point.y - t->point.y;
+   }
+
+   dtime = time - tp->hysteresis.last_motion_time;
+
+   tp->hysteresis.last_motion_time = time;
+   t->last_point = t->point;
+
+   if (dx == 0 && dy != 0) /* ignore y-only changes */
+   return;
+
+   if (dtime > ms2us(40)) {
+   t->hysteresis.x_motion_history = 0;
+   return;
+   }
+
+   t->hysteresis.x_motion_history <<= 1;
+   if (dx > 0) { /* right move */
+   static const char r_l_r = 0x5; /* {Right, Left, Right} */
+
+   t->hysteresis.x_motion_history |= 0x1;
+   if (t->hysteresis.x_motion_history == r_l_r) {
+   tp->hysteresis.enabled = true;
+   evdev_log_debug(tp->device, "hysteresis enabled\n");
+   }
+   }
+}
+
  static inline void
  tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -265,6 +320,7 @@ tp_new_touch(struct tp_dispatch *tp, struct tp_touch *t, 
uint64_t time)
t->time = time;
t->speed.last_speed = 0;
t->speed.exceeded_count = 0;
+   t->hysteresis.x_motion_history = 0;
tp->queued |= TOUCHPAD_EVENT_MOTION;
  }
  
@@ -1443,7 +1499,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
  
  		tp_thumb_detect(tp, t, time);

tp_palm_detect(tp, t, time);
-
+   tp_detect_wobbling(tp, t, time);
tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
  
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-tou

[PATCH libinput] indentation: add .dir-locals.el for emacs

[Konstantin Kharlamov]

It's pretty basic as compared to e.g. one of Mesa, but I don't see what
else could be needed, and if anything, it can be added later.

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
---
 .dir-locals.el | 4 
 1 file changed, 4 insertions(+)
 create mode 100644 .dir-locals.el

diff --git a/.dir-locals.el b/.dir-locals.el
new file mode 100644
index ..4ef2d135
--- /dev/null
+++ b/.dir-locals.el
@@ -0,0 +1,4 @@
+((nil . ((indent-tabs-mode . t)
+ (tab-width . 8)
+ (fill-column . 80)))
+ (c-mode . ((c-basic-offset . 8
-- 
2.15.1

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

Re: [PATCH v2] touchpad: add wobbling detection

[Konstantin Kharlamov]

On 21.02.2018 03:33, Peter Hutterer wrote:

It should work like it previously did; what bothers me however — shouldn't
that be specific to a touch, but not touchpad? Couldn't that get triggered
e.g. by 2-finger scrolling? (whatever that is, I dunno, touching with 2
finger doesn't scroll for me — I do scroll instead using the right edge of
the touchpad).


if you make it per-touch then yes, it can get triggered by 2-finger
scrolling but it'll work per-touch so either touch has to wobble to disable
the hysteresis. we can assume that if one touch wobbles, all of them will.

2-finger scrolling can be enabled with the
libinput_device_config_set_scroll_method() call, or libinput debug-events
--set-scroll-method=twofinger, you should give it a try. edge scrolling is
going the way of the dodo.


Thanks for your comments! Enabling 2-finger scroll did end up in crash 
for me previously, but it's fixed in git already. I did enable it with


	$ xinput set-prop "SynPS/2 Synaptics TouchPad" "libinput Scroll Method 
Enabled" 1 0 0


Well, can't say I'm a fan. It's possible I haven't got used to it yet, 
but for me it's not very comfortable because when I'm trying to scroll 
with 2 fingers, the cursor also starts moving at the beginning, so if 
e.g. I'm trying to scroll something small, or if my cursor is simply at 
the top of a scrollable widget — it can quit out of scrollable zone. And 
even when it starts working, it's sometimes stops scrolling and cursor 
starts moving…


I dunno, it might have something to do with my touchpad being "wobbly"… 
Either way, side scrolling definitely have no such point of failure, 
it's always reliable.

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

[PATCH 2/2 v3 libinput] touchpad: add wobbling detection

[Konstantin Kharlamov]

The details are explained in comment in the code. That aside, I shall
mention the check is so light, that it shouldn't influence CPU
performance even a bit, and can blindly be kept always enabled.

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

v2: rename the function, change comment style, and add calculation of
x_diff (the one that was used is an absolute coordinate).

FWIW, the algorithm don't care that now added "prev_x" is unintialized,
because that happening means the time threshold won't get satisfied
either. It's not like I can't default-init it — it's just that asking
oneself a question "what default value should it have" results in "none".

v3:
* style fixes (empty line after a block, declaration at top)
* s/x_diff/dx
* s/x_motion_in_threashold/x_motion_history
* compare with r_l_r instead of &
* store whole point instead of just x
* move x_motion_history and prev_coords into tp_touch
* move everything around tp_detect_wobbling() call into the function,
use goto for that purpose.
* per request: add a comparison for when previous coordinates don't
actually have the prev. coordinates yet.
* increased timeout from 20ms to 40ms just in case — it's still out of
human ability anyway, and in fact can be increased even further
* ignore Y-only changes, again just in case — it could happen that Y and
X events would be sent separately, and break the heuristic.

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
---
 src/evdev-mt-touchpad.c | 49 -
 src/evdev-mt-touchpad.h |  2 ++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index ead76456..e32597d4 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -131,6 +131,52 @@ tp_motion_history_push(struct tp_touch *t)
t->history.index = motion_index;
 }
 
+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp,
+  struct tp_touch *t,
+  uint64_t time)
+{
+   int dx = (!t->prev_coords.x)? 0 /* first invocation */
+   : t->prev_coords.x - t->point.x;
+
+   if (!(tp->queued & TOUCHPAD_EVENT_MOTION) || tp->hysteresis.enabled)
+   return;
+
+   /* Idea: if we got a tuple of *very* quick moves like {Left, Right,
+* Left}, or {Right, Left, Right}, it means touchpad jitters since no
+* human supposed to be able to move like that within thresholds
+*
+* Algo: we encode left moves as zeroes, and right as ones. We also drop
+* the array to all zeroes when contraints are not satisfied. Then we
+* search for the pattern {1,0,1}. It can't match {Left, Right, Left},
+* but it does match {Left, Right, Left, Right}, so it's okay.
+*/
+   if (!dx && t->prev_coords.y != t->point.y) { /* ignore Y-only changes */
+   t->prev_coords.y = t->point.y;
+   return;
+   }
+
+   if (time - tp->hysteresis.last_motion_time > ms2us(40) || dx == 0) {
+   t->x_motion_history = 0;
+   goto out;
+   }
+
+   t->x_motion_history <<= 1;
+   if (dx > 0) { /* right move */
+   static const char r_l_r = 5; /* {Right, Left, Right} */
+
+   t->x_motion_history |= 1;
+   if (t->x_motion_history == r_l_r) {
+   tp->hysteresis.enabled = true;
+   evdev_log_debug(tp->device, "hysteresis enabled\n");
+   }
+   }
+
+out:
+   tp->hysteresis.last_motion_time = time;
+   t->prev_coords = t->point;
+}
+
 static inline void
 tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -260,6 +306,7 @@ tp_new_touch(struct tp_dispatch *tp, struct tp_touch *t, 
uint64_t time)
t->time = time;
t->speed.last_speed = 0;
t->speed.exceeded_count = 0;
+   t->x_motion_history = 0;
tp->queued |= TOUCHPAD_EVENT_MOTION;
 }
 
@@ -1404,7 +1451,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
 
tp_thumb_detect(tp, t, time);
tp_palm_detect(tp, t, time);
-
+   tp_detect_wobbling(tp, t, time);
tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
 
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..48161efb 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -151,6 +151,8 @@ struct tp_touch {
int pressure;
bool is_tool_palm; /* MT_TOOL_PALM */
int major, minor;
+   char x_motion_history;
+   struct device_coords prev_coords;
 
bool was_down; /* if distance == 0, false for pure hovering
  touches */
-- 
2.15.1

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

Re: [PATCH v2] touchpad: add wobbling detection

[Konstantin Kharlamov]

On 20.02.2018 17:31, Konstantin Kharlamov wrote:

On 20.02.2018 13:44, Konstantin Kharlamov wrote:

On 20.02.2018 09:34, Peter Hutterer wrote:

On Sun, Feb 18, 2018 at 11:14:55PM +0300, Konstantin Kharlamov wrote:

+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp, int x_diff, uint64_t time)
+{
+    if (tp->hysteresis.enabled)
+    return;
+
+    /* Idea: if we got a tuple of *very* quick moves like {Left, 
Right, Left}, or
+ * {Right, Left, Right}, it means touchpad jitters since no 
human supposed to

+ * be able to move like that within thresholds
+ *
+ * Algo: we encode left moves as zeroes, and right as ones. We 
also drop the
+ * array to all zeroes when contraints are not satisfied. Then 
we search for
+ * the pattern {1,0,1}. It can't match {Left, Right, Left}, but 
it does match

+ * {Left, Right, Left, Right}, so it's okay.
+ */


the interesting bit here will be whether there are touchpads that 
wobble but
with more than one event per direction. I suspect there are but this 
should
be simple enough that it catches the first RLR movement (bound to 
happen at

some point) and enable it then. The main question is whether there is a
noticeable period of wobbling until this happens.
I honestly don't know, we won't know until ppl start shouting at us...


My touchpad definitely does; however it does so many events for every 
touch that I feel like it produces every possible combination within a 
second or two. The more that the number for the three slots is 2³ = 8, 
which isn't much.


+    if (time - tp->hysteresis.last_motion_time > ms2us(20) || 
x_diff == 0) {

+    tp->hysteresis.x_motion_in_threshold = 0;
+    return;
+    }


empty line here please.


+    tp->hysteresis.x_motion_in_threshold <<= 1;
+    if (x_diff > 0) { /* right move */


s/x_diff/dx/, we use that everywhere else for deltas.


+    tp->hysteresis.x_motion_in_threshold |= 1;
+    static const char r_l_r = 5; /* {Right, Left, Right} */


declarations at the top of the block please, separated by an empty 
line. See

CODING_STYLE in the repository.


+    if (tp->hysteresis.x_motion_in_threshold & r_l_r) {


this one will trigger on the first right movement, you need
   if ((tp->hysteresis.x_motion_in_threshold & r_l_r) == r_l_r)

With that change, it seems to work correctly on my non-wobbling touchpad
here. Does it still work for you when you add this?


Thank you, nice catch!


+    tp->hysteresis.enabled = true;
+    evdev_log_debug(tp->device, "hysteresis enabled\n");
+    }
+    }
+}
+
  static inline void
  tp_motion_hysteresis(struct tp_dispatch *tp,
   struct tp_touch *t)
@@ -1405,6 +1435,8 @@ tp_process_state(struct tp_dispatch *tp, 
uint64_t time)

  tp_thumb_detect(tp, t, time);
  tp_palm_detect(tp, t, time);
+    tp_detect_wobbling(tp, tp->hysteresis.prev_x - t->point.x, 
time);

+    tp->hysteresis.prev_x = t->point.x;


the dx calculation should happen within tp_detect_wobbling, just pass 
the
touch in and do the subtraction there. Plus that way you can do a 
simple:

 if (tp->prev.x == 0) dx = 0;
which makes things more obvious.


Well, not really can, the comparison for zero here would check 
coordinate of previous x, whereas I need to check whether the delta 
between prev. and curr. positions is zero, e.g. if a finger is still 
and FW filters.



  tp_motion_hysteresis(tp, t);
  tp_motion_history_push(t);
@@ -2918,6 +2950,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)
  tp->hysteresis.margin.x = res_x/2;
  tp->hysteresis.margin.y = res_y/2;
  tp->hysteresis.enabled = false;
+    tp->hysteresis.x_motion_in_threshold = 0;


'threshold' usually refers to a distance in libinput, here it's more a
timeout. But calling it x_motion_history would be best here, IMO.


  }
  static void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..398a18ed 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -276,6 +276,8 @@ struct tp_dispatch {
  struct device_coords margin;
  unsigned int other_event_count;
  uint64_t last_motion_time;
+    char x_motion_in_threshold;
+    int prev_x;


both of these need to be in the struct tp_touch, otherwise 2-finger
scrolling may enable the hysteresis when the fingers move 
independently from

each other but you're adding them to the same bitmask.


Thanks for you comments!

I tried addressing them, and found that the result of the time 
calculation "time - tp->hysteresis.last_motion_time" whilst my finger 
is down is constantly increasing, i.e. like "20, 30, 50, 100, 120…". 
It looks as if "tp->hysteresis.last_motion_time" is not the last 
motion time, but the last finger-down time.


Do you happen to know anything about that?


Although I did 

Re: [PATCH v2] touchpad: add wobbling detection

[Konstantin Kharlamov]

On 20.02.2018 13:44, Konstantin Kharlamov wrote:

On 20.02.2018 09:34, Peter Hutterer wrote:

On Sun, Feb 18, 2018 at 11:14:55PM +0300, Konstantin Kharlamov wrote:

+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp, int x_diff, uint64_t time)
+{
+    if (tp->hysteresis.enabled)
+    return;
+
+    /* Idea: if we got a tuple of *very* quick moves like {Left, 
Right, Left}, or
+ * {Right, Left, Right}, it means touchpad jitters since no 
human supposed to

+ * be able to move like that within thresholds
+ *
+ * Algo: we encode left moves as zeroes, and right as ones. We 
also drop the
+ * array to all zeroes when contraints are not satisfied. Then 
we search for
+ * the pattern {1,0,1}. It can't match {Left, Right, Left}, but 
it does match

+ * {Left, Right, Left, Right}, so it's okay.
+ */


the interesting bit here will be whether there are touchpads that 
wobble but
with more than one event per direction. I suspect there are but this 
should
be simple enough that it catches the first RLR movement (bound to 
happen at

some point) and enable it then. The main question is whether there is a
noticeable period of wobbling until this happens.
I honestly don't know, we won't know until ppl start shouting at us...


My touchpad definitely does; however it does so many events for every 
touch that I feel like it produces every possible combination within a 
second or two. The more that the number for the three slots is 2³ = 8, 
which isn't much.


+    if (time - tp->hysteresis.last_motion_time > ms2us(20) || x_diff 
== 0) {

+    tp->hysteresis.x_motion_in_threshold = 0;
+    return;
+    }


empty line here please.


+    tp->hysteresis.x_motion_in_threshold <<= 1;
+    if (x_diff > 0) { /* right move */


s/x_diff/dx/, we use that everywhere else for deltas.


+    tp->hysteresis.x_motion_in_threshold |= 1;
+    static const char r_l_r = 5; /* {Right, Left, Right} */


declarations at the top of the block please, separated by an empty 
line. See

CODING_STYLE in the repository.


+    if (tp->hysteresis.x_motion_in_threshold & r_l_r) {


this one will trigger on the first right movement, you need
   if ((tp->hysteresis.x_motion_in_threshold & r_l_r) == r_l_r)

With that change, it seems to work correctly on my non-wobbling touchpad
here. Does it still work for you when you add this?


Thank you, nice catch!


+    tp->hysteresis.enabled = true;
+    evdev_log_debug(tp->device, "hysteresis enabled\n");
+    }
+    }
+}
+
  static inline void
  tp_motion_hysteresis(struct tp_dispatch *tp,
   struct tp_touch *t)
@@ -1405,6 +1435,8 @@ tp_process_state(struct tp_dispatch *tp, 
uint64_t time)

  tp_thumb_detect(tp, t, time);
  tp_palm_detect(tp, t, time);
+    tp_detect_wobbling(tp, tp->hysteresis.prev_x - t->point.x, 
time);

+    tp->hysteresis.prev_x = t->point.x;


the dx calculation should happen within tp_detect_wobbling, just pass the
touch in and do the subtraction there. Plus that way you can do a simple:
 if (tp->prev.x == 0) dx = 0;
which makes things more obvious.


Well, not really can, the comparison for zero here would check 
coordinate of previous x, whereas I need to check whether the delta 
between prev. and curr. positions is zero, e.g. if a finger is still and 
FW filters.



  tp_motion_hysteresis(tp, t);
  tp_motion_history_push(t);
@@ -2918,6 +2950,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)
  tp->hysteresis.margin.x = res_x/2;
  tp->hysteresis.margin.y = res_y/2;
  tp->hysteresis.enabled = false;
+    tp->hysteresis.x_motion_in_threshold = 0;


'threshold' usually refers to a distance in libinput, here it's more a
timeout. But calling it x_motion_history would be best here, IMO.


  }
  static void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..398a18ed 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -276,6 +276,8 @@ struct tp_dispatch {
  struct device_coords margin;
  unsigned int other_event_count;
  uint64_t last_motion_time;
+    char x_motion_in_threshold;
+    int prev_x;


both of these need to be in the struct tp_touch, otherwise 2-finger
scrolling may enable the hysteresis when the fingers move 
independently from

each other but you're adding them to the same bitmask.


Thanks for you comments!

I tried addressing them, and found that the result of the time 
calculation "time - tp->hysteresis.last_motion_time" whilst my finger is 
down is constantly increasing, i.e. like "20, 30, 50, 100, 120…". It 
looks as if "tp->hysteresis.last_motion_time" is not the last motion 
time, but the last finger-down time.


Do you happen to know anything about that?


Although I did find what's the matter. Turns out, the las

Re: [PATCH v2] touchpad: add wobbling detection

[Konstantin Kharlamov]

On 20.02.2018 09:34, Peter Hutterer wrote:

On Sun, Feb 18, 2018 at 11:14:55PM +0300, Konstantin Kharlamov wrote:

+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp, int x_diff, uint64_t time)
+{
+   if (tp->hysteresis.enabled)
+   return;
+
+   /* Idea: if we got a tuple of *very* quick moves like {Left, Right, 
Left}, or
+* {Right, Left, Right}, it means touchpad jitters since no human 
supposed to
+* be able to move like that within thresholds
+*
+* Algo: we encode left moves as zeroes, and right as ones. We also 
drop the
+* array to all zeroes when contraints are not satisfied. Then we 
search for
+* the pattern {1,0,1}. It can't match {Left, Right, Left}, but it does 
match
+* {Left, Right, Left, Right}, so it's okay.
+*/


the interesting bit here will be whether there are touchpads that wobble but
with more than one event per direction. I suspect there are but this should
be simple enough that it catches the first RLR movement (bound to happen at
some point) and enable it then. The main question is whether there is a
noticeable period of wobbling until this happens.
I honestly don't know, we won't know until ppl start shouting at us...


My touchpad definitely does; however it does so many events for every 
touch that I feel like it produces every possible combination within a 
second or two. The more that the number for the three slots is 2³ = 8, 
which isn't much.



+   if (time - tp->hysteresis.last_motion_time > ms2us(20) || x_diff == 0) {
+   tp->hysteresis.x_motion_in_threshold = 0;
+   return;
+   }


empty line here please.


+   tp->hysteresis.x_motion_in_threshold <<= 1;
+   if (x_diff > 0) { /* right move */


s/x_diff/dx/, we use that everywhere else for deltas.


+   tp->hysteresis.x_motion_in_threshold |= 1;
+   static const char r_l_r = 5; /* {Right, Left, Right} */


declarations at the top of the block please, separated by an empty line. See
CODING_STYLE in the repository.


+   if (tp->hysteresis.x_motion_in_threshold & r_l_r) {


this one will trigger on the first right movement, you need
   if ((tp->hysteresis.x_motion_in_threshold & r_l_r) == r_l_r)

With that change, it seems to work correctly on my non-wobbling touchpad
here. Does it still work for you when you add this?


Thank you, nice catch!


+   tp->hysteresis.enabled = true;
+   evdev_log_debug(tp->device, "hysteresis enabled\n");
+   }
+   }
+}
+
  static inline void
  tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -1405,6 +1435,8 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
tp_thumb_detect(tp, t, time);
tp_palm_detect(tp, t, time);
  
+		tp_detect_wobbling(tp, tp->hysteresis.prev_x - t->point.x, time);

+   tp->hysteresis.prev_x = t->point.x;


the dx calculation should happen within tp_detect_wobbling, just pass the
touch in and do the subtraction there. Plus that way you can do a simple:
 if (tp->prev.x == 0) dx = 0;
which makes things more obvious.


Well, not really can, the comparison for zero here would check 
coordinate of previous x, whereas I need to check whether the delta 
between prev. and curr. positions is zero, e.g. if a finger is still and 
FW filters.



tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
  
@@ -2918,6 +2950,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)

tp->hysteresis.margin.x = res_x/2;
tp->hysteresis.margin.y = res_y/2;
tp->hysteresis.enabled = false;
+   tp->hysteresis.x_motion_in_threshold = 0;


'threshold' usually refers to a distance in libinput, here it's more a
timeout. But calling it x_motion_history would be best here, IMO.


  }
  
  static void

diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..398a18ed 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -276,6 +276,8 @@ struct tp_dispatch {
struct device_coords margin;
unsigned int other_event_count;
uint64_t last_motion_time;
+   char x_motion_in_threshold;
+   int prev_x;


both of these need to be in the struct tp_touch, otherwise 2-finger
scrolling may enable the hysteresis when the fingers move independently from
each other but you're adding them to the same bitmask.


Thanks for you comments!

I tried addressing them, and found that the result of the time 
calculation "time - tp->hysteresis.last_motion_time" whilst my finger is 
down is constantly increasing, i.e. like "20, 30, 50, 100, 120…". It 
looks as if "tp->hysteresis.last_motion_time" is not the last motion 
time, but the last

Re: [PATCH 0/2 libinput] [RFC] Add touchpad wobbliness detection

[Konstantin Kharlamov]

FWIW, given peoples complained about hysteresis algo latency, I'm also 
poking around with alternative hysteresis algos. One possible idea that 
came to my mind is to use the detection code posted here to figure the 
maximum wobbliness length (along each axis separately), then simply 
ignore movements of that length. No idea how's that gonna work, will 
check on holidays.


On 19.02.2018 10:59, Peter Hutterer wrote:

On Sun, Feb 18, 2018 at 01:09:22PM +0300, Konstantin Kharlamov wrote:

For the purposes it seems to work fine — it's marked RFC because I see a
small oddness, and I think it's better to ask someone more
acknowledgable in libinput codebase.


thanks, much appreciated. I was hoping I get to this today but didn't, I'll
try for it tomorrow. meanwhile:



For some reason every time I run

libinput debug-events --verbose | grep bled

I see a message about hysteresis being enabled, which then never appears
until I stop the command, and re-run it again. It looks like every time
the command ran, touchpad being reinitialized, making the function
tp_init_hysteresis() to ran again.
  
it's expected, yes. debug-events instantiates a new libinput context (you

can't get to the one in the compositor) and your code, from a quick glance,
only runs until enabled. so every time you start the tool, you start from
zero.

Cheers,
Peter


If this is an intentional behavior, then I have no other question, and
the patchset should be fine. Though it would be nice if someone with a
"non-wobbly" touchpad could test that it does not enable hysteresis for
them (it all should be good, but testing never hurts :)


P.S.: I don't have commit rights.

Konstantin Kharlamov (2):
   touchpad: remove the code for disabling hysteresis
   touchpad: add wobbling detection

  src/evdev-mt-touchpad.c | 42 --
  src/evdev-mt-touchpad.h |  1 +
  2 files changed, 29 insertions(+), 14 deletions(-)

--
2.15.1

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


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


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

[PATCH v2] touchpad: add wobbling detection

[Konstantin Kharlamov]

The details are explained in comment in the code. That aside, I shall
mention the check is so light, that it shouldn't influence CPU
performance even a bit, and can blindly be kept always enabled.

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

v2: rename the function, change comment style, and add calculation of
x_diff (the one that was used is an absolute coordinate).

FWIW, the algorithm don't care that now added "prev_x" is unintialized,
because that happening means the time threshold won't get satisfied
either. It's not like I can't default-init it — it's just that asking
oneself a question "what default value should it have" results in "none".

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
---
 src/evdev-mt-touchpad.c | 33 +
 src/evdev-mt-touchpad.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index ead76456..80f56072 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -131,6 +131,36 @@ tp_motion_history_push(struct tp_touch *t)
t->history.index = motion_index;
 }
 
+static inline void
+tp_detect_wobbling(struct tp_dispatch *tp, int x_diff, uint64_t time)
+{
+   if (tp->hysteresis.enabled)
+   return;
+
+   /* Idea: if we got a tuple of *very* quick moves like {Left, Right, 
Left}, or
+* {Right, Left, Right}, it means touchpad jitters since no human 
supposed to
+* be able to move like that within thresholds
+*
+* Algo: we encode left moves as zeroes, and right as ones. We also 
drop the
+* array to all zeroes when contraints are not satisfied. Then we 
search for
+* the pattern {1,0,1}. It can't match {Left, Right, Left}, but it does 
match
+* {Left, Right, Left, Right}, so it's okay.
+*/
+   if (time - tp->hysteresis.last_motion_time > ms2us(20) || x_diff == 0) {
+   tp->hysteresis.x_motion_in_threshold = 0;
+   return;
+   }
+   tp->hysteresis.x_motion_in_threshold <<= 1;
+   if (x_diff > 0) { /* right move */
+   tp->hysteresis.x_motion_in_threshold |= 1;
+   static const char r_l_r = 5; /* {Right, Left, Right} */
+   if (tp->hysteresis.x_motion_in_threshold & r_l_r) {
+   tp->hysteresis.enabled = true;
+   evdev_log_debug(tp->device, "hysteresis enabled\n");
+   }
+   }
+}
+
 static inline void
 tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -1405,6 +1435,8 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
tp_thumb_detect(tp, t, time);
tp_palm_detect(tp, t, time);
 
+   tp_detect_wobbling(tp, tp->hysteresis.prev_x - t->point.x, 
time);
+   tp->hysteresis.prev_x = t->point.x;
tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
 
@@ -2918,6 +2950,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)
tp->hysteresis.margin.x = res_x/2;
tp->hysteresis.margin.y = res_y/2;
tp->hysteresis.enabled = false;
+   tp->hysteresis.x_motion_in_threshold = 0;
 }
 
 static void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..398a18ed 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -276,6 +276,8 @@ struct tp_dispatch {
struct device_coords margin;
unsigned int other_event_count;
uint64_t last_motion_time;
+   char x_motion_in_threshold;
+   int prev_x;
} hysteresis;
 
struct {
-- 
2.15.1

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

[PATCH 0/2 libinput] [RFC] Add touchpad wobbliness detection

[Konstantin Kharlamov]

For the purposes it seems to work fine — it's marked RFC because I see a
small oddness, and I think it's better to ask someone more
acknowledgable in libinput codebase.

For some reason every time I run 

libinput debug-events --verbose | grep bled

I see a message about hysteresis being enabled, which then never appears
until I stop the command, and re-run it again. It looks like every time
the command ran, touchpad being reinitialized, making the function
tp_init_hysteresis() to ran again.

If this is an intentional behavior, then I have no other question, and
the patchset should be fine. Though it would be nice if someone with a
"non-wobbly" touchpad could test that it does not enable hysteresis for
them (it all should be good, but testing never hurts :)

P.S.: I don't have commit rights.

Konstantin Kharlamov (2):
  touchpad: remove the code for disabling hysteresis
  touchpad: add wobbling detection

 src/evdev-mt-touchpad.c | 42 --
 src/evdev-mt-touchpad.h |  1 +
 2 files changed, 29 insertions(+), 14 deletions(-)

-- 
2.15.1

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

[PATCH 1/2 libinput] touchpad: remove the code for disabling hysteresis

[Konstantin Kharlamov]

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
---
 src/evdev-mt-touchpad.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 3ae1da29..ead76456 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -131,22 +131,6 @@ tp_motion_history_push(struct tp_touch *t)
t->history.index = motion_index;
 }
 
-static inline void
-tp_maybe_disable_hysteresis(struct tp_dispatch *tp, uint64_t time)
-{
-   /* If the finger is down for 80ms without seeing motion events,
-  the firmware filters and we don't need a software hysteresis */
-   if (tp->nfingers_down >= 1 &&
-   time - tp->hysteresis.last_motion_time > ms2us(80)) {
-   tp->hysteresis.enabled = false;
-   evdev_log_debug(tp->device, "hysteresis disabled\n");
-   return;
-   }
-
-   if (tp->queued & TOUCHPAD_EVENT_MOTION)
-   tp->hysteresis.last_motion_time = time;
-}
-
 static inline void
 tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -1544,9 +1528,6 @@ static void
 tp_handle_state(struct tp_dispatch *tp,
uint64_t time)
 {
-   if (tp->hysteresis.enabled)
-   tp_maybe_disable_hysteresis(tp, time);
-
tp_process_state(tp, time);
tp_post_events(tp, time);
tp_post_process_state(tp, time);
@@ -2936,7 +2917,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)
res_y = tp->device->abs.absinfo_y->resolution;
tp->hysteresis.margin.x = res_x/2;
tp->hysteresis.margin.y = res_y/2;
-   tp->hysteresis.enabled = true;
+   tp->hysteresis.enabled = false;
 }
 
 static void
-- 
2.15.1

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

[PATCH 2/2 libinput] touchpad: add wobbling detection

[Konstantin Kharlamov]

The details are explained in comment in the code. That aside, I shall
mention the check is so light, that it shouldn't influence CPU
performance even a bit, and can blindly be kept always enabled.

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

Signed-off-by: Konstantin Kharlamov <hi-an...@yandex.ru>
---
 src/evdev-mt-touchpad.c | 33 +
 src/evdev-mt-touchpad.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index ead76456..e9f8ea4e 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -131,6 +131,37 @@ tp_motion_history_push(struct tp_touch *t)
t->history.index = motion_index;
 }
 
+static inline void
+tp_does_wobble(struct tp_dispatch *tp, int x, uint64_t time)
+{
+   if (tp->hysteresis.enabled)
+   return;
+
+   /* Idea: if we got a tuple of *very* quick moves like {Left, Right, 
Left}, or
+* {Right, Left, Right}, it means touchpad jitters since no human 
supposed to
+* be able to move like that within thresholds
+*
+* Algo: we encode left moves as zeroes, and right as ones. We also 
drop the
+* array to all zeroes when contraints are not satisfied. Then we 
search for
+* the pattern {1,0,1}. It can't match {Left, Right, Left}, but it does 
match
+* {Left, Right, Left, Right}, so it's okay.
+*/
+   if (time - tp->hysteresis.last_motion_time > ms2us(20) || x == 0) {
+   tp->hysteresis.x_motion_in_threshold = 0;
+   return;
+   }
+   tp->hysteresis.x_motion_in_threshold <<= 1;
+   if (x > 0) { // right move
+   tp->hysteresis.x_motion_in_threshold |= 1;
+   static const char r_l_r = 5; // {Right, Left, Right}
+   if (tp->hysteresis.x_motion_in_threshold & r_l_r) {
+   tp->hysteresis.enabled = true;
+   evdev_log_debug(tp->device, "hysteresis enabled\n");
+
+   }
+   }
+}
+
 static inline void
 tp_motion_hysteresis(struct tp_dispatch *tp,
 struct tp_touch *t)
@@ -1405,6 +1436,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
tp_thumb_detect(tp, t, time);
tp_palm_detect(tp, t, time);
 
+   tp_does_wobble(tp, t->point.x, time);
tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
 
@@ -2918,6 +2950,7 @@ tp_init_hysteresis(struct tp_dispatch *tp)
tp->hysteresis.margin.x = res_x/2;
tp->hysteresis.margin.y = res_y/2;
tp->hysteresis.enabled = false;
+   tp->hysteresis.x_motion_in_threshold = 0;
 }
 
 static void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 442f34a3..b738a592 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -276,6 +276,7 @@ struct tp_dispatch {
struct device_coords margin;
unsigned int other_event_count;
uint64_t last_motion_time;
+   char x_motion_in_threshold;
} hysteresis;
 
struct {
-- 
2.15.1

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