Add native touchscreen pinch-zoom to the Windows graph - #447
Merged
Merged
Conversation
- MainWindow.zig: register a GID_ZOOM-only SetGestureConfig (GID_PAN/ GID_ROTATE/GID_TWOFINGERTAP/GID_PRESSANDTAP explicitly blocked since unimplemented), capturing GetLastError immediately on failure. Real registration test (positive control) plus a negative cbSize control proving the failure path fires, both against a genuine unshown HWND. - GraphCanvas.zig: CanvasState.beginPinchZoom/continuePinchZoom/ endPinchZoom compute an absolute target zoom from the gesture's captured base distance (not per-message compounding), reusing the existing zoomBy clamp/anchor. u32 distance domain matches the OS's actual integer report. Full test matrix: base capture, non- compounding repeats, clamp range, zero/missing distance, end-of- gesture reset, and a large-u32 finiteness check. - CanvasInput.zig: pure classifyGesture(dwID, flags, in_canvas) decision table (forward_unhandled / forward_out_of_region / begin_zoom / continue_zoom / end_zoom), directly unit tested. - App.zig: new WM_GESTURE case decodes GID_ZOOM via classifyGesture, maps GESTUREINFO.ptsLocation through the same ScreenToClient + region classification WM_MOUSEWHEEL already uses, and applies the pinch methods. Every unhandled/out-of-region message returns false (relying on MainWindow.windowProc's existing single DefWindowProcW forward) rather than closing or re-forwarding the gesture handle, per the documented WM_GESTURE/CloseGestureInfoHandle ownership contract. One line added to WM_ACTIVATE resets pinch state on deactivation. Registration failure surfaces through the existing setStatus diagnostic path. - ui-parity-matrix.md: updates only the Pan and anchored zoom row to describe the new touchscreen pinch coverage, explicitly distinguish it from Precision Touchpad's documented Ctrl+wheel emulation default (not device-verified here), and note touch-driven pan is still unimplemented. Row stays Partial. All 283 zig tests pass (pinned Zig 0.15.2), including a real, unmocked SetGestureConfig round trip; full executable link/build verified. No live UIA capture performed this pass (no foreground slot held); this is source + automated-test evidence only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
Fixes against the coordinator's review of d706937, not the summarized approval card: 1. registerCanvasGestureConfig now registers only GID_ZOOM; GID_PAN/ GID_ROTATE/GID_TWOFINGERTAP/GID_PRESSANDTAP are no longer explicitly blocked (App.zig already forwards any non-GID_ZOOM message unhandled). 2. A per-gesture context identity (Wyhash of surface tag + current project path, computed fresh every message, never a borrowed pointer) is now threaded through beginPinchZoom/continuePinchZoom. A same-region destination change mid-gesture (surface flips to the terminal workspace, or the project changes while still graph-capable) resets the baseline instead of continuing to scale the wrong canvas. WM_GESTURE now also requires the active surface to actually render the graph canvas, not just the wheel-region rectangle (which the terminal workspace shares). 3. A failed GetClientRect is now guarded: pinch state resets and the message is treated as unhandled instead of classifying against an undefined rect. 4. The gesture handle is now closed before syncAccessibility()/ InvalidateRect (which can re-enter the message loop), not after. A new CanvasInput.GestureDecision.begin_and_end_zoom outcome handles a gesture delivered as a single combined begin+end message by establishing then immediately clearing a fresh baseline, rather than mapping it to end_zoom (which could apply a stale prior gesture's baseline). 5. Added a second registerCanvasGestureConfig test that calls the production helper itself with a genuinely invalid HWND, proving its own GetLastError capture path fires, not just the existing malformed-native-call negative control. 6. Reworded the gesture-registration diagnostic comment (removed the false "no logging mechanism exists" claim) and added a test proving window.gesture_config_registered/gesture_config_last_error survive the later setStatus() calls in App.run() that are known to overwrite the transient status line. 7. Verified via genuine temporary regressions, not GREEN-only tests: reintroduced the old per-message-relative (compounding) zoom math, confirmed "does not compound" fails, restored; removed the context check in continuePinchZoom, confirmed the mismatch test fails, restored; made registerCanvasGestureConfig ignore SetGestureConfig's result and always report success, confirmed the new invalid-HWND test fails, restored. Full per-file zig test suites, the 42-file WindowsShell.Tests.ps1 contract (anti-drift guard included), and a full zig build all pass. ui-parity-matrix.md's Pan and anchored zoom row reworded to match the corrected behavior; stays Partial pending live device evidence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
…/comments - App.zig: make the gesture-registration failure diagnostic durably observable. Extract formatGestureRegistrationFailure(buf, last_error) as the single production formatter, call it once, and log the result via std.log.warn in addition to the existing transient setStatus() call, so support/CI stderr retains the message (with the captured Win32 error code) even if a later startup setStatus() call overwrites the on-screen status line. Added a unit test asserting the exact formatted string (including the too-small-buffer fallback), and updated the existing durability test to build its expected string from the same production formatter instead of a hand-duplicated literal. - App.zig: trimmed the continue_zoom handle-close comment, which asserted that WM_GESTURE reentrancy specifically "already closed" another handle -- reentry alone doesn't establish that. Now simply states the handle must be released before syncAccessibility()/ InvalidateRect, which can pump messages. - GraphCanvas.zig: reworded continuePinchZoom's continuation-reset doc comment. It previously said the caller should begin a fresh pinch "on the next message"; it now says the reset is a no-op until a genuinely NEW beginPinchZoom establishes a fresh baseline, without implying anything about which message that arrives on. Verified after these changes: - zig test src/App.zig src/AccessibilityProvider.cpp ...: 289/289 pass - zig test src/GraphCanvas.zig ...: 109/109 pass - Tools/windows/Tests/WindowsShell.Tests.ps1 (42 wired source files, anti-drift structural guard included): PASS - zig build -Dwinghostty-dir=...: succeeds, graphcode-windows.exe produced No push. Awaiting coordinator clearance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens <coneilen@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds native touchscreen pinch-zoom through WM_GESTURE/GID_ZOOM.
compounding across repeated updates while retaining existing anchor/clamp behavior.
handles before publishing UI changes; unhandled gestures retain default handling.
Updates only the "Pan and anchored zoom" parity row, which remains Partial.
Regression evidence
These are post-implementation regression-sensitivity checks, not a claim of
test-first development.
RED: temporarily restore compounding zoom math -> the repeated-update regression
fails, producing 1.8 instead of the expected 1.5.
RED: temporarily remove the context-mismatch guard -> the context-switch regression
fails because zoom changes instead of resetting the gesture.
RED: temporarily make registerCanvasGestureConfig always report success ->
the production-helper invalid-HWND test fails.
GREEN: restore the production implementations -> all targeted regressions pass.
REGRESSION: run the wired App/AccessibilityProvider suite -> 289 tests pass;
MainWindow -> 9 tests pass; GraphCanvas -> 109 tests pass.
The coordinator independently reran App and MainWindow using pinned Zig 0.15.2
and coordinator-owned pinned Winghostty headers.
The 42-file WindowsShell contract suite and executable build also passed in the
author's environment. Those runs used an external Winghostty checkout, not an
own-worktree pinned provider build.
Limitations
in source and compiled, but not exercised through injected live WM_GESTURE failures.
Reviewed head: 7696e99.
All commits are signed off; the sign-off rebase preserved the reviewed tree.