xserver: Branch 'master' - 2 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Thu Feb 16 10:27:02 UTC 2023
dix/getevents.c | 120 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 89 insertions(+), 31 deletions(-)
New commits:
commit 0a22502c34f2ea9799a67386498f657d769c7af8
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date: Fri Feb 3 13:40:13 2023 +1000
dix: switch scroll button emulation to multiples of increment
The current algorithm triggers a bug in Xwayland when two devices have
different granularity of scrolling. In Xwayland, the scroll increment is
1 and all physical devices scroll through the same (x)wayland pointer
device.
This may cause events to get lost when changing devices:
- mouse scrolls by full increment, current value is 1.0
last scroll button was sent for valuator value 0.0,
delta is 1.0 and we emulate a button event.
- touchpad scrolls by partial increment, current value is 1.3
last scroll button was sent for valuator value 1.0, delta is 0.3
and no button event is emulated
- mouse scrolls by full increment, current value is -0.7,
last scroll button was sent for valuator value 1.0, delta is -0.7
and no button event is emulated
Thus the wheel event appears to get lost. Xwayland cannot reliably
detect this case because we don't see the physical devices.
We can work around this by instead emulating buttons whenever we cross
a multiple of increment. However, this has a drawback:
high-resolution scroll devices can now trigger a button event storm by
jittering across the multiple of increment. e.g. in the example above
the touchpad moving from 1.3 to 1.0 would cause a click, despite this
being a third of an increment.
Fixes #1339
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
Acked-by: Olivier Fourdan <ofourdan at redhat.com>
diff --git a/dix/getevents.c b/dix/getevents.c
index e9a73981f..d4441224e 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -1500,7 +1500,8 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
* @param type The real type of the event
* @param axis The axis number to generate events for
* @param mask State before this event in absolute coords
- * @param[in,out] last Last scroll state posted in absolute coords (modified
+ * @param last_valuators The device's last valuators value
+ * @param[in,out] lastScroll Last scroll state posted in absolute coords (modified
* in-place)
* @param ms Current time in ms
* @param max_events Max number of events to be generated
@@ -1512,15 +1513,19 @@ emulate_scroll_button_events(InternalEvent *events,
int type,
int axis,
const ValuatorMask *mask,
- ValuatorMask *last, CARD32 ms, int max_events)
+ const ValuatorMask *last_valuators,
+ ValuatorMask *lastScroll, CARD32 ms, int max_events)
{
AxisInfoPtr ax;
double delta;
double incr;
+ int direction = 0; /* -1 for up, 1 for down */
int num_events = 0;
- double total;
int b;
int flags = 0;
+ double last_val, /* abs axis value from previous event */
+ current_val, /* abs axis value for this event */
+ last_scroll_val; /* abs axis value we sent out the last scroll button for */
if (dev->valuator->axes[axis].scroll.type == SCROLL_TYPE_NONE)
return 0;
@@ -1538,25 +1543,73 @@ emulate_scroll_button_events(InternalEvent *events,
if (type != ButtonPress && type != ButtonRelease)
flags |= POINTER_EMULATED;
- if (!valuator_mask_isset(last, axis))
- valuator_mask_set_double(last, axis, 0);
+ if (!valuator_mask_isset(lastScroll, axis))
+ valuator_mask_set_double(lastScroll, axis, 0);
- delta =
- valuator_mask_get_double(mask, axis) - valuator_mask_get_double(last,
- axis);
- total = delta;
- b = (ax->scroll.type == SCROLL_TYPE_VERTICAL) ? 5 : 7;
+ /* The delta between the last value we sent a scroll button event for
+ * and the current event value (which has been applied already in
+ * fill_pointer_events). This tells us the scroll direction. */
+ delta = valuator_mask_get_double(mask, axis) - valuator_mask_get_double(lastScroll, axis);
+ direction = delta * incr > 0 ? 1 : -1;
- if ((incr > 0 && delta < 0) || (incr < 0 && delta > 0))
+ b = (ax->scroll.type == SCROLL_TYPE_VERTICAL) ? 5 : 7;
+ if (direction < 0)
b--; /* we're scrolling up or left → button 4 or 6 */
- while (fabs(delta) >= fabs(incr)) {
- int nev_tmp;
+ /* Note: we emulate scroll on multiples of the increment, regardless of the
+ * current delta, mostly for the benefit of Xwayland which doesn't (cannot)
+ * distinguish between devices, see #1339 and #1414.
+ *
+ * Where a device scrolls a fraction of an increment, a subsequent scroll in
+ * the other direction did not trigger a scroll event. For example, where
+ * the increment is 1.0, the current axis value is 3.0 and a device scrolls
+ * down by 0.7 (a), then up by -1.0 (b), no scroll event was emitted:
+ * -----|------b--|------a--|----
+ * 2.0 3.0 4.0
+ * For both events, the last button was sent at 3.0 and since the delta
+ * from that is never a full increment, no events were generated.
+ * With Xwayland this can happen when we switch between smooth-scroll
+ * devices and discrete devices.
+ *
+ * To avoid this, we now emulate button events whenever we cross a multiple
+ * of the scroll increment. For example, for a scroll increment of 1.0
+ * we expect events at -2.0, -1.0, 0.0, 1.0, 2.0,...
+ *
+ * In the above example, we go from 3.0 to 3.7 (no scroll button event),
+ * then from 3.7 to 2.7 which triggers a scroll button event because we
+ * cross 3.0.
+ *
+ * This trades off one bug for another. Previously, the first scroll button
+ * event after changing direction was always between
+ * [increment, 2 * increment). Above example again: the first event would be
+ * emulated at 2.0 so the full movement before a button event was actually
+ * -1.7.
+ *
+ * Now, the first scroll button event is always between (0.0, increment).
+ * Above example again: the first event would be emulated at 3.0
+ * so the full movement before a button event was actually -0.7.
+ *
+ * This only affects changes of directions. Above example again: the next
+ * button event in-direction would've been emulated at 4.0 so only 0.3
+ * from the current position.
+ */
+ last_val = valuator_mask_get_double(last_valuators, axis);
+ last_scroll_val = valuator_mask_get_double(lastScroll, axis);
+ current_val = valuator_mask_get_double(mask, axis);
+
+ /* We're crossing an increment multiple? */
+ if ((current_val < last_scroll_val && last_scroll_val < last_val) ||
+ (last_val < last_scroll_val && last_scroll_val < current_val)) {
+ last_scroll_val -= direction * incr;
+ }
- if (delta > 0)
- delta -= fabs(incr);
- else if (delta < 0)
- delta += fabs(incr);
+ while (TRUE) {
+ /* The next value we want to send out a button event for */
+ double next_val = last_scroll_val + direction * incr;
+
+ if ((direction > 0 && next_val > current_val) ||
+ (direction < 0 && next_val < current_val))
+ break;
/* fill_pointer_events() generates four events: one normal and one raw
* event for button press and button release.
@@ -1564,6 +1617,8 @@ emulate_scroll_button_events(InternalEvent *events,
* for. In that case, we keep decreasing delta, but skip events.
*/
if (num_events + 4 < max_events) {
+ int nev_tmp;
+
if (type != ButtonRelease) {
nev_tmp = fill_pointer_events(events, dev, ButtonPress, b, ms,
flags, NULL);
@@ -1577,14 +1632,11 @@ emulate_scroll_button_events(InternalEvent *events,
num_events += nev_tmp;
}
}
+ /* send out scroll event */
+ last_scroll_val = next_val;
}
- /* We emulated, update last.scroll */
- if (total != delta) {
- total -= delta;
- valuator_mask_set_double(last, axis,
- valuator_mask_get_double(last, axis) + total);
- }
+ valuator_mask_set_double(lastScroll, axis, last_scroll_val);
return num_events;
}
@@ -1614,6 +1666,7 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
{
CARD32 ms = GetTimeInMillis();
int num_events = 0, nev_tmp;
+ ValuatorMask last_valuators;
ValuatorMask mask;
ValuatorMask scroll;
int i;
@@ -1642,6 +1695,12 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
valuator_mask_copy(&mask, mask_in);
+ /* Back up the current value of last.valuators. fill_pointer_events()
+ * overwrites those but we need them for scroll button emulation */
+ valuator_mask_zero(&last_valuators);
+ for (size_t idx = 0; idx < pDev->last.numValuators; idx++)
+ valuator_mask_set_double(&last_valuators, idx, pDev->last.valuators[idx]);
+
/* Turn a scroll button press into a smooth-scrolling event if
* necessary. This only needs to cater for the XIScrollFlagPreferred
* axis (if more than one scrolling axis is present) */
@@ -1712,7 +1771,7 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type,
nev_tmp =
emulate_scroll_button_events(events, pDev, realtype, i, &scroll,
- pDev->last.scroll, ms,
+ &last_valuators, pDev->last.scroll, ms,
GetMaximumEventsNum() - num_events);
events += nev_tmp;
num_events += nev_tmp;
commit 6f0cd15155c59eb4f882345cbc31bfca5e73f324
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date: Wed Feb 15 10:53:01 2023 +1000
dix: remove pointless "flexible" x/y axis mapping
storeLastValuators() takes the index in the mask for the x and y axis.
Completely pointless because any device that doesn't have x/y on 0 and
1, respectively, is going to break in fun ways anyway. And we only have
two callers two this function, both of which hardcode 0 and 1.
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
diff --git a/dix/getevents.c b/dix/getevents.c
index 32bafe285..e9a73981f 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -1247,19 +1247,18 @@ transformAbsolute(DeviceIntPtr dev, ValuatorMask *mask)
}
static void
-storeLastValuators(DeviceIntPtr dev, ValuatorMask *mask,
- int xaxis, int yaxis, double devx, double devy)
+storeLastValuators(DeviceIntPtr dev, ValuatorMask *mask, double devx, double devy)
{
int i;
/* store desktop-wide in last.valuators */
- if (valuator_mask_isset(mask, xaxis))
+ if (valuator_mask_isset(mask, 0))
dev->last.valuators[0] = devx;
- if (valuator_mask_isset(mask, yaxis))
+ if (valuator_mask_isset(mask, 1))
dev->last.valuators[1] = devy;
for (i = 0; i < valuator_mask_size(mask); i++) {
- if (i == xaxis || i == yaxis)
+ if (i == 0 || i == 1)
continue;
if (valuator_mask_isset(mask, i))
@@ -1448,7 +1447,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
clipValuators(pDev, &mask);
- storeLastValuators(pDev, &mask, 0, 1, devx, devy);
+ storeLastValuators(pDev, &mask, devx, devy);
/* Update the MD's coordinates, which are always in desktop space. */
if (!IsMaster(pDev) && !IsFloating(pDev)) {
@@ -2008,7 +2007,7 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
clipValuators(dev, &mask);
if (emulate_pointer)
- storeLastValuators(dev, &mask, 0, 1, devx, devy);
+ storeLastValuators(dev, &mask, devx, devy);
/* Update the MD's coordinates, which are always in desktop space. */
if (emulate_pointer && !IsMaster(dev) && !IsFloating(dev)) {
More information about the xorg-commit
mailing list