[0.84] Fix ScrollView Pressables stay stuck and next tap is dead on high-DPI displays#16139
Conversation
Are we sure that this isn't something like TryRedirectForManipulation returning false in this case? There is also https://learn.microsoft.com/en-us/windows/windows-app-sdk/api/winrt/microsoft.ui.input.inputpointersource.pointerroutedaway?view=windows-app-sdk-1.8 ? Maybe that gets fired instead? (Just throwing out ideas, since its obviously not ideal to be faking pointer capture lost when we dont get an event. |
@acoates-ms Thank you for the push back! Didn't know about PointerRoutedAway. I added a PointerRoutedAway subscriber alongside the existing PointerCaptureLost one as a verification step and it fires reliably on the TryRedirectForManipulation codepath, so I've ripped the entire InteractingStateEntered plumbing out and routed Fix A through it instead. The new shape mirrors onPointerCaptureLost: Initialize() adds one more InputPointerSource event handler, onPointerRoutedAway calls a shared CancelActiveTouchForPointerInternal helper Fix B is unchanged. I have updated the pr description to match the new functionality |
|
Much better! - Thanks I appreciate that you want the fix in 0.84 asap, but lets make sure to also get a main version so that the change doesn't get lost going forward. |
|
/azp run |
Of course, I'll bring this over and create a pr to main right now |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Pr created #16140 |
Addresses a "works on my computer" issue caused by display scaling.
Pressables inside ScrollView remained stuck in the pressed state after a touch-driven scroll, and on non-100% Windows display scales the next tap on a row would not register
press.Two underlying causes were addressed:
VisualInteractionSource::TryRedirectForManipulationdoes not deliverPointerCaptureLostfor the redirected pointer, leaving a zombie entry inCompositionEventHandler::m_activeTouches— now resolved by synthesizing a touchcancel from theInputPointerSource.PointerRoutedAwayevent, which fires reliably on the redirect path;ScrollViewComponentView::updateStateWithContentOffsetwrote the raw physical-pixelScrollPositionintoScrollViewShadowNodestate'scontentOffset, which Fabric layout treats as DIPs, so JSUIManager.measure()over-subtracted the offset bypointScaleFactorafter any scroll on a >100% display, causing Pressability to fireLEAVE_PRESS_RECTsynchronously and suppress press — now divides bypointScaleFactorto match the JS event-emitter paths in the same file.Type of Change
Why
On Fabric, tapping a
Pressableinside aScrollView, then scrolling thatScrollViewwith a touch drag, leaves behind two compounding bugs:Pressablestays visually stuck in its pressed state —onPressOutnever fires because React Native is never told the touch ended.ScrollViewalso fails to register apress. The touch visibly lands on the row, but the press is silently dismissed aspressIn→pressOut(0ms)with nopressevent.Both symptoms compound into the "stuck button + dead column" behavior reported in the issue. The previous fix in #16106 only addressed (1) via
onPointerCaptureLost, which doesn't fire on theInteractionTrackerredirect codepath; this PR adds the missing redirect-path handler and addresses (2) for the first time.Resolves #16047
What
Two underlying causes, each with a focused fix.
Fix A — synthesized touchcancel from
InputPointerSource.PointerRoutedAway(the "stuck Pressable" symptom)When
ScrollViewcallsVisualInteractionSource::TryRedirectForManipulationto hand the active pointer over to the OSInteractionTracker, WinAppSDK does NOT firePointerCaptureLostfor the redirected pointer. It does, however, fireInputPointerSource.PointerRoutedAway— verified empirically on the repro environment (thanks to @acoates-ms for the pointer to that event). Without any cleanup hook on this path,CompositionEventHandler::m_activeTouchesretained a zombie entry whose target is the originally-pressedPressable, and JS never received atouchcancel/touchend.The fix subscribes to
PointerRoutedAwayon the sameInputPointerSourcewe already use for the other pointer events, and routes it into a newonPointerRoutedAwaymethod that calls a sharedCancelActiveTouchForPointerInternalhelper (also extracted out ofonPointerCaptureLostso both paths use the same implementation). The helper looks up the active touch byPointerId, removes it, and dispatches the synthesizedtouchcancel. No new IDL surface, no new public API, no new event flowing across module boundaries — just one moreInputPointerSourceevent handler on the same source we already use, mirroring the existingPointerCaptureLostshape.Fix B — DIP conversion in
updateStateWithContentOffset()(the "next tap is dead at non-100% DPI" symptom)m_scrollVisual.ScrollPosition()returns theInteractionTrackerposition in physical pixels (the scroll visual is sized aslayoutMetrics.frame.size.* * pointScaleFactor— seeupdateLayoutMetrics/updateContentVisualSize), butScrollViewShadowNode::ConcreteState::contentOffsetis in DIPs. The other consumers ofargs.Position()inScrollViewComponentView.cpp(the throttledScrollPositionChangedhandler and the embedded-element handler) already divide bypointScaleFactorbefore publishing to the JS event emitter — only the authoritative state-write path,updateStateWithContentOffset, was missing the same conversion.At any non-100% Windows display scale, this caused JS
UIManager.measure()to over-subtract the scroll offset bypointScaleFactor×. The touch still landed at the correct visual position (native composition hit-testing uses the raw physical scroll value consistently — seeScrollViewComponentView::hitTest), but Pressability's_responderRegionmeasurement reported pre-scroll-relative bounds that didn't contain the touch coordinates.LEAVE_PRESS_RECTthen fired synchronously insidepressIn→pressOut(0ms)→ nopress.The fix is one conversion in
updateStateWithContentOffset()— divideScrollPosition()bypointScaleFactorbefore storing intocontentOffset. Plus one related fix in the same file:ScrollMomentumEndnow also callsupdateStateWithContentOffset()so the final settled scroll position isn't dropped by the per-frame throttle. It was the only completion path that was missing this call;ScrollEndDragandScrollBeginDragalready had it.Important Files Changed
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cppInputPointerSource.PointerRoutedAway; addsonPointerRoutedAway; extractsCancelActiveTouchForPointerInternalshared by bothonPointerCaptureLostandonPointerRoutedAway.vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.honPointerRoutedAway,m_pointerRoutedAwayToken, and the shared internal helper.vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cppupdateStateWithContentOffset()and call it fromScrollMomentumEndso the final settled offset isn't lost.change/react-native-windows-d89877b5-…jsonprerelease/patch changeset.Screenshots
Before:
dropped_touch.mp4
After:
fixed_touch.mp4
Testing
No automated regression test. Both bugs require a real
InteractionTrackerdriven by a real WinAppSDKInputPointerSourceat a >100% display scale, which is not currently mockable in the existing integration-test harness without significant new scaffolding.Manual smoke test on the original 150%-scaled repro environment from #16047:
e2e-test-app-fabric:yarn windowsfrompackages/e2e-test-app-fabric.Pressablerows (e.g. the RNTester component browser).onPressfires, the row highlights briefly, no stuck pressed state.onPressfires for the new row. (Was broken before Fix B.)yarn windowsbuilds clean against the final sources.Changelog
Yes.