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

Pointer confinement support (opt-in) #17169

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

zoltanvb
Copy link
Contributor

@zoltanvb zoltanvb commented Nov 6, 2024

Description

New environment set call to enable a bit more sensible handling of absolute pointing devices (pointer and lightgun).

There was a discussion on Discord a while ago, which concluded that it would be better if the returned coordinates would be clamped to (-0x7fff,0x7fff), and not report (0,0) (or -0x8000, in some cases) as an indication of being outside the active content. The most easily understandable example is RTS games in DOS, where scrolling is done by moving the mouse to the edge of the screen, and that is tricky with absolute pointers. Another improvement may be offscreen detection in lightgun mode.

Since there are some cores that may rely on (0,0), the new behavior was made optional behind a new environment API call, with the actual state being stored in runloop (as opposed to a configuration item).

Remote Retropad was updated with a debug switch that enables turning this option on. Behaves as expected, running the content in a 16:9 monitor and moving the pointer to the black padding bars, cursor stays stuck to the edge if the option is enabled, but jumps back to 0,0 if not enabled.

The change in video_driver.c should cover the issue without having to touch all input drivers. I tested with wayland and udev.

Reviewers

@schellingb

New environment set call to enable a bit more sensible handling of
absolute pointing devices (pointer and lightgun). With the
confinement enabled, pointing devices will not return neither
-0x8000 nor (0,0), which was anyway dependent on the input driver,
instead they will stay at the extreme edge.
@LibretroAdmin LibretroAdmin merged commit 90ee413 into libretro:master Nov 6, 2024
27 checks passed
@schellingb
Copy link
Contributor

This was already merged before I saw it but here's my response.

As a core developer, while this would solve my issues I have with RetroArch, I really don't think this is an issue in libretro that needs to be fixed there. Introducing a new environment call to work around a bug in RetroArch seems like a Band-Aid that only pollutes libretro with unnecessary environment calls.

The buggy behavior of RetroArch that this new environment call fixes is not documented, until now it has not been described by libretro.h. So unless another frontend were to copy the buggy code in question, it very well might not have the same issue.

So, do we know of any other frontend that has copied this buggy behavior of RetroArch? The only frontend I know that supports mouse inputs is RALibretro which does not at all have this buggy behavior. It already always clamps the coordinates reported to the core to -0x7fff,0x7fff and doesn't treat 0,0 as anything but the cursor being perfectly in the screen center.

Since there are some cores that may rely on (0,0), the new behavior was made optional behind a new environment API call, with the actual state being stored in runloop (as opposed to a configuration item).

I have looked through search result of RETRO_DEVICE_POINTER in the libretro organization to try to figure out which cores might be affected by this. Of course this isn't every libretro core in existence but it covers a large amount of them. As far as I can tell no core relies in 0,0 being off screen. A few cores just ignore it but as far as I can tell this is just to work around the bug in RetroArch.

Here's my finding on cores that just ignore 0,0:

Here's two cores that look suspiciously like they would treat 0,0 special but carefully reading through the code reveals it's not really doing that:

While the code parts look different, I'm pretty sure they are based on the same code at some point. They both have this suspicious code part:

   // Handle offscreen by checking corrected x and y values
   if ( gun_x == 0 || gun_y == 0 )

But looking at the calculation of gun_x and gun_y, which is based on a return value of RETRO_DEVICE_POINTER, but rescaled into an unsigned range and never below zero. So instead of ignoring a reported 0,0 it ignores -0x7fff,-0x7fff!

So as far as I can tell the issues are purely in RetroArch and in its many input device drivers. The effort should be spent in unifying the behaviors of both RETRO_DEVICE_POINTER and RETRO_DEVICE_LIGHTGUN across its input device drivers. For example:

  • On Android RETRO_DEVICE_POINTER already seems clamped but buggy? Touching the black area to the left or right gives X -32768 but leaves Y valid. Touching the black area above or below gives Y -32768 but leaves X valid.
  • RETRO_DEVICE_ID_LIGHTGUN_IS_OFFSCREEN is the correct way for a core to check for an offscreen pointer. dinput/winraw/x11 (and others) report anything above +/- 32700 as off screen but not all input drivers follow that.
  • Most input drivers use the function video_driver_translate_coord_viewport which was touched by this PR for mouse/touch/lightgun inputs, but not all.

