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

Update SDL3, bringing in support for Windows Ink pen input, and enable it on desktop platforms once again #6511

Merged
merged 6 commits into from
Jan 28, 2025

Conversation

@frenzibyte frenzibyte changed the title Update SDL3, bringing in support for Windows Ink pen input Update SDL3, bringing in support for Windows Ink pen input, and enable it on desktop platforms once again Jan 25, 2025
@frenzibyte
Copy link
Member

frenzibyte commented Jan 25, 2025

I was going to come here to mention that I've only deprioritised and no longer looked at ppy/osu#28222 as we still reverted back to using SDL2, but now that it is being enabled back again in this PR, I've went back and tested to see how fullscreen is behaving currently. I was glad to see that apparently I cannot reproduce the yabai issue anymore, so then I wanted to confirm that on SDL's test projects but somehow I still encountered it there.

I don't feel interested enough to care about why that happened but I'm giving this PR an i-dont-care approval from me with regards to having SDL3 enabled on macOS. If I get bothered enough, I'll report it out loud.

@frenzibyte frenzibyte requested a review from a team January 25, 2025 22:08
@Walavouchey
Copy link
Member

used susko's instructions to test the newer sdl3 binary with frenzi's pr + this pr's sdl hints, and it does allow touch tapping with pen aim (with noticeably more latency than regular touch aim and tapping), but it breaks hover input completely (can only drag) (logs if ya need 'em)

also summons the touch device mod when using the pen, which isn't quite expected either

@Susko3
Copy link
Member Author

Susko3 commented Jan 25, 2025

used susko's instructions to test the newer sdl3 binary with frenzi's pr + this pr's sdl hints, and it does allow touch tapping with pen aim (with noticeably more latency than regular touch aim and tapping), but it breaks hover input completely (can only drag) (logs if ya need 'em)

The logs confirm you're using the latest SDL3 👍

When you say that hover is broken, is it broken only in gameplay, or also in menus? If it's only gameplay, try applying this patch and see if the hovering playstyle works.

Which device are you using to test this?

When you say there's noticeable latency, do you think that is display latency or pen input latency? (I understand that it might be hard to tell.) Could be that there is an overlay display when using the pen, kicking osu! out of exclusive fullscreen. In my testing, I noticed a microstutter when the pen entered or left the hover range (indicative of fullscreen overlay shenanigans), but that might be due to Wacom drivers.

@Susko3
Copy link
Member Author

Susko3 commented Jan 26, 2025

@Walavouchey this branch ppy/osu@master...Susko3:osu:enable-mouse-hover-touch-click-play-style should work, with and without frenzi's PR. It was build with hovering playstyle in mind, but pressing the pen down shouldn't be too broken. Please give it a test.

@Walavouchey
Copy link
Member

Walavouchey commented Jan 26, 2025

Which device are you using to test this?

Surface Pro 8

When you say that hover is broken, is it broken only in gameplay, or also in menus? If it's only gameplay, try applying this patch and see if the hovering playstyle works.

everywhere (immediately noticeable when opening the game and using the pen)

When you say there's noticeable latency, do you think that is display latency or pen input latency? (I understand that it might be hard to tell.)

it's not like the framerate suffers or that there are microstutters (no spikes on the frame graph either). pen taps, touch taps, and touch taps while dragging with a finger are fine with no perceived latency, but specifically the touch taps while dragging the pen register later (i get around +50 ms hit error).

Could be that there is an overlay display when using the pen, kicking osu! out of exclusive fullscreen. In my testing, I noticed a microstutter when the pen entered or left the hover range (indicative of fullscreen overlay shenanigans), but that might be due to Wacom drivers.

no such stutters that i notice. i think windows vs fullscreen might make a few ms difference on my device but it's harder to notice than the latency i'm describing

this branch ppy/osu@master...Susko3:osu:enable-mouse-hover-touch-click-play-style should work, with and without frenzi's PR. It was build with hovering playstyle in mind, but pressing the pen down shouldn't be too broken. Please give it a test.

