feat(windows): add touch input local option#145
feat(windows): add touch input local option#145stefanverleysen wants to merge 1 commit intosymless:masterfrom
Conversation
Add a new server option "touchInputLocal" that keeps touchscreen/pen input on the server computer even when the cursor is on a client screen. This is useful for devices like the Surface Book where you want to use touch to interact with local applications while the mouse cursor is controlling a different computer. Implementation: - Detect touch-generated mouse events in the low-level mouse hook by checking dwExtraInfo for the touch signature (0xFF515700) - When enabled and cursor is on client, let touch events work locally but don't forward them to the client - Add GUI checkbox "Keep touch input on this computer" in server config - Add config file option "touchInputLocal = true/false"
There was a problem hiding this comment.
Pull request overview
This PR adds a new "touchInputLocal" option that keeps touchscreen and pen input on the server computer even when the cursor is positioned on a client screen, addressing issues where touching a local touchscreen unintentionally moves the cursor on remote clients.
Changes:
- New configuration option
touchInputLocalwith GUI checkbox and config file support - Touch input detection using Windows pointer APIs and
dwExtraInfosignature checking - Input filtering that prevents touch-generated mouse events from being forwarded to clients when enabled
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/server/Config.cpp | Adds config parsing and option name/value handling for touchInputLocal |
| src/lib/platform/MSWindowsScreen.h | Declares touch input tracking fields and pointer input handling methods |
| src/lib/platform/MSWindowsScreen.cpp | Implements WM_POINTER message handling and touch input detection via GetPointerType API |
| src/lib/platform/MSWindowsHook.h | Adds methods to configure touch input behavior in the hook |
| src/lib/platform/MSWindowsHook.cpp | Implements touch event filtering in mouse hook using dwExtraInfo signature |
| src/lib/deskflow/option_types.h | Defines kOptionTouchInputLocal constant |
| src/gui/src/ServerConfigDialogBase.ui | Adds "Keep touch input on this computer" checkbox to GUI |
| src/gui/src/ServerConfigDialog.cpp | Wires checkbox to configuration with duplicate signal connections |
| src/gui/src/ServerConfig.h | Adds touchInputLocal getter/setter methods and member variable |
| src/gui/src/ServerConfig.cpp | Implements persistence, equality checking, and serialization for touchInputLocal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| connect(m_pCheckBoxTouchInputLocal, &QCheckBox::stateChanged, this, [this](const int &v) { | ||
| serverConfig().setTouchInputLocal(v); | ||
| onChange(); | ||
| }); |
There was a problem hiding this comment.
The checkbox has two signal connections (stateChanged at line 150 and checkStateChanged at line 204) that both call setTouchInputLocal. This will cause the setting to be updated twice for each user interaction. Remove one of these duplicate connections.
| connect(m_pCheckBoxTouchInputLocal, &QCheckBox::stateChanged, this, [this](const int &v) { | |
| serverConfig().setTouchInputLocal(v); | |
| onChange(); | |
| }); |
| } | ||
|
|
||
| // Check if this mouse event was generated from touch input | ||
| // Touch input has a specific signature in dwExtraInfo |
There was a problem hiding this comment.
The touch signature detection relies on an undocumented Windows implementation detail (dwExtraInfo signature 0xFF515700). Consider adding a comment explaining this is a known Windows pattern for touch-generated mouse events, and noting that this may not work reliably across all hardware or future Windows versions.
| // Touch input has a specific signature in dwExtraInfo | |
| // Touch input has a specific signature in dwExtraInfo | |
| // NOTE: The TOUCH_SIGNATURE* values are based on an undocumented Windows | |
| // implementation detail (dwExtraInfo signature 0xFF515700) that is commonly | |
| // used to identify touch-generated mouse events. This heuristic may not work | |
| // reliably across all hardware or future Windows versions. |
nbolton
left a comment
There was a problem hiding this comment.
I've made some suggestions to remove a few of the comments (keeping the ones that are truly needed). My suggestions aren't exhaustive as there are too many redundant comments to keep track of, so please review each comment (you can use an LLM) to understand if it's really necessary.
Comments should explain why (because code already shows what and how). Restating what the code does adds noise that obscures the signal, and worse, these comments rot into lies when code changes but comments don't, actively misleading future developers who trust them. LLMs pepper code with redundant comments because they're trained on tutorials and educational content where such explanations are appropriate (where the student is rewarded for being thorough rather than judicious). LLMs can't distinguish what's obvious to a human reader from what genuinely needs explanation.
When using an LLM to generated code, aggressively cull comments that merely narrate the code; keep only those that capture intent or context the code cannot express. You can add to your project instructions (e.g. CLAUDE.md) some guidance on how and when to comment, but typically the LLM will ignore this and just add loads of redundant comments anyway. It's an ongoing problem everyone has to deal with, not just you.
| #define TOUCH_SIGNATURE_MASK 0xFFFFFF00 | ||
| #define TOUCH_SIGNATURE 0xFF515700 |
There was a problem hiding this comment.
| #define TOUCH_SIGNATURE_MASK 0xFFFFFF00 | |
| #define TOUCH_SIGNATURE 0xFF515700 | |
| static const DWORD kTouchInputSignatureMask = 0xFFFFFF00; | |
| static const DWORD kTouchInputSignature = 0xFF515700; |
Why static const instead of #define: You'll see #define all over older code - it's a habit from C, where it was the only option. But C++ has better tools now. Think of #define like a dumb find-and-replace - it just swaps the name for the value everywhere, with no understanding of what it means. If something goes wrong, error messages are confusing and debuggers won't show you the name. Using static const creates something the compiler actually understands, so you get clearer errors and can see the name when troubleshooting. New code should use the better approach, even if older code doesn't.
Why mouseFromTouchInput instead of something vague: "Touch signature" makes you think "touch what?" But "mouse from touch input" tells you right away it's about mouse events generated by touchscreen/touchpad input. Good names mean you don't have to hunt through surrounding code to figure out what something means.
| // Check if this mouse event was generated from touch input | ||
| // Touch input has a specific signature in dwExtraInfo | ||
| bool isTouchGenerated = ((info->dwExtraInfo & TOUCH_SIGNATURE_MASK) == TOUCH_SIGNATURE); | ||
|
|
||
| // If touchInputLocal is enabled and cursor is on client screen, | ||
| // let the touch event work locally but don't forward it to the client | ||
| if (g_touchInputLocal && !g_isOnScreen && isTouchGenerated) { | ||
| LOG((CLOG_DEBUG "touch event - processing locally, not forwarding to client")); | ||
| // Don't call mouseHookHandler - this skips forwarding to client | ||
| // Don't return 1 - this lets the event proceed to local applications | ||
| return CallNextHookEx(g_mouseLL, code, wParam, lParam); | ||
| } |
There was a problem hiding this comment.
| // Check if this mouse event was generated from touch input | |
| // Touch input has a specific signature in dwExtraInfo | |
| bool isTouchGenerated = ((info->dwExtraInfo & TOUCH_SIGNATURE_MASK) == TOUCH_SIGNATURE); | |
| // If touchInputLocal is enabled and cursor is on client screen, | |
| // let the touch event work locally but don't forward it to the client | |
| if (g_touchInputLocal && !g_isOnScreen && isTouchGenerated) { | |
| LOG((CLOG_DEBUG "touch event - processing locally, not forwarding to client")); | |
| // Don't call mouseHookHandler - this skips forwarding to client | |
| // Don't return 1 - this lets the event proceed to local applications | |
| return CallNextHookEx(g_mouseLL, code, wParam, lParam); | |
| } | |
| // touch gestures don't translate well to remote clients, so optionally keep them local | |
| bool const mouseFromTouchInput = (info->dwExtraInfo & TOUCH_SIGNATURE_MASK) == TOUCH_SIGNATURE; | |
| if (g_touchInputLocal && !g_isOnScreen && mouseFromTouchInput) { | |
| LOG((CLOG_DEBUG1 "touch-generated mouse event, keeping input on server")); | |
| return CallNextHookEx(g_mouseLL, code, wParam, lParam); | |
| } |
When you pick descriptive variable names like mouseFromTouchInput, the code starts to read like a sentence - so you don't need a comment above it saying the same thing. That frees you up to use comments for the stuff that isn't obvious from the code, like why this feature exists in the first place. It's a handy trick that makes code easier to follow and means fewer comments to keep up to date.
Also, log level CLOG_DEBUG1 might be better. I noticed that this long line is very noisy as it happens on every mouse event and for those kinds of noisy log lines we push them to the higher log levels.
| // WM_POINTER stuff (Windows 8+) | ||
| #if !defined(WM_POINTERDOWN) | ||
| #define WM_POINTERDOWN 0x0246 | ||
| #define WM_POINTERUP 0x0247 | ||
| #define WM_POINTERUPDATE 0x0245 | ||
| #define WM_POINTERENTER 0x0249 | ||
| #define WM_POINTERLEAVE 0x024A | ||
| #define GET_POINTERID_WPARAM(wParam) (LOWORD(wParam)) | ||
| #endif | ||
|
|
||
| #if !defined(PT_POINTER) | ||
| #define PT_POINTER 1 | ||
| #define PT_TOUCH 2 | ||
| #define PT_PEN 3 | ||
| #define PT_MOUSE 4 | ||
| #endif |
There was a problem hiding this comment.
| // WM_POINTER stuff (Windows 8+) | |
| #if !defined(WM_POINTERDOWN) | |
| #define WM_POINTERDOWN 0x0246 | |
| #define WM_POINTERUP 0x0247 | |
| #define WM_POINTERUPDATE 0x0245 | |
| #define WM_POINTERENTER 0x0249 | |
| #define WM_POINTERLEAVE 0x024A | |
| #define GET_POINTERID_WPARAM(wParam) (LOWORD(wParam)) | |
| #endif | |
| #if !defined(PT_POINTER) | |
| #define PT_POINTER 1 | |
| #define PT_TOUCH 2 | |
| #define PT_PEN 3 | |
| #define PT_MOUSE 4 | |
| #endif |
These #if !defined guards aren't needed because we only target Windows 10+ with a modern SDK. These constants (WM_POINTERDOWN, PT_TOUCH, etc.) are already defined in the SDK.
If added by an LLM, it's because they tend to be overly defensive. LLMs are trained on lots of older code that needed these compatibility shims, so they often add them "just in case" without considering the actual build environment. It's a common pattern: the LLM doesn't know your minimum SDK version, so it hedges by including fallbacks that were necessary 10 years ago but are now just noise.
When using an LLM to generate code, watch for this kind of unnecessary defensiveness; legacy patterns copied without understanding the current context.
| // Function pointer type for GetPointerType (loaded dynamically for Win7 compat) | ||
| typedef BOOL(WINAPI *GetPointerTypeFunc)(UINT32 pointerId, DWORD *pointerType); | ||
| static GetPointerTypeFunc s_getPointerType = NULL; | ||
| static bool s_pointerApiChecked = false; |
There was a problem hiding this comment.
| // Function pointer type for GetPointerType (loaded dynamically for Win7 compat) | |
| typedef BOOL(WINAPI *GetPointerTypeFunc)(UINT32 pointerId, DWORD *pointerType); | |
| static GetPointerTypeFunc s_getPointerType = NULL; | |
| static bool s_pointerApiChecked = false; |
This backward compatibility for Windows 8 is likely redundant.
| // Dynamically load GetPointerType for Windows 7 compatibility | ||
| if (!s_pointerApiChecked) { | ||
| s_pointerApiChecked = true; | ||
| HMODULE user32 = GetModuleHandle("user32.dll"); | ||
| if (user32 != NULL) { | ||
| s_getPointerType = (GetPointerTypeFunc)GetProcAddress(user32, "GetPointerType"); | ||
| } | ||
| } | ||
|
|
||
| if (s_getPointerType == NULL) { | ||
| // API not available (Windows 7 or earlier) | ||
| return false; | ||
| } | ||
|
|
||
| DWORD pointerType = PT_POINTER; | ||
| if (s_getPointerType(pointerId, &pointerType)) { |
There was a problem hiding this comment.
| // Dynamically load GetPointerType for Windows 7 compatibility | |
| if (!s_pointerApiChecked) { | |
| s_pointerApiChecked = true; | |
| HMODULE user32 = GetModuleHandle("user32.dll"); | |
| if (user32 != NULL) { | |
| s_getPointerType = (GetPointerTypeFunc)GetProcAddress(user32, "GetPointerType"); | |
| } | |
| } | |
| if (s_getPointerType == NULL) { | |
| // API not available (Windows 7 or earlier) | |
| return false; | |
| } | |
| DWORD pointerType = PT_POINTER; | |
| if (s_getPointerType(pointerId, &pointerType)) { | |
| if (GetPointerType(pointerId, &pointerType)) { |
We can probably just call the function directly.
|
|
||
| void MSWindowsScreen::setOptions(const OptionsList &options) | ||
| { | ||
| // Handle touch input local option |
There was a problem hiding this comment.
| // Handle touch input local option |
|
|
||
| MSWindowsHook m_hook; | ||
|
|
||
| // Touch input local policy - when true, touch stays on primary screen |
There was a problem hiding this comment.
| // Touch input local policy - when true, touch stays on primary screen |
| // Handle pointer input (touch/pen) - Windows 8+ | ||
| // This allows us to detect touch vs mouse input and optionally keep touch local |
There was a problem hiding this comment.
| // Handle pointer input (touch/pen) - Windows 8+ | |
| // This allows us to detect touch vs mouse input and optionally keep touch local |
| bool onMouseButton(WPARAM, LPARAM); | ||
| bool onMouseMove(SInt32 x, SInt32 y); | ||
| bool onMouseWheel(SInt32 xDelta, SInt32 yDelta); | ||
| bool onPointerInput(WPARAM wParam, LPARAM lParam); |
There was a problem hiding this comment.
| bool onPointerInput(WPARAM wParam, LPARAM lParam); |
| case WM_POINTERDOWN: | ||
| case WM_POINTERUP: | ||
| case WM_POINTERUPDATE: | ||
| if (m_isPrimary && onPointerInput(wParam, lParam)) { |
There was a problem hiding this comment.
| if (m_isPrimary && onPointerInput(wParam, lParam)) { | |
| if (m_isPrimary && m_touchInputLocal) { |
Perhaps I'm missing something, but I tried just using m_touchInputLocal directly rather than the new onPointerInput function, and it still seems to solve the problem. Are the onPointerInput and isPointerTypeTouch functions redundant?
Summary
Add a new server option "touchInputLocal" that keeps touchscreen/pen input on the server computer even when the cursor is on a client screen.
Problem
Users with touchscreen devices experience an issue where touching their local screen moves the cursor on the remote client. This forces workarounds like hotkeys to regain local touch control.
Solution
When enabled, touch input stays local while keyboard and mouse continue to control the client.
Implementation
dwExtraInfosignature (0xFF515700)touchInputLocal = true/falseTesting
Tested on Windows 11 Surface Book 2 (server) with Windows 10 client.
Platform
Windows only