I think a good idea would be to write down the behavior of each input driver in regards to both RETRO_DEVICE_POINTER and RETRO_DEVICE_LIGHTGUN and go from there.

@JesseTG
Copy link
Contributor

JesseTG commented Nov 7, 2024

I have looked through search result of RETRO_DEVICE_POINTER in the libretro organization to try to figure out which cores might be affected by this. Of course this isn't every libretro core in existence but it covers a large amount of them. As far as I can tell no core relies in 0,0 being off screen. A few cores just ignore it but as far as I can tell this is just to work around the bug in RetroArch.

I use behavior a bit with melonDS DS, but I'm fine with rethinking that assumption. Especially since the screen resolution is unlikely to be large enough for a player to accidentally hit that exact position...and the touch screen isn't likely to even contain this point, anyway.

@schellingb
Copy link
Contributor

@JesseTG Aren't you emulating a touch screen though? Shouldn't an emulated touch screen only think it is touched while the mouse button is down or while a real touch screen is being touched? The issue at hand should only affect mice (and lightguns) that can hover on and off screen. If RetroArch reports a mouse button being held down (or a finger being touched) the pointer is always on screen with coordinates inside the screen. RetroArch doesn't report a mouse or finger being pressed to the core while off screen.

Looking through your code I think this isn't affecting the actual emulation but the rendering of an on-screen cursor, right? I personally think it's fine if the cursor is still drawn while the real (host) mouse cursor is off one side of the screen. It would even still report to mouse clicks while pressed off-screen so this would make it easier to tap on things near the edge, just like it would make RTS style games in a DOS core much more playable.

@JesseTG
Copy link
Contributor

JesseTG commented Nov 7, 2024

@JesseTG Aren't you emulating a touch screen though? Shouldn't an emulated touch screen only think it is touched while the mouse button is down or while a real touch screen is being touched?

Sure...except there are games that expect you to hold down the stylus at the edge of the screen. I've gotten tickets about this before.

Looking through your code I think this isn't affecting the actual emulation but the rendering of an on-screen cursor, right?

Sure, but then how would I tell where the "stylus" is being held?

@schellingb
Copy link
Contributor

Sure, but then how would I tell where the "stylus" is being held?

It sounds like you explicitly want to know whether the mouse is in the black area around the screen in RetroArch. Of course that black area around the screen does not exist if the viewport (windowed or fullscreen) matches the core's video aspect ratio (depending on scaling options in RetroArch).

Maybe for that introducing a new RETRO_DEVICE_ID_MOUSE_IS_OFFSCREEN to match the already existing RETRO_DEVICE_ID_LIGHTGUN_IS_OFFSCREEN could work for that instead of the 0,0 bug/hack. But what should be considered off screen for a mouse or a touch? Being just a single pixel off in the black area? Being a few pixels off? I think there could be a discussion about that which would provide a better user experience.

Can you confirm though that if RetroArch were to stop reporting an off screen mouse position as 0,0 (and clamp to -0x7fff to +0x7fff), the emulation of melonDS DS would not break, it would just continue to draw an on-screen mouse cursor at the edge while the real mouse is off the edge. Also it would report touches at that edge when the mouse is clicked while in the black area around the screen.

@zoltanvb
Copy link
Contributor Author

zoltanvb commented Nov 7, 2024

Aw. Should have remembered to mark it as draft. (Code was/is fine from my part, but there may be some different conclusion than my original solution.)

Introducing a new environment call to work around a bug in RetroArch seems like a Band-Aid that only pollutes libretro with unnecessary environment calls.

Umm... yeah. Not the cleanest approach. But, as you mention, so far these edge cases were undocumented and a bit random at places. Introducing an unconditional change in the frontend behavior can make things better, but also worse, if something was overlooked and some cores will start acting weird. As RetroArch does not have a huge automated test machinery for detecting regressions (only users who can get annoyed), my preference would still be for the API extension. But if the final decision is to revert this PR and just have it as a new default, then so be it.

The effort should be spent in unifying the behaviors of both RETRO_DEVICE_POINTER and RETRO_DEVICE_LIGHTGUN across its input device drivers.

