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

windows: handle WM_POINTERCAPTURECHANGED events, fix maybe-undefined behavior. #11869

Closed
wants to merge 1 commit into from

Conversation

icculus
Copy link
Collaborator

@icculus icculus commented Jan 6, 2025

Fixes #11843.
Fixes #11844.

CC @Susko3, does this cover it?

Copy link
Contributor

@Susko3 Susko3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WM_POINTERCAPTURECHANGED looks correct.

@@ -1195,6 +1198,8 @@ LRESULT CALLBACK WIN_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPara

case WM_POINTERDOWN:
case WM_POINTERUP: {
returnCode = 0; // always mark this event handled, so it doesn't go to DefWindowProc (see #11843).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's likely that always handling these messages might regress touch input. Since SDL is lying to windows about handling the PT_TOUCH messages, even though it's not doing anything with them (i.e. not sending to the application).

The two issues I opened show violation of the documentation, but any real-world impact is untested. I invite you to you to test some of the hypothetical problems I listed. I'm currently travelling and don't have access to my touchscreen monitor and graphics tablet to test. Sorry if my original issue description is unclear about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I might back this part out for now and wait for bug reports to arrive...I'm not really clear that this is even what Microsoft is asking me to do, honestly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Microsoft likely wants you to handle all pointer events properly, but SDL already has perfectly good touch handling via WM_TOUCH. I'm fine with only the WM_POINTERCAPTURECHANGED change going in (please update the PR title and description accordingly). I'll try to borrow some graphics tablets and touchscreen monitors and do some testing as set out in #11843.

@icculus
Copy link
Collaborator Author

icculus commented Jan 7, 2025

Put in the WM_POINTERCOUNTERCHANGED fixes in 0180ca5. Dumping the rest for now.

@icculus icculus closed this Jan 7, 2025
@icculus icculus deleted the sdl3-windows-ink-fix-ub branch January 7, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants