Fix/all touch and perf fixes#10
Conversation
- Fix touch/pen pointer device type detection and screenPoint coordinates - Fix touch cancel to include all active touches per W3C spec - Synthesize touch-cancel for stale pointers and releases outside views - Fix TextInput pointer message translation (use mouse-style messages for RichEdit) - Fix ShouldSubmit modifier key checks (altDown, ctrlKey) - Add null safety to RootComponentView() for island teardown - Fix Pressability hover timeout and tabIndex focusable mapping - Cache event path to root to avoid repeated tree walks - Use unordered_set for pointer capture tracking - Eliminate O(n²) hit testing by caching visual children - Skip snap scroll reconfiguration when unchanged - Improve TextInput reliability: thread-safe loading, null safety, use-after-free fix - Fix Timing data race and remove duplicate image error allocation - Use unordered_set for animated node and component registry lookups - Clean up dead code in ScrollView and simplify Modal event emitter init Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR consolidates a broad set of touch correctness fixes and performance improvements across the Windows Composition layer. Key behavioural fixes include: stale-pointer cancellation on repeated press (replacing a silent early-return), correct pointer-device-type detection (Touch/Pen/Mouse), proper Confidence Score: 5/5PR is safe to merge; all findings are confirmations of intentional bug fixes, with no new defects introduced. All inline comments describe fixes rather than new bugs. The comments left are P2 observations confirming that the fixes are correct. No regressions found across the C++ or JS changes. CompositionEventHandler.cpp deserves a manual touch-flow smoke-test given the scope of the stale-pointer cancellation logic. Important Files Changed
Sequence DiagramsequenceDiagram
participant OS as OS Pointer Source
participant CEH as CompositionEventHandler
participant AT as m_activeTouches
participant EE as EventEmitter (JS)
OS->>CEH: onPointerPressed(pointerId)
CEH->>AT: find(pointerId)
alt stale touch found
AT-->>CEH: staleTouch
CEH->>EE: DispatchSynthesizedTouchCancel(staleTouch)
CEH->>AT: erase(pointerId)
end
CEH->>AT: insert new ActiveTouch (Touch/Pen/Mouse type)
CEH->>EE: onTouchStart / onPointerDown
OS->>CEH: onPointerReleased(pointerId)
CEH->>AT: find(pointerId)
alt tag == -1 (pointer left surface)
CEH->>EE: DispatchSynthesizedTouchCancel
CEH->>AT: erase(pointerId)
else normal release
CEH->>EE: onTouchEnd / onPointerUp
CEH->>AT: erase(pointerId)
end
OS->>CEH: onPointerCaptureLost
CEH->>AT: releasePointerCapture (unordered_set.erase)
Reviews (2): Last reviewed commit: "pr comments" | Re-trigger Greptile |
|
@greptile review |
- Fix IsPointerWithinInitialTree to hit-test at current pointer coords instead of trivially matching tree membership (P1 fix) - Add runtime bounds clamp in InsertAt to prevent cache/layer desync in release builds where assert is stripped - Remove unnecessary vector copy in anyHitTestHelper since single_threaded_vector::GetAt is already O(1) - Replace silent nullptr return with winrt::check_bool in createVisual for unrecoverable text services init failure Made-with: Cursor
Addresses P1 review feedback: GetAt returning nullptr silently shifts crashes to opaque call sites. The assert fires in debug builds so the root cause is caught at the source. Made-with: Cursor
No description provided.