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.

Sign up for GitHub

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
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions src/video/windows/SDL_windowsevents.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@
#ifndef WM_POINTERLEAVE
#define WM_POINTERLEAVE 0x024A
#endif
#ifndef WM_POINTERCAPTURECHANGED
#define WM_POINTERCAPTURECHANGED 0x024C
#endif
#ifndef WM_UNICHAR
#define WM_UNICHAR 0x0109
#endif
Expand Down Expand Up @@ -1163,25 +1166,25 @@ LRESULT CALLBACK WIN_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPara
returnCode = 0;
} break;

case WM_POINTERCAPTURECHANGED:
case WM_POINTERLEAVE:
{
const UINT32 pointerid = GET_POINTERID_WPARAM(wParam);
void *hpointer = (void *) (size_t) pointerid;
const SDL_PenID pen = SDL_FindPenByHandle(hpointer);
if (pen == 0) {
break; // not a pen, or not a pen we already knew about.
}

// if this just left the _window_, we don't care. If this is no longer visible to the tablet, time to remove it!
if (!IS_POINTER_INCONTACT_WPARAM(wParam)) {
SDL_RemovePenDevice(WIN_GetEventTimestamp(), pen);
if (pen != 0) {
// if this just left the _window_, we don't care. If this is no longer visible to the tablet, time to remove it!
if ((msg == WM_POINTERCAPTURECHANGED) || !IS_POINTER_INCONTACT_WPARAM(wParam)) {
SDL_RemovePenDevice(WIN_GetEventTimestamp(), pen);
}
}
returnCode = 0;
} break;

case WM_POINTERUPDATE: {
POINTER_INPUT_TYPE pointer_type = PT_POINTER;
if (!data->videodata->GetPointerType || !data->videodata->GetPointerType(GET_POINTERID_WPARAM(wParam), &pointer_type)) {
returnCode = 0;
break; // oh well.
}

Expand All @@ -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.


POINTER_PEN_INFO pen_info;
const UINT32 pointerid = GET_POINTERID_WPARAM(wParam);
void *hpointer = (void *) (size_t) pointerid;
Expand Down Expand Up @@ -1242,8 +1247,6 @@ LRESULT CALLBACK WIN_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPara
if (msg == WM_POINTERDOWN) {
SDL_SendPenTouch(timestamp, pen, window, (pen_info.penFlags & PEN_FLAG_INVERTED) != 0, true);
}

returnCode = 0;
} break;

case WM_MOUSEMOVE:
Expand Down
Loading