all with new sdl3 binary
that branch logs-with-frenzy-framework.zip:

  1. hover not broken, touch taps while dragging or hovering pen still have latency
  2. during gameplay (not menu), dragging the pen leaves the cursor motionless where you started the drag, unless you started dragging on a circle (a bit inconsistently)

that branch logs-without-modified-framework.zip:

  1. hover not broken, touch taps while dragging or hovering pen still have latency
  2. dragging the pen causes the cursor to lag (as in its position updates at a lower frequency)
  3. touch device mod flickers off and on (mostly on) while dragging the pen (seen in song select)

the patch you linked instead of that branch logs-with-frenzy-framework-susko-patch-2.zip:

  1. hover not broken, touch taps while dragging or hovering pen still have latency
    ^ best so far!

the patch you linked instead of that branch logs-without-modified-framework-susko-patch-2.zip

  1. hover not broken
  2. touch taps while dragging or hovering the pen put the cursor where you tap (can't play at all)
  3. dragging the pen causes the cursor to lag (as in its position updates at a lower frequency)
  4. touch device mod flickers off and on (mostly on) while dragging the pen (seen in song select)

in all these cases, TD mod is on while dragging pen and off when hovering

@smoogipoo
Copy link
Contributor

New SDL3-CS package: https://www.nuget.org/packages/ppy.SDL3-CS/2025.127.0

@Susko3
Copy link
Member Author

Susko3 commented Jan 27, 2025

Looks good on my part.

Further testing can be done based on this PR, to reduce possible error when setting up the environment. For example, logs-without-modified-framework.zip show that this test was done on an older commit, before c901353.

@Walavouchey please give it a test again, this PR + ppy/osu@master...Susko3:osu:enable-mouse-hover-touch-click-play-style, no additional mangling with SDL3.dll needed. Touch device mod should not activate if you only use the pen. If it does, check what InputHandlers are reporting inputs in Ctrl+F2.

Also useful is to enable SDL event logging, to check if we're receiving correct events:

diff --git a/osu.Framework/Platform/SDL3/SDL3Window.cs b/osu.Framework/Platform/SDL3/SDL3Window.cs
index 8f7783571..70a895b28 100644
--- a/osu.Framework/Platform/SDL3/SDL3Window.cs
+++ b/osu.Framework/Platform/SDL3/SDL3Window.cs
@@ -213,6 +213,7 @@ public virtual void Create()
             SDL_SetHint(SDL_HINT_PEN_TOUCH_EVENTS, "0"u8);
             SDL_SetHint(SDL_HINT_PEN_MOUSE_EVENTS, "0"u8);
             SDL_SetHint(SDL_HINT_IME_IMPLEMENTED_UI, "composition"u8);
+            SDL_SetHint(SDL_HINT_EVENT_LOGGING, "2"u8);
 
             SDLWindowHandle = SDL_CreateWindow(title, Size.Width, Size.Height, flags);
 

@Walavouchey
Copy link
Member

Walavouchey commented Jan 27, 2025

@Walavouchey please give it a test again, this PR + ppy/osu@master...Susko3:osu:enable-mouse-hover-touch-click-play-style, no additional mangling with SDL3.dll needed. Touch device mod should not activate if you only use the pen. If it does, check what InputHandlers are reporting inputs in Ctrl+F2.

logs .osr replay enabled sdl logs

  • works ok, same as test 3 in my previous comment
  • playing with only the pen doesn't activate TD, while pen + touch does
  • looks like i didn't have mouse buttons disabled, so i miss the first note when dragging the pen and tapping for the first time (normally happens with keyboard + pen but not with two-hand touch)

@smoogipoo
Copy link
Contributor

Does the above mean this PR is ready to review?

@Susko3
Copy link
Member Author

Susko3 commented Jan 28, 2025

Yep, ready for review.

@peppy peppy self-requested a review January 28, 2025 12:54
@peppy
Copy link
Member

peppy commented Jan 28, 2025

Let's get this in and give ample time for devs to dogfood the change before we roll out the next major release.

@peppy peppy merged commit 6bbcfd8 into ppy:master Jan 28, 2025
14 checks passed
@Susko3 Susko3 deleted the update-SDL3 branch January 28, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants