Skip to content

Commit df07be9

Browse files
authored
Fix ScrollView Pressables stuck and dead tap on high-DPI displays (#16140)
* fix scroll touch scaling issue * Refactor #16047 Fix A to use InputPointerSource.PointerRoutedAway * format
1 parent 528d52b commit df07be9

4 files changed

Lines changed: 110 additions & 22 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Fix #16047: 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: (1) VisualInteractionSource::TryRedirectForManipulation does not deliver PointerCaptureLost for the redirected pointer, leaving a zombie entry in CompositionEventHandler::m_activeTouches — now resolved by synthesizing a touchcancel from the InputPointerSource.PointerRoutedAway event, which fires reliably on the redirect path; and (2) ScrollViewComponentView::updateStateWithContentOffset wrote the raw physical-pixel ScrollPosition into ScrollViewShadowNode state's contentOffset, which Fabric layout treats as DIPs, so JS UIManager.measure() over-subtracted the offset by pointScaleFactor after any scroll on a >100% display, causing Pressability to fire LEAVE_PRESS_RECT synchronously and suppress press — now divides by pointScaleFactor to match the JS event-emitter paths in the same file.",
4+
"packageName": "react-native-windows",
5+
"email": "gordomacmaster@gmail.com",
6+
"dependentChangeType": "patch"
7+
}

vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,28 @@ void CompositionEventHandler::Initialize() noexcept {
224224
}
225225
});
226226

