Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add alternate raw mouse motion events with windows implementation #10042

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

expikr
Copy link
Contributor

@expikr expikr commented Jun 16, 2024

Proof of concept that closes #7674. Other platforms can be implemented on a rolling basis but shouldn't be a blocker for merge.

@expikr expikr marked this pull request as ready for review June 17, 2024 00:14
@expikr expikr changed the title add SDL_EVENT_RAW_MOTION, and implement for Windows add alternate raw mouse motion events with windows implementation Jun 17, 2024
@expikr expikr force-pushed the patch-1 branch 3 times, most recently from 14d8c11 to 53a374f Compare June 17, 2024 06:00
@slouken
Copy link
Collaborator

slouken commented Jun 17, 2024

So thank you for the proof of concept. I won't accept this as-is, because it would need some style cleanup and additional work, but this clearly demonstrates what you're going for.

Since it's been a while, can you remind me of the use cases for this API?

@sam20908
Copy link
Contributor

I originally wanted this in the context of having raw inputs being mapped to sdl codes across platforms. So raw mouse and keyboard would have their corresponding codes I can reuse across platforms. Maybe if sdl exposes some internal methods for the conversion between win32, evdev, etc tables or such

@expikr
Copy link
Contributor Author

expikr commented Jun 20, 2024

The primary motivation for this is the ability to receive raw device counts while staying in non-relative mode.

An example use-case is a twitch-action MOBA demanding low-latency hardware cursor (ruling out simulating a relativemode cursor) where the cursor is constrained inside the window client area, and you want to be able to pan the camera based on mouse displacement by moving the cursor to the edge of the screen (as opposed to the traditional way of moving the camera at a fixed rate when the game detects that the cursor hits the edge of the window)

Also, I'd like to use this PR as an opportunity to properly learn how SDL's is structured (the current impl was intended to be a throwaway sketch anyways), so if you can give me some leads as to which part of the codebase I should first look into to implement such functionality in a non-shoehorned way, it would be greatly speed up my familiarization.

@slouken
Copy link
Collaborator

slouken commented Jun 23, 2024

The primary motivation for this is the ability to receive raw device counts while staying in non-relative mode.

An example use-case is a twitch-action MOBA demanding low-latency hardware cursor (ruling out simulating a relativemode cursor) where the cursor is constrained inside the window client area, and you want to be able to pan the camera based on mouse displacement by moving the cursor to the edge of the screen (as opposed to the traditional way of moving the camera at a fixed rate when the game detects that the cursor hits the edge of the window)

Yup, this makes sense.

Also, I'd like to use this PR as an opportunity to properly learn how SDL's is structured (the current impl was intended to be a throwaway sketch anyways), so if you can give me some leads as to which part of the codebase I should first look into to implement such functionality in a non-shoehorned way, it would be greatly speed up my familiarization.

Sure, I'll add some comments to show what I would change.

include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
include/SDL3/SDL_hints.h Outdated Show resolved Hide resolved
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
src/events/SDL_mouse.c Outdated Show resolved Hide resolved
src/events/SDL_mouse_c.h Outdated Show resolved Hide resolved
src/events/SDL_events.c Outdated Show resolved Hide resolved
@expikr expikr force-pushed the patch-1 branch 2 times, most recently from 2eb9cd8 to e8c43ff Compare June 24, 2024 08:49
@slouken
Copy link
Collaborator

slouken commented Jun 24, 2024

Yes, this looks good. If this functionality is merged, would you still need SDL_HINT_MOUSE_RELATIVE_CURSOR_VISIBLE and SDL_HINT_MOUSE_RELATIVE_CLIP_INTERVAL?

@expikr
Copy link
Contributor Author

expikr commented Jun 24, 2024

For SDL2, both are still required as workaround since it won’t have the new rawinput subsystem.

For SDL3, I’d still like to have the option of manual cursor visibility management in cases where I sometime still need to fall back to using relative mode -- such as dynamically varying SDL’s mouse scaling based on asynchronous state -- implementing from pure raw motion channel would entail rolling my own signalling and callback logic, when SDL already provides it for free. But strictly speaking yes the raw motion channel provides enough of an escape hatch for me to live without that knob.

Manual cursor confinement management is also a nice-to-have QoL, though working around its absence isn’t as much of an inconvenience as lacking manual cursor visibility management. Though the other way of using that hint -- making the refresh more frequent -- may be a use case that others might find useful, such as circumventing interference from other applications.

Beyond that, both of those also have minor value as a debugging knob.

Ultimately though, having the raw motion channel as the ultimate escape hatch makes those two hints less of a pressing need, opening up breathing room to potentially refactor them into a more cohesive form that is supplementary as opposed to a stopgap shoehorn.

I should note that I haven’t directly tested the PR myself, since I haven’t yet figured out how to drop it into Love2D 11.x, so I don’t know if it currently has any unexpected interactions with the relative mode state machine

@slouken
Copy link
Collaborator

slouken commented Jun 24, 2024

I'm inclined to accept this PR. It allows the application more flexibility listening to raw mouse deltas and is the only way for it to get them when SDL has raw input enabled on Windows, because there can be only one raw input listener at a time.

@icculus, thoughts?

