x11: add support for precision/pixel scrolling

[wooosh]

This patch adds support for pixel scrolling for the x11 backend for xinput2.

Description

This patch will automatically detect precision scroll devices using xinput2, allowing for SDL applications to detect scrolling changes smaller than one line, allowing for smooth scrolling on devices with touchpads, trackpoints, and trackballs.

There are a few things about this patch I'm not certain about:

• Should there be a define to enable this? It requires xinput 2.1 (2011) to compile.
• I've only been able to test this on my laptop at the moment, but I should be able to get some help with testing it on other hardware in the next few days.
• Currently the state to track the xinput devices is allocated statically, with a maximum of 8 devices.
  • Should this automatically allocate for the number of devices present? I don't have a good feel for SDL2's stance on allocation.
  • Should this state be stored somewhere else? I couldn't find a better place to put it, as I'm not very familiar with the codebase, but maybe it would be better off in the private x11 data struct or the private mouse data (if there is any for mice, that is).

Existing Issue(s)

When precision scrolling was originally added to SDL, the X11 backend was left out:

#1267
On 2016-06-01 16:12:29 +0000, Martijn Courteaux wrote:

Note that this doesn't fix it for X11. I'm a X11 noob, but I think that SDL doesn't use xinput2, since @r.kreis said that.

[BanchouBoo]

I tested this with my trackball and it worked fine

[sezero]

https://github.com/libsdl-org/SDL/runs/5441478306?check_suite_focus=true#step:6:190

/home/runner/work/SDL/SDL/src/video/x11/SDL_x11xinput2.c:170:22: warning: declaration of ‘i’ shadows a previous local [-Wshadow]
  170 |         int ndevices,i,j,k,dev_i=0;
      |                      ^
/home/runner/work/SDL/SDL/src/video/x11/SDL_x11xinput2.c:138:21: note: shadowed declaration is here
  138 |     int event, err, i;
      |                     ^
/home/runner/work/SDL/SDL/src/video/x11/SDL_x11xinput2.c: In function ‘X11_HandleXinput2Event’:
/home/runner/work/SDL/SDL/src/video/x11/SDL_x11xinput2.c:310:13: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
  310 |             int pointer_emulated = (xev->flags & XIPointerEmulated);
      |             ^~~
cc1: some warnings being treated as errors

[wooosh]

Apologies for the back and forth over the CI compiler warnings, specifically with C89/C90 compliance, for some reason I cannot get cmake to produce warnings or errors for shadowing or mixed code and declarations locally.

[sezero]

Possibly needs autotools and cmake checks for XIScrollClassInfo

[sezero]

Possibly needs autotools and cmake checks for XIScrollClassInfo

Or, since SDL_VIDEO_DRIVER_X11_XINPUT2_SUPPORTS_MULTITOUCH already requires 2.1+, the new code probably can be guarded with that existing macro. Maintainers to decide.

[wooosh]

I added checks for both cmake and autotools, though I'm not sure if I set up the autotools check properly for this.

[sezero]

@icculus or @slouken should review now

[wooosh]

During further testing, I found a bug in my PR where both precise and non-precise scrolling events will still get sent even if a precision scrolling device is detected.

[wooosh]

xinput2 valuators have many pitfalls I was not aware of when I started this pull request, so I’m going to leave this as a draft until I’m more confident that this is free of bugs.

Currently I’m aware of the following issues, though I suspect I will find more:

• Because the scroll valuators are represented as an absolute position, scrolling in another window invalidates the previous values stored in SDL to calculate the relative movement
• Hotplugging scrollable devices does not function properly
• The absolute scroll coordinates will wrap around at a certain point

[icculus]

This is still in progress, so I'm tentatively putting it in the 2.0.22 milestone, but we'll slide it to 2.0.24 if necessary.

[wooosh]

I've fixed all of the issues I am currently aware of, so I'm going to continue to test this over the next few days to try and find any remaining edge cases.

This should support multiple distinct logical pointers, but I'm not certain it works 100% properly as I was unable to set up multiple pointers under FVWM, XFCE, or herbstluftwm. However, given that SDL makes no attempts to support multiple logical cursors (at least as far as I can tell, based on the wiki excerpt below), and my assumption that it's used relatively rarely, I believe this should not be an issue in practice. I suspect any issues caused by multiple logical pointers would be limited to not detecting creation and removal of logical pointers, and jumpy scrolling when both provide scroll events at the same time.

Please note that this ONLY discusses "mice" with the notion of the desktop GUI. You (usually) have one system cursor, and the OS hides the hardware details from you. If you plug in 10 mice, all ten move that one cursor. For many applications and games this is perfect, and this API has served hundreds of SDL programs well since its birth.

Regardless, if it's something deemed worth further investigation, I'm willing to put more effort into attempting to test multiple logical cursors.

[wooosh]

It's worth noting that the first scroll event after any of the following will be discarded:

• The pointer leaves and enters the window again
• A device change event
  • Master device changes - physically plugging or unplugging a device
  • Slave device changes - the user scrolls on one device, and then starts using another (already plugged in) device

This is an known deficiency in XInput2, due to scrolling being represented as absolute coordinates, any event that prevents us from knowing the previous coordinate forces us to eat the next input event.

As far as I can tell, there is zero reliable ways around this, as Chrome, GTK, or QT all share this behavior.

[slouken]

Is this ready for review and testing in 2.26.0?

[slouken]

@wooosh, can you review this and update it for the latest main code?

[wooosh]

Yes, I will update this to the latest code in main and review it.

[slouken]

@wooosh ping? I'd like to get this in for release if it's ready.

[icculus]

I resolved the merge conflicts but have not tested this...at this point, I think we bump to 2.28.0 or SDL3 and merge right away, as it's probably good but way too late for 2.26.0.

[slouken]

3.0 it is. :)

[slouken]

@wooosh, can you update this PR? It would be great to get in for SDL 3.0. Thanks!

[zlago]

tried compiling 55231f4

/src/video/x11/SDL_x11xinput2.c: In function ‘X11_Xinput2Select’:
/src/video/x11/SDL_x11xinput2.c:577:14: error: ‘SDL_VideoData’ has no member named ‘xinput_scrolling’
  577 |     if (!data->xinput_scrolling && !X11_Xinput2IsMultitouchSupported()) {
      |              ^~
/src/video/x11/SDL_x11xinput2.c:585:13: error: ‘SDL_VideoData’ has no member named ‘xinput_scrolling’
  585 |     if (data->xinput_scrolling) {
      |             ^~

this field is only defined if macro SDL_VIDEO_DRIVER_X11_XINPUT2_SUPPORTS_SCROLLINFO is true (src/video/x11/SDL_x11video.h:137)

it seems to work just fine for me when i forcibly compile with -DSDL_VIDEO_DRIVER_X11_XINPUT2_SUPPORTS_SCROLLINFO=1

[icculus]

Let's kick this to 3.4 and I'll get it updated and tested, for real this time.