[0.83] fix(pointer): backport #16140 (PointerRoutedAway + ScrollView contentOffset DIP) - #16335
Open
Collin Schneide (FaithfulAudio) wants to merge 1 commit into
Conversation
…w contentOffset DIP) to 0.83-stable Backports upstream PR microsoft#16140 by @gmacmaster (merged to main 2026-05-13, itself a cherry-pick of microsoft#16139 onto 0.84-stable) so that 0.83-stable also gets it. A literal cherry-pick does not build here, so two adaptations were required and are documented in the PR description: - onPointerRoutedAway uses the 0.83-era DispatchTouchEvent(TouchEventType::Cancel, ...) plus std::find_if over pair.second.touch.identifier, because microsoft#16140's CancelActiveTouchForPointerInternal / DispatchSynthesizedTouchCancelForActiveTouch helpers do not exist on this branch. - microsoft#16140's onPointerCaptureLost refactor is omitted: on main that function already had an active-touch cleanup block to refactor; on 0.83-stable it has none. Everything else - the subscription lambda, the destructor unsubscribe, the header declaration and token, the pointScaleFactor conversion, and the momentum-end updateStateWithContentOffset() call - is byte-for-byte identical to microsoft#16140.
Contributor
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Andrew Coates (acoates-ms)
changed the title
Andrew Coates (acoates-ms)
approved these changes
Aug 3, 2026
Contributor
|
/azp run |
Contributor
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Performance Test ResultsBranch: ❌ Regressions DetectedButton
✅ Passed152 scenario(s) across 27 suite(s) — no regressionsSectionList
FlatList
TouchableOpacity
ScrollView
TouchableHighlight
Pressable
Modal
Image
ActivityIndicator
Switch
TextInput
View
Text
SectionList.native-perf-test.ts
FlatList.native-perf-test.ts
TouchableHighlight.native-perf-test.ts
TouchableOpacity.native-perf-test.ts
Pressable.native-perf-test.ts
ScrollView.native-perf-test.ts
ActivityIndicator.native-perf-test.ts
TextInput.native-perf-test.ts
Switch.native-perf-test.ts
Button.native-perf-test.ts
Image.native-perf-test.ts
Modal.native-perf-test.ts
View.native-perf-test.ts
Text.native-perf-test.ts
|
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
2 participants
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.
[0.83-stable] Backport #16140: ScrollView Pressables stuck / dead tap on high-DPI displays
Backport of #16140 ("Fix ScrollView Pressables stuck and dead tap on high-DPI displays", merged to
main2026-05-13, commitdf07be9) onto0.83-stable. Original fix by Gordon MacMaster (@gmacmaster); #16140 was itself themaincherry-pick of #16139 ("[0.84] Fix ScrollView Pressables stay stuck and next tap is dead on high-DPI displays", merged to0.84-stable). All credit for the diagnosis and the fix belongs to that work — this PR only carries it back to 0.83. Fixes #16047 ("Scrolling Views not working with touch screen devices") on 0.83.0.83-stabledoes not contain any of it today —PointerRoutedAwayand thepointScaleFactordivision inupdateStateWithContentOffsetboth have zero hits at that ref.Problem
In a Fabric/Composition app on a touch device:
Pressable/TouchableOpacityinside aScrollViewstays stuck in its pressed state after the finger starts a scroll, and a later tap elsewhere can be attributed to that originally-pressed target.pressat all.Root cause
Two independent defects, exactly as diagnosed in #16047 / #16140:
(a)
m_activeToucheskeeps a zombie entry whenScrollViewredirects the pointer.ScrollViewComponentViewcallsVisualInteractionSource::TryRedirectForManipulation, and the OS reassigns the pointer to theInteractionTracker. WinAppSDK does not raisePointerCaptureLoston ourInputPointerSourcefor that path — it raisesPointerRoutedAway, whichCompositionEventHandlernever subscribed to. After the redirect we get noPointerMoved/PointerReleased/PointerCaptureLostfor that pointer, so the active-touch entry is never erased and the pressed Pressable never receives a cancel.(b)
ScrollViewwrites physical pixels into a DIP-typed state field.ScrollViewComponentView::updateStateWithContentOffset()storedm_scrollVisual.ScrollPosition()straight intoScrollViewShadowNode::ConcreteState::Data::contentOffset.ScrollPosition()is in physical pixels — the scroll visual is sizedlayoutMetrics.frame.size.* * pointScaleFactor(seeupdateLayoutMetrics/updateContentVisualSize) — butcontentOffsetis consumed as DIPs. So JSUIManager.measure()over-subtracted the offset bypointScaleFactorafter any scroll on a >100% scale, andPressabilityfiredLEAVE_PRESS_RECTsynchronously insidepressIn, suppressingpress. Every other consumer in the same file already divides bypointScaleFactor—getScrollMetrics(),ScrollInteractionTrackerOwner::ValuesChanged(), andhitTest(); only the state write did not.Additionally,
ScrollMomentumEndwas the one scroll-completion path that did not callupdateStateWithContentOffset()(ScrollBeginDragandScrollEndDragboth do), so the last inertia delta could leavecontentOffsetstale even with the unit fix in place.Fix
Three files, matching #16140:
CompositionEventHandler.cpp/.h— subscribe toInputPointerSource.PointerRoutedAway, addonPointerRoutedAway, which cancels and erases the active touch for that pointer; addm_pointerRoutedAwayTokenand unsubscribe it in the destructor alongside the other pointer tokens.ScrollViewComponentView.cpp— divideScrollPosition()bypointScaleFactor(guarded to1.0fwhen non-positive) before writingcontentOffset. The scrollbar components keep receiving the raw physical-pixel position, which is what they expect.ScrollViewComponentView.cpp— callupdateStateWithContentOffset()at the top of theScrollMomentumEndhandler, before notifying JS.How this differs from #16140 on
main— please readA literal cherry-pick cannot build on
0.83-stable, because #16140 was written on top of infrastructure that only exists onmain.DispatchSynthesizedTouchCancelForActiveTouchandCancelActiveTouchForPointerInternalboth have zero occurrences inCompositionEventHandler.{cpp,h}at0.83-stable, so the merged form ofonPointerRoutedAwaywould not resolve.main,onPointerCaptureLostalready contained an active-touch cancel block, and Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140 refactored it into a sharedCancelActiveTouchForPointerInternal()helper built onDispatchSynthesizedTouchCancelForActiveTouch(). Neither the pre-existing cancel block nor either helper exists on0.83-stable. This backport therefore implementsonPointerRoutedAwayas a standalone function using the 0.83-eraDispatchTouchEvent(TouchEventType::Cancel, ...)API, and looks the entry up with thestd::find_ifoverpair.second.touch.identifierpattern already used byonPointerPressedandonPointerReleasedin this same file. Net effect on the redirect path is identical.onPointerCaptureLostis left untouched here. OnmainFix ScrollView Pressables stuck and dead tap on high-DPI displays #16140 rewrote that function; on0.83-stablethere is nothing there to rewrite, and adding capture-loss cleanup is separate work — it is the subject of my other 0.83-stable PR, #16333, so I have deliberately kept it out of this one so the two review independently. I tested both stacking orders locally withgit apply: on a clean0.83-stablebase each applies at exit 0, and applying one then the other also succeeds at exit 0 (withgit applyreducing context by a line or two, since [0.83] fix(fabric): touch-input reliability — primary-button labeling, guaranteed touch END/CANCEL, phantom active-touch self-heal #16333 inserts a block just insideonPointerCaptureLost's closing brace and this PR appends a new function just after it). There is no semantic overlap — worst case one of us rebases. Happy to sequence them however you prefer.#include "ReactNativeIsland.h"toScrollViewComponentView.cpp. Nothing in the backported code needs it on0.83-stable, so it is omitted.onPointerRoutedAway(// (later taps can then be attributed to that original target).) carried over from themainonPointerCaptureLostcomment, since 0.83-stable has no equivalent comment nearby. Drop it if you would rather the text match Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140 exactly.I diffed the added-line sets against #16140 to confirm this: the only things that differ are the
onPointerRoutedAwaybody, themain-only helper/onPointerCaptureLostrefactor, theReactNativeIsland.hinclude, and that one comment line. Everything else — thePointerRoutedAwaysubscription lambda, the destructor unsubscribe, the header declaration,m_pointerRoutedAwayToken, thepointScaleFactordivision block, and the momentum-end call — is byte-for-byte identical to #16140.Validation
git apply --checkagainst0.83-stableis clean for all three files — exit 0 both strictly and with--recount -C2, verified against the three files exactly as they stand at this ref.UseExperimentalNuget=false, so the patchedMicrosoft.ReactNativeis what the app actually loads — not a prebuilt NuGet. The app carries these hunks as ayarn patchagainst thereact-native-windowsnpm package (npm-package paths map to this repo by adding thevnext/prefix), which is what this PR upstreams.What I have not verified — please weigh these before merging:
0.83-stablebackport specifically. My confidence rests on the added lines being byte-identical to Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140 outside the documented deltas, not on a fresh A/B. If you would rather see that A/B before merging, say so and I will run it; alternatively confirm that Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140's validation is considered sufficient for a same-content backport.0.83-stable. Azure Pipelines on this repo is gated behind a maintainer comment, so I cannot start it myself — a/azp runwould be appreciated.Caveats for reviewers:
onPointerCaptureLostdivergence above is the thing to check. Because 0.83-stable'sonPointerCaptureLosthas no active-touch cleanup, the touch-cancel coverage on this branch after merging only this PR is narrower than onmain: the ScrollView redirect path is fixed, but other system-driven capture losses (focus change, another window stealing input) still leak. That gap is intentional scope separation, not an oversight.std::find_ifoverpair.second.touch.identifierrather thanmain'sm_activeTouches.find(pointerId), purely to match the surrounding 0.83 code. The two are equivalent here (the map key is the pointer id); happy to switch to the plainfind.DispatchSynthesizedTouchCancelForActiveTouch+CancelActiveTouchForPointerInternalon 0.83-stable, say so and I will — it just pulls in more ofmainthan a minimal backport needs, and it collides with [0.83] fix(fabric): touch-input reliability — primary-button labeling, guaranteed touch END/CANCEL, phantom active-touch self-heal #16333.pointScaleFactor > 0.0f ? ... : 1.0fguard is Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140's; keeping it avoids a divide-by-zero ifupdateStateWithContentOffset()ever runs before the firstupdateLayoutMetrics.m_verticalScrollbarComponent->ContentOffset(rawScrollPosition)receives exactly the same value the pre-patch code passed, so scrollbar behaviour is unchanged by construction. (The scrollbar helper stores it and multiplies incoming DIP hit-test points bym_scaleFactor, i.e. it works in physical pixels internally.) Only the shadow-node state is converted.type: patch). Let me know if you would preferprereleaseto match Fix ScrollView Pressables stuck and dead tap on high-DPI displays #16140's own change file.Microsoft Reviewers: Open in CodeFlow