@slouken slouken requested a review from icculus June 24, 2024 13:04
@expikr
Copy link
Contributor Author

expikr commented Jun 24, 2024

Also I should probably add a button event counterpart to the raw channel, since mixing WM_MOUSE* button events with an asynchronous thread will result in out-of-sync ordering of button presses and movement. I didn’t add it earlier because I expected to still use relative mode for the main motion, raw movement being supplementary for the occasional interactions, but if the two hints are removed then a raw-channel-only implementation must also read the raw button pressses to get the correct ordering of events.

@icculus
Copy link
Collaborator

icculus commented Jun 24, 2024

Exposing the raw data like this is a good idea, I think. Is this meant to be Windows only, or can we wire this up for things like Xinput2 on X11, etc?

(If so, we can do that in later commits, and not hold up this PR.)

@slouken
Copy link
Collaborator

slouken commented Jun 24, 2024

Exposing the raw data like this is a good idea, I think. Is this meant to be Windows only, or can we wire this up for things like Xinput2 on X11, etc?

We can do this for other platforms too.

@slouken
Copy link
Collaborator

slouken commented Jun 24, 2024

Also I should probably add a button event counterpart to the raw channel, since mixing WM_MOUSE* button events with an asynchronous thread will result in out-of-sync ordering of button presses and movement. I didn’t add it earlier because I expected to still use relative mode for the main motion, raw movement being supplementary for the occasional interactions, but if the two hints are removed then a raw-channel-only implementation must also read the raw button pressses to get the correct ordering of events.

Good point.

@expikr expikr force-pushed the patch-1 branch 3 times, most recently from e939a7b to 944e1c9 Compare June 25, 2024 03:16
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
src/events/SDL_events.c Outdated Show resolved Hide resolved
src/events/SDL_events.c Outdated Show resolved Hide resolved
@expikr expikr force-pushed the patch-1 branch 3 times, most recently from 06bdb11 to d453b22 Compare June 25, 2024 05:18
@expikr
Copy link
Contributor Author

expikr commented Jun 27, 2024

I've integrated wheel support into the raw motion struct via a different event type tag, and renamed the struct as axis rather than motion events. Also renamed the union names to use m- prefix for raw mouse data to conform to other device types. This set of changes are packaged as one commit on top of the last approved state.

@Kontrabant
Copy link
Contributor

Kontrabant commented Jun 28, 2024

The raw wheel event needs some reference point for client applications to interpret it, as different platforms can report this value in many ways. Win32 and Wayland with high-resolution scroll wheel support use a system where 120 units equals 1 whole scroll wheel tick, Wayland without high-resolution scroll support uses 10 units per tick, on X11 1 unit equals 1 tick, and Cocoa sends wheel deltas as floats.

Two solutions are:

  • Use a float instead of an integer value
  • Specify that the integer delta is a numerator that needs to be divided by some denominator to interpret the fractional delta (120 seems to be commonly used)

@expikr
Copy link
Contributor Author

expikr commented Jul 23, 2024

The idea of the raw events is simply to forward the raw data without any opinionated choice of unifying platform behaviour---for that we already have the regular mouse events. The integers are only meant hold raw bits as-is (GCMouseInput reports mouse movement in floats as well, but it should still be packed into int32), it is up to individual developers to interpret based on specific platforms---some platforms would lack this event altogether, since the expectation for this is purely an optional escape hatch that the developer chooses to work with in a supplementary fashion.

Given the above reasoning, it also means that RELATIVE_CURSOR_VISIBLE should probably remain so that devs can have the option to implement the aforementioned usecase in vanilla relative mode as well.

@slouken considering the above raised point about platform-specific bits representation, do you think it would be more appropriate to unilaterally pass data as unsigned integers instead and document that gamedevs are expected reinterpret them based on platform behaviour?

@slouken
Copy link
Collaborator

slouken commented Jul 23, 2024

If we are passing through the native data, then we should probably use floats. The GCMouseInput deltas definitely have sub-pixel precision, and if we want to adhere to your reasoning, we should pass that through as-is.

@expikr expikr requested a review from slouken October 13, 2024 11:46
@expikr
Copy link
Contributor Author

expikr commented Oct 19, 2024

@Kontrabant I have implemented a hybrid approach:

  • raw axis readings are supplied as int
  • associated denominators for each axis are supplied as float

This way, platforms with floating point counts can be represented with the 23 mantissa bits as numerator integer and the denominator being signed negative powers of two, while platforms with integer counts can have the numerator as-is while the denominator is just a float value of 1.0.

Logically it also makes sense in terms of being a raw "axis" event, i.e. hardwares in general report a logical numeric value relative to some arbitrary denomination size, which is then calibrated to report a normalized "scale bar" of real units.

This also potentially opens up for future possibility of per-device resolution normalizations where self-reported mouse DPI can change dynamically.

@slouken Would this fall under the purview of ABI lock-in or just API?

@slouken
Copy link
Collaborator

slouken commented Oct 20, 2024

@slouken Would this fall under the purview of ABI lock-in or just API?

This is new functionality, so it can be added anytime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

separate SDL_RawMotionEvent to receive raw mouse delta
5 participants