-
Notifications
You must be signed in to change notification settings - Fork 420
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
Touch support for desktop platforms #5299
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done a code-only pass on this and it lgtm.
All working well now, until raw input is turned on, where things still seem to fall apart (movement seems to work but drag operations don't work, and tap operations only sometimes work). I'll investigate further tomorrow if the issue isn't something obvious. |
if (Native.Input.IsTouchEvent(Native.Input.GetMessageExtraInfo())) | ||
// touch events are handled by TouchHandler | ||
return IntPtr.Zero; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this never gets hit in testing.
I also tested
if (Native.Input.IsTouchEvent(data.Mouse.ExtraInformation))
and it also returns false even for touch.
Using mouse.ExtraInformation>0
(see incorrect commented out version below) does work.
For reference, ExtraInformation
is 139
in the touch event I tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you comment out our custom handling and let SDL do everything? Curious if SDL will properly filter out the raw mouse touch events.
Maybe the || Native.Input.GetMessageExtraInfo() & 0x82 == 0x82
check really is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon blaming through SDL, they seem to have been using the same IsTouchEvent
method used here, until 2 years ago, in which they added an additional condition with the intent of fixing touch input being sent as regular mouse messages, I've also found this thread from a year ago as well, which touches on the fact that the touch signature (0xFF515700
) being missing (along with the touch identifier bit, except that it does seem to be present with ExtraInformation
here).
I think that deduces the necessity of the GetMessageExtraInfo() & 0x82 == 0x82
condition, due to a seemingly looking bug in recent Windows versions I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can get behind only checking for 0x80
(and omitting the MI_WP_SIGNATURE_MASK
check), but the 0x02
there confuses me.
https://docs.microsoft.com/en-us/windows/win32/tablet/system-events-and-mouse-messages:
The lower 8 bits returned fromGetMessageExtraInfo
are variable. Of those bits, 7 (the lower 7, masked by 0x7F) are used to represent the cursor ID, zero for the mouse or a variable value for the pen ID.
Checking for 0x02
is probably wrong. This thread confirms that; the received values were 0xC7/C8/F7/F8/F9
-- some of which don't have 0x2
set.
Why I think just 0x80
is fine:
https://docs.microsoft.com/en-us/windows/win32/tablet/system-events-and-mouse-messages (cont.):
Additionally, in Windows Vista, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch).
https://discord.com/channels/188630481301012481/589331078574112768994938919701467226
GetMessageExtraInfo
always seemed to return 0
It seems GetMessageExtraInfo
is not reliable, so mouse.ExtraInformation
is what we want to use. It contains at least some useful information, as peppy said above.
Works perfectly now! |
if (Native.Input.IsTouchEvent(Native.Input.GetMessageExtraInfo())) | ||
// sometimes GetMessageExtraInfo returns 0, so additionally, mouse.ExtraInformation is checked below. | ||
// touch events are handled by TouchHandler | ||
return IntPtr.Zero; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably argue this is unnecessary at this point, but probably no harm in leaving it here..
Touch currently uses mouse emulation on desktop platforms. This PR sets the correct flag and implements
SDL_TouchFingerEvent
handling. Windows also emulates mouse events for touch, and those events have also been properly ignored inWindowsMouseHandler
.Initial testing by peppy on discord.
Raw input touch event detection inspired by SDL and confirmed by Microsoft Docs. (The
0x82
check seems to be unnecessary, as it's already covered by0x80
.)getTouchSource
/assignNextAvailableTouchSource
logic taken from iOS touch handler.