dix: leave last.valuators alone for unknown axes from the lastSlave

Merged Peter Hutterer requested to merge whot/xserver:wip/last-valuator-reset into master

dix: leave last.valuators alone on slave switch

Terms: dev->last.valuator[] is the last value given to us by the driver dev->valuator.axisVal[] is the last value sent to the client dev->last.scroll[] is the abs value of the scroll axis as given by the driver, used for button emulation calculation (and the remainder)

This function updates the device's last.valuator state based on the current master axis state. This way, relative motion continues fluidly when switching between devices. Before mouse 2 comes into effect, it's valuator state is updated to wherever the pointer currently is so the relative event applies on top of that.

This can only work for x/y axes, all other axes aren't guaranteed to have the same meaning and/or may not be present:

  • xtest device: no valuator 2
  • mouse: valuator 2 is horizontal scroll axis
  • tablet: valuator 2 is pressure

Scaling the current value from the pressure range into the range for horizontal scrolling makes no sense. And it causes scroll jumps:

  • scroll down, last.valuator == axisVal == 20
  • xdotool click 1, the XTest device doesn't have that valuator
  • scroll up
  • updateSlaveDeviceCoords reset last.valuator to 0 (axisVal == 20)
  • DeviceClassesChangedEvent includes value 20 for the axis
    • event is processed, last.value changes from 0 to -1
    • axisVal is updated to -1, causing a jump of -21

The same applies when we switch from tablet to mouse wheel if the pressure value is 0 on proximity out (basically guaranteed). So let's drop this code altogether and only leave the scaling for the relative x/y motion.

Reproducer is simple: open a Qt app with a scroll bar, scroll up with mouse. xdotool click 1, scroll down one notch -> jump GTK has code to skip the first scroll event, so it's not affected by this jump.

I admit while writing this patch I could not figure out what the last.valuator scale is supposed to be for on non-x/y axes. it makes very little sense and I think only affects things when we have relative higher axes (except for scrolling those don't really exist). For any absolute axis, the next value overrides whatever we do here anyway.

And it makes even less sense when you consider that axis[2] is horizontal scrolling on your mouse, but pressure on your tablet. Scaling from one to the other is ... awkward. So we can probably skip this scale altogether, but this patch here seems like the minimum impact patch. The updated patch removes the scaling for anything but x/y, turns out the previous patch didn't work correctly where a tablet was involved.

cc @carlosg, @daniels

Edited by Peter Hutterer

Merge request reports