Regarding input drivers, I scanned through them and there are only a few exceptions, Switch does its own thing (I have not looked into that in detail), Vita and WiiU do not seem to have the translation, others will use video_driver_translate_coord_viewport or skip pointer support entirely. Anyway I plan to check this, and also the overlay pointer/lightgun case, which is again a bit different. BTW lightgun explicitly mentions -0x8000 as out-of-bounds, I am not sure that is actually honored on all platforms, but at least that one should be a straightforward fix from API point of view.

I am not familiar with ScummVM, but my guess would be that it would prefer to use the relative mouse device, and only fall back to the absolute pointer when it must, and on WiiU there is no support for relative mouse, while on Switch it looks as if it only refers to physical mouse, touch input is not converted.

@schellingb
Copy link
Contributor

Ok, I'm not too upset if this stays merged. As mentioned, it will solve my issues with RetroArch as a core developer and it is something that can be done today as opposed to doing the investigation which might just take time and lead to nothing. So in the end maybe this is the most realistic solution with how libretro is tied to the reference frontend RetroArch.

Regarding input drivers, I scanned through them and there are only a few exceptions

Did you look at Android? When I tried it (just by logging the data returned for RETRO_DEVICE_POINTER and RETRO_DEVICE_LIGHTGUN from the core), the data of RETRO_DEVICE_POINTER already seems clamped but buggy. Touching the black area to the left or right gives X -32768 but leaves Y valid. Touching the black area above or below gives Y -32768 but leaves X valid.

Another thing that needs to be checked and made sure is that pressing things on the overlay in RetroArch (the on-screen buttons that are enabled by default on mobile phones but can optionally be enabled on any platform) will never be reported to the core. Ideally mis-presses near the overlay should not be reported to the core, unless the overlay is directly over the screen. So testing and making sure the behavior of these scenarios:

  • Clicking/tapping on a overlay button that sits outside the game screen -> Not report to the core
  • Clicking/tapping on a overlay button that sits over the game screen -> Not report to the core
  • Clicking/tapping near a overlay button that sits outside the game screen -> Not report to the core
  • Clicking/tapping near a overlay button that sits over the game screen -> Do report to the core

@LibretroAdmin
Copy link
Contributor

I'll revert it for now so we can explore a solution that both of you can agree with

LibretroAdmin added a commit that referenced this pull request Nov 7, 2024
LibretroAdmin added a commit that referenced this pull request Nov 7, 2024
@JesseTG
Copy link
Contributor

JesseTG commented Nov 7, 2024

It sounds like you explicitly want to know whether the mouse is in the black area around the screen in RetroArch. Of course that black area around the screen does not exist if the viewport (windowed or fullscreen) matches the core's video aspect ratio (depending on scaling options in RetroArch).

Not just whether. Where. I can map such input to the edge of the touch screen without losing information (e.g. direction)

@schellingb
Copy link
Contributor

Now I'm lost 😅

Nothing that is implemented now or in the past, or has been suggested to be implemented, ever gives you the information exactly where off the screen the mouse is. Of course you always know where the mouse is over the core's video rectangle by polling X/Y and checking if those are in a valid range.

If we were to change that the current behavior (which returns 0,0 as soon as the mouse leaves any edge of the core's video rectangle) to return the coordinate clamped -0x7fff to +0x7fff, and add a RETRO_DEVICE_ID_MOUSE_IS_OFFSCREEN a core would know whether the mouse is over the core's video rectangle or not, and if not, off of which edge it is. It still couldn't tell how far off the screen the mouse is.

@zoltanvb
Copy link
Contributor Author

zoltanvb commented Nov 9, 2024

As I understand clamping should make it possible to map the pointer position to the edges, i.e. no contradiction with what is needed for MelonDS DS.

Any recommended content for checking behavior in actual games?

@JesseTG
Copy link
Contributor

JesseTG commented Nov 9, 2024

As I understand clamping should make it possible to map the pointer position to the edges, i.e. no contradiction with what is needed for MelonDS DS.

Any recommended content for checking behavior in actual games?

The melonDS DS ticket I linked to (posted by @DarkSide1305) contains two brief videos. This works the way OP expected it to. I think the recorded behavior uses RETRO_DEVICE_MOUSE, but I'm not sure.

This second one is what happens now in melonDS DS, using RETRO_DEVICE_POINTER. This behavior is undesired, as the cursor (i.e. virtual stylus) is lost when it hits the edge.

Do these videos help you understand the problem I'm facing? Or would you still like me to suggest a game for you?

@zoltanvb
Copy link
Contributor Author

Oh boy, more variables :) 2 questions:

  • the trackpad seen on the videos, is acting on OS level as a separate mouse, or is it already combined into one touchscreen input by the time it gets to RetroArch?
  • RetroArch viewport for MelonDS DS is 1 rectangle as usual, and the layout for 2 screens is done entirely inside the core, right?

