Skip to content

Perf/pointer event handling #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gmacmaster wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Perf/pointer event handling #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8663466
Use unordered_set for pointer capture tracking to reduce O(n) lookups
gmacmaster Apr 17, 2026
0ef200a
Cache event path to root to avoid repeated tree walks on pointer events
gmacmaster Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,15 @@ void CompositionEventHandler::onCharacterReceived(
}
}

std::vector<winrt::Microsoft::ReactNative::ComponentView> GetTouchableViewsInPathToRoot(
std::vector<winrt::Microsoft::ReactNative::ComponentView> CompositionEventHandler::GetTouchableViewsInPathToRoot(
const winrt::Microsoft::ReactNative::ComponentView &componentView) {
if (componentView) {
auto tag = componentView.Tag();
if (tag == m_cachedEventPathTag) {
return m_cachedEventPath;
}
Comment on lines +669 to +673
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cache has no invalidation on tree mutations

The cache is keyed solely on componentView.Tag(). In Fabric, a parent view can be inserted, removed, or replaced between pointer events (via a state-driven re-render) while the leaf component retains its tag. When that happens the cached path m_cachedEventPath becomes stale, and HandleIncomingPointerEvent / IsPointerWithinInitialTree will traverse the wrong set of ancestors — firing enter/leave events on views that are no longer in the path, or missing views that were added.

A minimal fix is to clear the cache at the start of each top-level event dispatch (e.g., in onPointerMoved, onPointerPressed, etc.) rather than relying on tag identity alone.

}

std::vector<winrt::Microsoft::ReactNative::ComponentView> results;
auto view = componentView;
while (view) {
Expand All @@ -674,6 +681,12 @@ std::vector<winrt::Microsoft::ReactNative::ComponentView> GetTouchableViewsInPat
}
view = view.Parent();
}

if (componentView) {
m_cachedEventPathTag = componentView.Tag();
m_cachedEventPath = results;
}

return results;
}

