Fix pan & swipe update event values on Windows#33540
Fix pan & swipe update event values on Windows#33540kubaflo merged 2 commits intodotnet:inflight/currentfrom
Conversation
Using the delta from ManipulationDeltaRoutedEventArgs for cumulative displacement generates invalid event data. Consider the following sequence for X coordinates moving the mouse from left to right: x = 0; delta = 0; => totalX = 0; x = 10; delta = 10; => totalX = 20; x = 11 delta = 1; => totalX = 12; On low end hardware or complex scenes, this causes the pan to jump around when dragging. Fixes: dotnet#33539
|
Hey there @@jeremy-visionaid! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
@PureWeen Looks like you wrote this part of the gesture manager. Any idea why it's using the manipulation delta? I assume there was some reason for it, but the values it generates can lead to wildly incorrect values for the pan & swipe events. |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33540 | Stop adding e.Delta.Translation to e.Cumulative.Translation and forward only cumulative translation values for Windows pan/swipe update callbacks. |
⏳ PENDING (Gate) | src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs |
Original PR |
🚦 Gate — Test Verification
Gate Result: ⚠️ SKIPPED
Platform: android
Mode: Full Verification
- Tests FAIL without fix: ❌
- Tests PASS with fix: ❌
Notes
- PR Fix pan & swipe update event values on Windows #33540 contains no test files.
- The linked issue Pan gesture update events have incorrect values on Windows #33539 is Windows-only, while the requested test platform is android.
- The changed file,
src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.Windows.cs, is never exercised on android, so fail/pass verification on android would be meaningless. - A meaningful Gate for this PR would require Windows-based validation.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) |
Accumulate e.Delta.Translation in dedicated running fields reset at gesture start, then send the tracked totals. |
1 file | Alternative to cumulative only; blocked because Windows validation is unavailable on Linux. |
|
| 2 | try-fix (claude-sonnet-4.6) |
Compute translation from e.Position - startPosition instead of any translation properties. |
1 file | Geometric alternative using start position; also blocked by missing Windows validation. | |
| 3 | try-fix (gpt-5.3-codex) |
Restrict the cumulative-translation change to running updates only instead of all statuses. | 1 file | Narrower variant; still unvalidated and less convincing than the PR fix. | |
| 4 | try-fix (gemini-3-pro-preview) |
Compute translation in global coordinate space from a captured start point. | 1 file | Stable-frame geometric variant; also unvalidated on this host. | |
| 5 | try-fix (gemini-3-pro-preview, cross-pollination) |
Set the manipulation container/reference frame to the window root so cumulative translation is measured in a stable coordinate space. | 1 file | Interesting WinUI-specific idea, but broader/riskier and still unvalidated on Linux. | |
| 6 | try-fix (claude-opus-4.6, cross-pollination) |
Capture the initial cumulative translation as a baseline and subtract it from subsequent cumulative values. | 1 file | More defensive variant; mostly equivalent to the PR fix in normal WinUI behavior, but more complex. | |
| 7 | try-fix (claude-sonnet-4.6, cross-pollination) |
Derive translation directly from pointer position minus the gesture start position. | 1 file | Avoids translation APIs entirely, but still needs Windows validation. | |
| 8 | try-fix (gemini-3-pro-preview, cross-pollination) |
Track raw active-pointer centroid positions and accumulate translation manually. | 1 file | Most invasive approach; far riskier than the PR fix and still unvalidated. | |
| PR | PR #33540 | Send only e.Cumulative.Translation for Windows pan/swipe updates. |
1 file | Original PR fix; simplest and most directly aligned with the reported arithmetic bug. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Delta-only accumulation fields instead of using e.Cumulative. |
| claude-sonnet-4.6 | 1 | Yes | Derive translation from current pointer position minus gesture start position. |
| gpt-5.3-codex | 1 | Yes | Limit the fix to running gesture updates rather than all states. |
| gemini-3-pro-preview | 1 | Yes | Derive translation from a stable global coordinate frame. |
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | No | NO NEW IDEAS |
| gemini-3-pro-preview | 2 | Yes | Set manipulation container/root to stabilize WinUI cumulative translation. |
| claude-opus-4.6 | 3 | Yes | Subtract an initial cumulative baseline from later cumulative values. |
| claude-sonnet-4.6 | 3 | Yes | Use pointer-position deltas instead of manipulation translation. |
| gpt-5.3-codex | 3 | No | NO NEW IDEAS |
| gemini-3-pro-preview | 3 | Yes | Use raw pointer centroid positions instead of manipulation translation. |
Exhausted: Yes (3 rounds completed; no validated alternative beat the PR fix)
Selected Fix: PR — best current candidate because it directly removes the double-counting arithmetic with the smallest, clearest change. Full validation still requires Windows.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Windows-only gesture bug, 1 implementation file changed, no tests added. |
| Gate | Requested platform was android, but the bug and fix are Windows-only, so meaningful fail/pass verification was not possible. | |
| Try-Fix | ✅ COMPLETE | 8 attempts, 0 passing, all blocked by lack of Windows runtime/SDK on Linux host. |
| Report | ✅ COMPLETE |
Summary
The PR fix is plausible and is the simplest candidate explored: it removes the obvious double-counting bug by forwarding only e.Cumulative.Translation for Windows pan and swipe updates. However, this review could not validate the fix empirically because the requested test platform (android) does not exercise the changed Windows code, the PR adds no tests, and the current host cannot run Windows validation.
Root Cause
GesturePlatformManager.Windows.cs was combining e.Delta.Translation with e.Cumulative.Translation for pan and swipe updates. Because the cumulative value already includes the current delta, this double-counts movement and can make the reported gesture coordinates jump backward or overshoot.
Fix Quality
The PR change is the best option among the explored candidates from a code-quality standpoint: it is the smallest and most direct correction to the arithmetic bug, while the alternatives were either more complex, more invasive, or both. Still, without a Windows Gate run or test coverage that reproduces the regression, the fix remains unverified. I recommend requesting changes for Windows-based verification and/or regression coverage before merge.
Code Review — PR #33540✅ Verdict: APPROVEThis is a clean, correct 2-line fix for a real arithmetic bug. Root Cause AnalysisThe original code computed displacement as: Per the WinUI docs:
Since Cumulative already includes the current Delta, adding them together double-counts movement. The PR description example illustrates this perfectly: Cross-Platform Consistency CheckThe
The fix brings Windows in line with how iOS and Android already work. What ChangedBoth Potential Concern: No TestsThis PR does not add tests, which is understandable — testing this requires mocking SummaryMinimal, surgical, correct. The math was wrong, now it is right, and it matches the other platforms. 👍 |
### Description of Change Using the delta from ManipulationDeltaRoutedEventArgs for cumulative displacement generates invalid event data. Consider the following sequence for X coordinates moving the mouse from left to right: x = 0; delta = 0; => totalX = 0; x = 10; delta = 10; => totalX = 20; x = 11; delta = 1; => totalX = 12; On low end hardware or complex scenes, this causes the pan to jump around when dragging. ### Issues Fixed Fixes: #33539 Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
### Description of Change Using the delta from ManipulationDeltaRoutedEventArgs for cumulative displacement generates invalid event data. Consider the following sequence for X coordinates moving the mouse from left to right: x = 0; delta = 0; => totalX = 0; x = 10; delta = 10; => totalX = 20; x = 11; delta = 1; => totalX = 12; On low end hardware or complex scenes, this causes the pan to jump around when dragging. ### Issues Fixed Fixes: dotnet#33539 Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
### Description of Change Using the delta from ManipulationDeltaRoutedEventArgs for cumulative displacement generates invalid event data. Consider the following sequence for X coordinates moving the mouse from left to right: x = 0; delta = 0; => totalX = 0; x = 10; delta = 10; => totalX = 20; x = 11; delta = 1; => totalX = 12; On low end hardware or complex scenes, this causes the pan to jump around when dragging. ### Issues Fixed Fixes: #33539 Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
Description of Change
Using the delta from ManipulationDeltaRoutedEventArgs for cumulative displacement generates invalid event data. Consider the following sequence for X coordinates moving the mouse from left to right:
x = 0; delta = 0; => totalX = 0;
x = 10; delta = 10; => totalX = 20;
x = 11; delta = 1; => totalX = 12;
On low end hardware or complex scenes, this causes the pan to jump around when dragging.
Issues Fixed
Fixes: #33539