@DarkSide1305
Copy link

The Decks Touchpads just operate the OS Mouse

@JesseTG
Copy link
Contributor

JesseTG commented Nov 11, 2024

As for your second question...

* RetroArch viewport for MelonDS DS is 1 rectangle as usual, and the layout for 2 screens is done entirely inside the core, right?

That's right. From RetroArch's perspective, nothing unusual is going on. All the linear algebra (screen layouts, cursor coordinates, touch screen coordinates...) is happening inside the core.

@zoltanvb
Copy link
Contributor Author

zoltanvb commented Nov 15, 2024

After some investigation... here's how the input drivers currently treat mouse, pointer, and lightgun. Input drivers missing from the list do not handle either one. Mouse can have a maximum of 5 buttons and 2 scroll wheels, screen query refers to special mouse and pointer device queries used by menu, which ignores the viewport.

Input driver and type Input from Multi-device support Multi-button support Viewport translation invoked screen query supported Other notes
winraw mouse mouse devices yes 5/5, 2/2 n/a yes
winraw pointer pointer device or mouse fallback yes multi-touch yes yes If pointer is out-of-bounds, 0 is returned for all queries
winraw lightgun mouse devices yes via key/mouse binds yes n/a offscreen detection OK, -0x8000 not returned
dinput mouse system mouse port 0 only 5/5, 2/2 n/a yes
dinput pointer pointer device or mouse fallback port 0 only multi-touch yes yes If pointer is out-of-bounds, 0 is returned for all queries
dinput lightgun system mouse same value reported for all ports via key/mouse binds yes n/a offscreen detection OK, -0x8000 not returned, idx used but why?
uwp mouse curr input same value reported for all ports 5/5, 2/2 n/a yes
uwp pointer curr input same value reported for all ports multi-touch yes yes
sdl mouse mouse port 0 only 5/5, 2/2 n/a bad
sdl pointer mouse same value reported for all ports 1 touch mapped to left mouse button yes yes query of LIGHTGUN_IS_OFFSCREEN (!) is allowed
sdl lightgun deprecated relative lightgun only, rework needed
x11 mouse system mouse same value reported for all ports 5/5, 2/2 n/a yes
x11 pointer system mouse port 0 only 1 touch mapped to left mouse button yes yes If pointer is out-of-bounds, 0 is returned for all queries
x11 lightgun system mouse same value reported for all ports via key/mouse binds yes n/a offscreen detection OK, -0x8000 not returned
wayland mouse system cursor port 0 only 5/5, 2/2 n/a yes
wayland pointer system cursor port 0 only 1 touch mapped to left mouse button yes yes If pointer is out-of-bounds, 0 is returned for all queries, touch input is only used for screen queries
wayland lightgun system cursor port 0 only fixed map yes n/a offscreen detection OK, extra binds not supported, if out-of-bounds, 0 is returned for all queries
udev mouse physical device yes 5/5,2/2 n/a yes if udev touch exists, it will be substituted
udev pointer physical device yes 1 touch custom yes if udev touch exists, it will be substituted, -0x8000 not returned due to special translation
udev lightgun physical device yes via key/mouse binds custom n/a if udev touch exists, nothing is returned, -0x8000 not returned due to special translation
android mouse mouse port 0 only 3/5, 1/2 n/a yes
android pointer touch same value reported for all ports multi-touch yes yes If pointer is out-of-bounds, touches are not reported, -0x8000 is returned if pointer is out-of-bounds in some direction
android lightgun deprecated relative lightgun only, rework needed
cocoa mouse mouse same value reported for all ports 2/5, 2/2 n/a yes
cocoa pointer touch same value reported for all ports multi-touch yes yes If pointer is out-of-bounds, touches are not reported
gx mouse mouse yes 2/5, 0/2 n/a no
gx lightgun mouse yes custom yes n/a Lightgun button binds are not handled, custom offscreen detection in platform code
wiiu pointer _ same value reported for all ports 1 touch no no
switch mouse mouse same value reported for all ports 3/5, 1/2 n/a yes
switch pointer touch same value reported for all ports multi-touch yes yes
ps3 mouse physical mouse same value reported for all ports 2/5, 0/2 n/a no
ps3 lightgun physical lightgun same value reported for all ports custom yes n/a offscreen detection OK, binds not supported, -0x8000 not returned
psp mouse physical mouse same value reported for all ports 3/5, 0/2 n/a yes
qnx pointer _ same value reported for all ports multi-touch yes yes If pointer is out-of-bounds, touches are not reported
rweb mouse mouse same value reported for all ports 5/5, 2/2 n/a yes
rweb pointer mouse same value reported for all ports 1 touch mapped to left mouse button yes yes if pointer is out-of-bounds, 0 is returned for all queries except LIGHTGUN_IS_OFFSCREEN (!)
overlay mouse pointer provided by input driver same value reported for all ports 3/5, 0/2 n/a n/a
overlay pointer pointer provided by input driver same value reported for all ports multi-touch n/a n/a If pointer is out-of-bounds, touches are not reported
overlay lightgun pointer provided by input driver one configurable port overlay buttons n/a n/a -0x8000 is returned if pointer is out-of-bounds

