[perf] hit testing and scroll

[gmacmaster]

No description provided.

[greptile-apps]

Greptile Summary

This PR introduces two performance optimisations for hit-testing and scroll: a m_childrenCache vector in CompVisualImpl to make GetAt O(1) (previously an O(n) WinRT iterator walk), and a m_previousSnapPositions guard in CompScrollerVisual to skip redundant snap-point reconfiguration when the set hasn't changed. Both changes are logically sound, and the cache is consistently updated through the sole InsertAt/Remove mutation paths.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/hardening suggestions with no blocking defects.

Both changes are logically correct: the cache is consistently maintained through the only two mutation paths, and the snap-point guard uses exact equality on a sorted, deduplicated float vector computed from the same inputs. The two P2 notes (unchecked index assertion and complexity-comment accuracy) are hardening suggestions, not correctness issues.

No files require special attention.

Important Files Changed

Filename	Overview
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionContextHelper.cpp	Introduces m_childrenCache in CompVisualImpl to make GetAt O(1) instead of O(n); adds snap-point deduplication guard in CompScrollerVisual. Cache logic is correct but GetAt has an unchecked index access.
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionViewComponentView.cpp	Refactors anyHitTestHelper to collect children into a local vector before iterating in reverse; reduces algorithmic complexity but adds per-call allocation and COM ref-count overhead whose net benefit depends on actual IVector backing type.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph CompVisualImpl["CompVisualImpl (updated)"]
        IA["InsertAt(visual, index)"]
        RM["Remove(visual)"]
        GA["GetAt(index)"]
        Cache["m_childrenCache (std::vector)"]
        WRT["WinRT ContainerVisual.Children()"]

        IA -->|"InsertAtBottom / InsertAbove"| WRT
        IA -->|"push_back / insert at index"| Cache
        RM -->|"Children().Remove()"| WRT
        RM -->|"find_if + erase"| Cache
        GA -->|"operator[] — O(1)"| Cache
    end

    subgraph CompScrollerVisual["CompScrollerVisual (updated)"]
        CSP["ConfigureSnapPoints()"]
        Prev["m_previousSnapPositions"]
        IT["InteractionTracker resting values"]

        CSP -->|"sort + unique"| Sorted["sorted snapPositions"]
        Sorted -->|"== m_previousSnapPositions?"| Prev
        Prev -->|"unchanged → early return"| Skip["skip reconfiguration"]
        Prev -->|"changed → update + continue"| IT
    end

    subgraph anyHitTestHelper["ComponentView::anyHitTestHelper (updated)"]
        AH["anyHitTestHelper()"]
        Vec["local std::vector (children copy)"]
        Rev["reverse iterate → hitTest()"]

        AH -->|"range-for O(n) copy"| Vec
        Vec --> Rev
    end

Loading

_{Reviews (1): Last reviewed commit: "Skip snap scroll reconfiguration when sn..." | Re-trigger Greptile}

[greptile-apps]

Misleading complexity comment and potential allocation overhead

The comment says "repeated O(n) IVector::GetAt calls", implying the original loop was O(n²). In practice, WinRT IVector<T>::GetAt on a vector-backed collection (the common case) is O(1), so the original loop was already O(n) total. The new code introduces a per-call heap allocation and n COM AddRef/Release operations on a hot hit-testing path. For shallow trees (the common case in React Native views) this could be slower. Consider profiling to confirm this is a net win before committing to the extra overhead.

[greptile-apps]

Unchecked vector index in noexcept function

operator[] is undefined behaviour when index >= m_childrenCache.size(). Because the method is marked noexcept you can't use .at() here (that would call std::terminate on OOB), so the right fix is an explicit guard. The original WinRT-iterator path would also crash on an invalid index, so the risk level is unchanged, but since this PR is already touching this method it's a good moment to add an assert:

  winrt::Microsoft::ReactNative::Composition::Experimental::IVisual GetAt(uint32_t index) noexcept {
    assert(index < m_childrenCache.size());
    return m_childrenCache[index];
  }