Expand Down Expand Up @@ -1030,7 +1043,7 @@ void CompositionEventHandler::getTargetPointerArgs(
// to apply the current origin
ptScaled += RootComponentView().layoutMetrics().frame.origin;

if (std::find(m_capturedPointers.begin(), m_capturedPointers.end(), pointerId) != m_capturedPointers.end()) {
if (m_capturedPointers.count(pointerId)) {
assert(m_pointerCapturingComponentTag != -1);
tag = m_pointerCapturingComponentTag;

Expand All @@ -1054,7 +1067,7 @@ void CompositionEventHandler::onPointerCaptureLost(

if (m_pointerCapturingComponentTag) {
Comment on lines 1067 to 1068
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unnecessary copy to unordered_set in onPointerCaptureLost

The comment says "copy array to avoid iterator being invalidated during deletion", which was the original intent. Now that m_capturedPointers is already an unordered_set, copying it into another unordered_set is still correct but wasteful. A plain std::vector<PointerId> snapshot is lighter — it just needs to be iterable, not to support O(1) lookup:

Suggested change
if (m_pointerCapturingComponentTag) {
std::vector<PointerId> capturedPointers(m_capturedPointers.begin(), m_capturedPointers.end());

// copy array to avoid iterator being invalidated during deletion
std::vector<PointerId> capturedPointers = m_capturedPointers;
std::unordered_set<PointerId> capturedPointers = m_capturedPointers;

for (auto pointerId : capturedPointers) {
releasePointerCapture(pointerId, m_pointerCapturingComponentTag);
Expand Down Expand Up @@ -1322,7 +1335,7 @@ bool CompositionEventHandler::CapturePointer(
}

m_pointerCapturingComponentTag = tag;
m_capturedPointers.push_back(pointer.PointerId());
m_capturedPointers.insert(pointer.PointerId());
return true;
}

Expand All @@ -1337,11 +1350,9 @@ bool CompositionEventHandler::releasePointerCapture(PointerId pointerId, faceboo
bool result = false;

if (m_pointerCapturingComponentTag == tag) {
auto it = std::find(m_capturedPointers.begin(), m_capturedPointers.end(), pointerId);
if (it == m_capturedPointers.end()) {
if (m_capturedPointers.erase(pointerId) == 0) {
return false;
}
m_capturedPointers.erase(it);

if (std::shared_ptr<FabricUIManager> fabricuiManager =
::Microsoft::ReactNative::FabricUIManager::FromProperties(m_context.Properties())) {
Expand All @@ -1352,7 +1363,7 @@ bool CompositionEventHandler::releasePointerCapture(PointerId pointerId, faceboo
->OnPointerCaptureLost();
}

if (m_capturedPointers.size() == 0) {
if (m_capturedPointers.empty()) {
m_pointerCapturingComponentTag = -1;
return true;
}
Expand Down
12 changes: 10 additions & 2 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <winrt/Windows.Devices.Input.h>
#include <optional>
#include <set>
#include <unordered_set>

namespace winrt {
using namespace Windows::UI;
Expand Down Expand Up @@ -141,7 +142,7 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
ReactTaggedView initialComponentView{nullptr};
};

static bool IsPointerWithinInitialTree(const ActiveTouch &activeTouch) noexcept;
bool IsPointerWithinInitialTree(const ActiveTouch &activeTouch) noexcept;
static bool IsEndishEventType(TouchEventType eventType) noexcept;
static const char *PointerTypeCStringFromUITouchType(UITouchType type) noexcept;
static facebook::react::PointerEvent CreatePointerEventFromActiveTouch(
Expand All @@ -150,6 +151,9 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
static void
UpdateActiveTouch(ActiveTouch &activeTouch, facebook::react::Point ptScaled, facebook::react::Point ptLocal) noexcept;

std::vector<winrt::Microsoft::ReactNative::ComponentView> GetTouchableViewsInPathToRoot(
const winrt::Microsoft::ReactNative::ComponentView &componentView);

void UpdateCursor() noexcept;
void SetCursor(facebook::react::Cursor cursor, HCURSOR hcur) noexcept;

Expand All @@ -161,12 +165,16 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
winrt::Microsoft::ReactNative::ReactContext m_context;

facebook::react::Tag m_pointerCapturingComponentTag{-1}; // Component that has captured input
std::vector<PointerId> m_capturedPointers;
std::unordered_set<PointerId> m_capturedPointers;
HCURSOR m_hcursor{nullptr};
bool m_hcursorOwned{false}; // If we create the cursor, so we need to destroy it
facebook::react::Cursor m_currentCursor{facebook::react::Cursor::Auto};
winrt::Microsoft::UI::Input::InputCursor m_inputCursor{nullptr};

// Single-entry cache for GetTouchableViewsInPathToRoot to avoid repeated tree walks
facebook::react::Tag m_cachedEventPathTag{-1};
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Cache sentinel -1 collides with valid tag values

m_cachedEventPathTag is initialised to -1. If any component view legitimately returns Tag() == -1 (a value that is also used as a sentinel elsewhere in this file, e.g. m_pointerCapturingComponentTag{-1}), the very first call to GetTouchableViewsInPathToRoot with that view would hit the cache and return an empty vector instead of computing the real path.

Using std::optional<facebook::react::Tag> for the key makes the uninitialised state unambiguous:

// .h
std::optional<facebook::react::Tag> m_cachedEventPathTag;
std::vector<winrt::Microsoft::ReactNative::ComponentView> m_cachedEventPath;
// .cpp — cache-read guard
if (componentView) {
  auto tag = componentView.Tag();
  if (m_cachedEventPathTag.has_value() && tag == *m_cachedEventPathTag) {
    return m_cachedEventPath;
  }
}
// ...
if (componentView) {
  m_cachedEventPathTag = componentView.Tag();
  m_cachedEventPath = results;
}

std::vector<winrt::Microsoft::ReactNative::ComponentView> m_cachedEventPath;

winrt::event_token m_pointerPressedToken;
winrt::event_token m_pointerReleasedToken;
winrt::event_token m_pointerMovedToken;
Expand Down