227+
// Issue #16047: when ScrollView calls VisualInteractionSource::TryRedirectForManipulation
228+
// and the OS hands the pointer over to the InteractionTracker, WinAppSDK
229+
// does not fire PointerCaptureLost on this source — but it does fire
230+
// PointerRoutedAway. Treat it the same way as captureloss: cancel any
231+
// active touch RN is tracking for this pointer so Pressables don't get
232+
// stuck in their pressed state.
233+
m_pointerRoutedAwayToken =
234+
pointerSource.PointerRoutedAway([wkThis = weak_from_this()](
235+
winrt::Microsoft::UI::Input::InputPointerSource const &,
236+
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
237+
if (auto strongThis = wkThis.lock()) {
238+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
239+
if (strongThis->SurfaceId() == -1)
240+
return;
241+
242+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
243+
args.CurrentPoint(), strongRootView.ScaleFactor());
244+
strongThis->onPointerRoutedAway(pp, args.KeyModifiers());
245+
}
246+
}
247+
});
248+
227249
m_pointerWheelChangedToken =
228250
pointerSource.PointerWheelChanged([wkThis = weak_from_this()](
229251
winrt::Microsoft::UI::Input::InputPointerSource const &,
@@ -369,6 +391,7 @@ CompositionEventHandler::~CompositionEventHandler() {
369391
pointerSource.PointerReleased(m_pointerReleasedToken);
370392
pointerSource.PointerMoved(m_pointerMovedToken);
371393
pointerSource.PointerCaptureLost(m_pointerCaptureLostToken);
394+
pointerSource.PointerRoutedAway(m_pointerRoutedAwayToken);
372395
pointerSource.PointerWheelChanged(m_pointerWheelChangedToken);
373396
pointerSource.PointerExited(m_pointerExitedToken);
374397
auto keyboardSource = winrt::Microsoft::UI::Input::InputKeyboardSource::GetForIsland(island);
@@ -1117,24 +1140,49 @@ void CompositionEventHandler::onPointerCaptureLost(
11171140
m_pointerCapturingComponentTag = -1;
11181141
}
11191142

1120-
// Also cancel any active touch for the specific pointer that lost capture, even
1121-
// when no JS-level CapturePointer was ever issued. This handles ScrollView (and
1122-
// any other VisualInteractionSource) calling TryRedirectForManipulation: the OS
1123-
// reassigns the pointer to the InteractionTracker, fires PointerCaptureLost, and
1124-
// then stops delivering PointerMoved/PointerReleased to us. Without this cleanup
1125-
// m_activeTouches keeps a zombie entry whose target is the originally-pressed
1126-
// Pressable, leaving it visually pressed and causing later taps to be attributed
1127-
// to that original target. If the entry was already cleared above (for a JS-level
1128-
// capture) or by onPointerReleased running first, the find() is a no-op.
1129-
PointerId pointerId = pointerPoint.PointerId();
1143+
// Defense-in-depth cleanup for the specific pointer that lost capture, even
1144+
// when no JS-level CapturePointer was ever issued. The ScrollView
1145+
// TryRedirectForManipulation path comes in via PointerRoutedAway, not
1146+
// PointerCaptureLost (see onPointerRoutedAway and issue #16047), so this
1147+
// path covers the remaining system-driven losses (focus change, another
1148+
// window stealing input, system back gesture, etc.).
1149+
CancelActiveTouchForPointerInternal(pointerPoint.PointerId(), pointerPoint, keyModifiers);
1150+
}
1151+
1152+
void CompositionEventHandler::onPointerRoutedAway(
1153+
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
1154+
winrt::Windows::System::VirtualKeyModifiers keyModifiers) noexcept {
1155+
if (SurfaceId() == -1)
1156+
return;
1157+
1158+
// Issue #16047: WinAppSDK fires PointerRoutedAway when the OS hands the
1159+
// pointer to another InputPointerSource — most importantly for us, when
1160+
// ScrollView calls VisualInteractionSource::TryRedirectForManipulation and
1161+
// the InteractionTracker takes the gesture for scrolling. We never get
1162+
// PointerMoved / PointerReleased / PointerCaptureLost for that pointer
1163+
// afterwards, so without this cleanup m_activeTouches keeps a zombie entry
1164+
// and the originally-pressed Pressable stays stuck in its pressed state.
1165+
CancelActiveTouchForPointerInternal(pointerPoint.PointerId(), pointerPoint, keyModifiers);
1166+
}
1167+
1168+
bool CompositionEventHandler::CancelActiveTouchForPointerInternal(
1169+
PointerId pointerId,
1170+
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
1171+
winrt::Windows::System::VirtualKeyModifiers keyModifiers) noexcept {
11301172
auto activeTouch = m_activeTouches.find(pointerId);
1131-
if (activeTouch != m_activeTouches.end()) {
1132-
ActiveTouch cancelledTouchCopy = std::move(activeTouch->second);
1133-
m_activeTouches.erase(activeTouch);
1134-
if (cancelledTouchCopy.eventEmitter) {
1135-
DispatchSynthesizedTouchCancelForActiveTouch(cancelledTouchCopy, pointerPoint, keyModifiers);
1136-
}
1173+
if (activeTouch == m_activeTouches.end()) {
1174+
return false;
1175+
}
1176+
1177+
ActiveTouch cancelledTouchCopy = std::move(activeTouch->second);
1178+
m_activeTouches.erase(activeTouch);
1179+
1180+
if (!cancelledTouchCopy.eventEmitter) {
1181+
return false;
11371182
}
1183+
1184+
DispatchSynthesizedTouchCancelForActiveTouch(cancelledTouchCopy, pointerPoint, keyModifiers);
1185+
return true;
11381186
}
11391187

11401188
void CompositionEventHandler::onPointerMoved(

vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
6666
void onPointerCaptureLost(
6767
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
6868
winrt::Windows::System::VirtualKeyModifiers keyModifiers) noexcept;
69+
void onPointerRoutedAway(
70+
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
71+
winrt::Windows::System::VirtualKeyModifiers keyModifiers) noexcept;
6972
void onKeyDown(const winrt::Microsoft::ReactNative::Composition::Input::KeyRoutedEventArgs &args) noexcept;
7073
void onKeyUp(const winrt::Microsoft::ReactNative::Composition::Input::KeyRoutedEventArgs &args) noexcept;
7174
void onCharacterReceived(
@@ -156,6 +159,13 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
156159
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
157160
winrt::Windows::System::VirtualKeyModifiers keyModifiers);
158161

162+
// Look up the active touch for pointerId, erase it, and dispatch cancel events.
163+
// Returns true iff a touch was found and cancel events were dispatched.
164+
bool CancelActiveTouchForPointerInternal(
165+
PointerId pointerId,
166+
const winrt::Microsoft::ReactNative::Composition::Input::PointerPoint &pointerPoint,
167+
winrt::Windows::System::VirtualKeyModifiers keyModifiers) noexcept;
168+
159169
std::vector<winrt::Microsoft::ReactNative::ComponentView> GetTouchableViewsInPathToRoot(
160170
const winrt::Microsoft::ReactNative::ComponentView &componentView);
161171

@@ -186,6 +196,7 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
186196
winrt::event_token m_pointerMovedToken;
187197
winrt::event_token m_pointerWheelChangedToken;
188198
winrt::event_token m_pointerCaptureLostToken;
199+
winrt::event_token m_pointerRoutedAwayToken;
189200
winrt::event_token m_pointerExitedToken;
190201
winrt::event_token m_keyDownToken;
191202
winrt::event_token m_keyUpToken;

vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cpp

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <functional>
2424
#include "ContentIslandComponentView.h"
2525
#include "JSValueReader.h"
26+
#include "ReactNativeIsland.h"
2627
#include "RootComponentView.h"
2728

2829
namespace winrt::Microsoft::ReactNative::Composition::implementation {
@@ -849,13 +850,27 @@ void ScrollViewComponentView::updateStateWithContentOffset() noexcept {
849850
return;
850851
}
851852

852-
auto scrollPosition = m_scrollVisual.ScrollPosition();
853-
m_verticalScrollbarComponent->ContentOffset(scrollPosition);
854-
m_horizontalScrollbarComponent->ContentOffset(scrollPosition);
855-
856-
m_state->updateState([scrollPosition](const facebook::react::ScrollViewShadowNode::ConcreteState::Data &data) {
853+
// Issue #16047: m_scrollVisual.ScrollPosition() returns the InteractionTracker
854+
// position in PHYSICAL pixels (the visual is sized as
855+
// layoutMetrics.frame.size.* * pointScaleFactor — see updateLayoutMetrics /
856+
// updateContentVisualSize) but ScrollViewShadowNode state's contentOffset is
857+
// in DIPs. Without the conversion, JS UIManager.measure() over-subtracts by
858+
// pointScaleFactor on non-100% display scales, leaving Pressables inside a
859+
// scrolled ScrollView with stale page-space bounds that don't contain the
860+
// touch — Pressability fires LEAVE_PRESS_RECT inside pressIn and suppresses
861+
// press. The JS-event-emitter paths in this file (see lines using
862+
// args.Position() / pointScaleFactor) already do this division.
863+
auto rawScrollPosition = m_scrollVisual.ScrollPosition();
864+
const float pointScaleFactor = m_layoutMetrics.pointScaleFactor > 0.0f ? m_layoutMetrics.pointScaleFactor : 1.0f;
865+
facebook::react::Point contentOffsetDips{
866+
rawScrollPosition.x / pointScaleFactor, rawScrollPosition.y / pointScaleFactor};
867+
868+
m_verticalScrollbarComponent->ContentOffset(rawScrollPosition);
869+
m_horizontalScrollbarComponent->ContentOffset(rawScrollPosition);
870+
871+
m_state->updateState([contentOffsetDips](const facebook::react::ScrollViewShadowNode::ConcreteState::Data &data) {
857872
auto newData = data;
858-
newData.contentOffset = {scrollPosition.x, scrollPosition.y};
873+
newData.contentOffset = contentOffsetDips;
859874
return std::make_shared<facebook::react::ScrollViewShadowNode::ConcreteState::Data const>(newData);
860875
});
861876
}
@@ -1389,6 +1404,13 @@ winrt::Microsoft::ReactNative::Composition::Experimental::IVisual ScrollViewComp
13891404
[this](
13901405
winrt::IInspectable const & /*sender*/,
13911406
winrt::Microsoft::ReactNative::Composition::Experimental::IScrollPositionChangedArgs const &args) {
1407+
// Issue #16047: push the FINAL settled scroll position into Fabric's
1408+
// shadow tree before notifying JS. The per-frame ScrollPositionChanged
1409+
// updates can drop the last inertia delta, leaving contentOffset stale
1410+
// and JS UIManager.measure() returning pre-settle-relative bounds.
1411+
// ScrollEndDrag / ScrollBeginDrag already call this; momentum-end was
1412+
// the missing completion path.
1413+
updateStateWithContentOffset();
13921414
auto eventEmitter = GetEventEmitter();
13931415
if (eventEmitter) {
13941416
auto scrollMetrics = getScrollMetrics(eventEmitter, args);

0 commit comments

Comments
 (0)