@JesseTG
Copy link
Contributor

JesseTG commented Nov 15, 2024

@zoltanvb Wow, nice work! Which commit or release of RetroArch did you check this on?

@zoltanvb
Copy link
Contributor Author

On kind of current code, slightly newer than this PR.

@zoltanvb
Copy link
Contributor Author

Still thinking about the conclusion, but looking at some specific points:

* On Android RETRO_DEVICE_POINTER already seems clamped but buggy? Touching the black area to the left or right gives X -32768 but leaves Y valid. Touching the black area above or below gives Y -32768 but leaves X valid.

This is actually what happens if the result of the current viewport translation is just passed on to the client. It's just that most desktop drivers will do a check and return 0,0 instead.

* `RETRO_DEVICE_ID_LIGHTGUN_IS_OFFSCREEN` is the correct way for a core to check for an offscreen pointer. dinput/winraw/x11 (and others) report anything above +/- 32700 as off screen but not all input drivers follow that.

Some input drivers support that, but it is giving me itches seeing it in the pointer query case branches. We could at least introduce RETRO_DEVICE_ID_POINTER_IS_OFFSCREEN with same numerical value, and have it as official part of the API. Mouse offscreen as a concept is debatable, although most drivers do maintain an absolute position as well.

Also, if offscreen is true, then x/y could still be clamped, and not set to -0x8000 as it carries no more information. At least with pointer, since for lightgun it is supposed to return -0x8000 (but actually doesn't, with the exception of the overlay lightgun), so maybe that could also be unified in the API. On a slightly related note, going fully out-of-bounds is not always possible, so the core shouldn't really rely on that.

* Most input drivers use the function `video_driver_translate_coord_viewport` which was touched by this PR for mouse/touch/lightgun inputs, but not all.

I have found only 1 that does not (wiiu), and udev has its own logic for the same, but everywhere else that function is used well.

there are games that expect you to hold down the stylus at the edge of the screen.

For this one, I can not easily resolve the conflict between not reporting touches outside the viewport, vs. allowing the user to do the touch slightly outside (where there may be overlay elements, etc.).

@JesseTG
Copy link
Contributor

JesseTG commented Nov 17, 2024

For this one, I can not easily resolve the conflict between not reporting touches outside the viewport, vs. allowing the user to do the touch slightly outside (where there may be overlay elements, etc.).

Maybe this is where the environment call from this PR would've helped? Or one that's similar.

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.

5 participants