Skip to content
145 changes: 107 additions & 38 deletions src/Reactor/Core/AccessibilityScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ public static partial class AccessibilityScanner
// implementation and its dependency chain.
private static IScanExtension? s_scanExtension;

// Reused per-thread scratch list for BuildContext's child extraction so a
// per-finding Where(...).ToList() is not allocated each call (perf #64).
// Cleared before and after use so it never pins Element references.
[ThreadStatic]
private static List<Element>? t_buildCtxChildren;

/// <summary>
/// Registration hook for a subsystem-specific scanner extension (e.g. the
/// Charting accessibility checker). Installed once when the subsystem
Expand Down Expand Up @@ -175,7 +181,13 @@ private static void Walk(Element? el, ScanContext ctx, List<A11yDiagnostic> find
{
if (el is null or EmptyElement) return;

ctx.Push(el);
// Compute children once and reuse for both sibling-text collection
// (inside Push) and the recursion below. GetChildren does a full
// switch dispatch and (for the single-child arms) allocates a fresh
// array literal on every call, so calling it twice per element
// doubles that cost (perf #62).
var children = GetChildren(el);
ctx.Push(el, children);

// Per-element checks
CheckIconButton(el, ctx, findings);
Expand All @@ -196,7 +208,7 @@ private static void Walk(Element? el, ScanContext ctx, List<A11yDiagnostic> find
CollectLandmark(el, ctx);

// Recurse into children
foreach (var child in GetChildren(el))
foreach (var child in children)
Walk(child, ctx, findings);

ctx.Pop();
Expand Down Expand Up @@ -480,31 +492,43 @@ private static void CheckNoMainLandmark(ScanContext ctx, List<A11yDiagnostic> fi
/// <summary>A11Y_007: Non-sequential TabIndex values (gaps > 1).</summary>
private static void CheckTabIndexGaps(ScanContext ctx, List<A11yDiagnostic> findings)
{
if (ctx.TabIndices.Count < 2) return;
int count = ctx.TabIndices.Count;
if (count < 2) return;

var sorted = ctx.TabIndices.OrderBy(t => t).ToList();
for (int i = 1; i < sorted.Count; i++)
// Snapshot the set into a rented buffer and sort in place instead of
// OrderBy(...).ToList() (which allocates an iterator + a List) — perf #66.
var buffer = global::System.Buffers.ArrayPool<int>.Shared.Rent(count);
try
{
if (sorted[i] - sorted[i - 1] > 1)
ctx.TabIndices.CopyTo(buffer);
Array.Sort(buffer, 0, count);
for (int i = 1; i < count; i++)
{
findings.Add(new A11yDiagnostic
if (buffer[i] - buffer[i - 1] > 1)
{
Id = "A11Y_007",
Severity = "info",
Message = $"TabIndex gap: {sorted[i - 1]} → {sorted[i]}. Non-sequential values may confuse keyboard navigation order",
WcagCriterion = "2.1.1",
ElementType = "(tree-wide)",
Fix = new A11yFixSuggestion
findings.Add(new A11yDiagnostic
{
Modifier = "TabIndex",
SuggestedValue = null,
CodeSnippet = "Renumber TabIndex values sequentially",
},
Context = new A11yContext(),
});
break; // Report only the first gap
Id = "A11Y_007",
Severity = "info",
Message = $"TabIndex gap: {buffer[i - 1]} → {buffer[i]}. Non-sequential values may confuse keyboard navigation order",
WcagCriterion = "2.1.1",
ElementType = "(tree-wide)",
Fix = new A11yFixSuggestion
{
Modifier = "TabIndex",
SuggestedValue = null,
CodeSnippet = "Renumber TabIndex values sequentially",
},
Context = new A11yContext(),
});
break; // Report only the first gap
}
}
}
finally
{
global::System.Buffers.ArrayPool<int>.Shared.Return(buffer);
}
}

/// <summary>A11Y_008: .LabeledBy() references an AutomationId not found in the tree.</summary>
Expand Down Expand Up @@ -584,18 +608,29 @@ private static bool IsAccessibilityHidden(Element el) =>
private static bool IsLikelyEmoji(string? text)
{
if (string.IsNullOrEmpty(text)) return false;
var span = text.AsSpan();
// Single character or very short string that's non-ASCII → likely emoji/icon
if (text.Length <= 2 && text.Any(c => c > 0x2000)) return true;
// Unicode symbol ranges commonly used for icons
return text.Length <= 4 && text.All(c =>
c > 0x2000 || char.IsHighSurrogate(c) || char.IsLowSurrogate(c));
if (span.Length <= 2)
{
foreach (var c in span)
if (c > 0x2000) return true;
}
// Unicode symbol ranges commonly used for icons: short string where
// every char is a high symbol or a surrogate half.
if (span.Length > 4) return false;
foreach (var c in span)
{
if (!(c > 0x2000 || char.IsHighSurrogate(c) || char.IsLowSurrogate(c)))
return false;
}
return true;
}

private static string Truncate(string text, int maxLen) =>
text.Length <= maxLen ? text : text[..(maxLen - 3)] + "...";

private static string[] GetSiblingTexts(ScanContext ctx) =>
ctx.CurrentSiblingTexts.ToArray();
ctx.SiblingTextsCached;

// ════════════════════════════════════════════════════════════════
// Scan context (maintained during tree walk)
Expand All @@ -615,6 +650,14 @@ private sealed class ScanContext : IScanContext
// Current sibling context (texts of siblings in the same container)
public readonly List<string> CurrentSiblingTexts = new();

// Cached array snapshot of CurrentSiblingTexts for the current frame.
// Multiple findings on the same element previously each allocated a
// fresh ToArray() copy; this caches one snapshot per element and
// invalidates it on Push (perf #63). The array is immutable once
// exposed, so sharing it across that element's findings is safe.
private string[]? _siblingTextsCache;
public string[] SiblingTextsCached => _siblingTextsCache ??= CurrentSiblingTexts.ToArray();

public string? CurrentComponent => _stack.Count > 0 ? _stack.Peek().ComponentType : null;

// ── IScanContext (issue #498): exposes the scanner's private helpers to
Expand All @@ -623,7 +666,7 @@ private sealed class ScanContext : IScanContext
bool IScanContext.HasAutomationName(Element el) => AccessibilityScanner.HasAutomationName(el);
A11yContext IScanContext.BuildContext(Element el) => BuildContext(el, AccessibilityScanner.GetSiblingTexts(this));

public void Push(Element el)
public void Push(Element el, IEnumerable<Element?> children)
{
var frame = new ContextFrame();

Expand Down Expand Up @@ -651,15 +694,19 @@ public void Push(Element el)
if (el is ComponentElement comp)
frame.ComponentType = comp.ComponentType?.Name;

// Collect sibling texts from container children
// Collect sibling texts from the already-computed children
// (passed in by Walk so GetChildren runs once per element — #62).
CurrentSiblingTexts.Clear();
foreach (var child in GetChildren(el))
foreach (var child in children)
{
if (child is TextBlockElement t)
CurrentSiblingTexts.Add(t.Content);
else if (child is ButtonElement b && !string.IsNullOrEmpty(b.Label))
CurrentSiblingTexts.Add(b.Label);
}
// Invalidate the per-element sibling-text snapshot; it is
// recomputed lazily on first read for this frame (perf #63).
_siblingTextsCache = null;

_stack.Push(frame);
}
Expand All @@ -674,20 +721,42 @@ public A11yContext BuildContext(Element el, string[] siblingTexts)
{
var frame = _stack.Count > 0 ? _stack.Peek() : new ContextFrame();

// Extract child info
var children = GetChildren(el).Where(c => c is not null and not EmptyElement).ToList();
string? childContent = null;
// Extract child info in a single pass over GetChildren(el),
// replacing the previous Where/Select/OfType LINQ chains (perf #64).
// A reused thread-static scratch list avoids the per-finding
// intermediate List allocation; the only allocation kept is the
// childTypes result array (pre-sized), which the LINQ path also built.
var children = t_buildCtxChildren ??= new List<Element>();
children.Clear();
string? childText = null;
string? childContent = null;
string[]? childTypes = null;

if (children.Count > 0)
// Wrap the scratch populate/use in try/finally so a mid-population
// exception can't leave element refs pinned on the thread-static list
// until the next BuildContext call on this thread (Copilot review).
try
{
foreach (var c in GetChildren(el))
{
if (c is null or EmptyElement) continue;
children.Add(c);
if (childText is null && c is TextBlockElement tb)
childText = tb.Content;
}
Comment on lines +740 to +746

if (children.Count > 0)
{
childTypes = new string[children.Count];
for (int i = 0; i < children.Count; i++)
childTypes[i] = children[i].GetType().Name;
if (children.Count == 1)
childContent = children[0].ToString();
}
}
finally
{
childTypes = children.Select(c => c!.GetType().Name).ToArray();
var firstText = children.OfType<TextBlockElement>().FirstOrDefault();
if (firstText is not null)
childText = firstText.Content;
if (children.Count == 1)
childContent = children[0]!.ToString();
children.Clear();
}

// For ButtonElement, use ContentElement if present
Expand Down
36 changes: 36 additions & 0 deletions src/Reactor/Core/Context.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ namespace Microsoft.UI.Reactor.Core;
public abstract class ContextBase
{
internal abstract object? DefaultValueBoxed { get; }

/// <summary>
/// Compares this context's current value in <paramref name="scope"/> against a
/// previously-recorded boxed value, without boxing the current value (perf #25).
/// Semantically equivalent to <c>object.Equals(scope.Read(this), lastBoxed)</c>.
/// </summary>
internal abstract bool CurrentValueEquals(ContextScope scope, object? lastBoxed);
}

/// <summary>
Expand All @@ -26,4 +33,33 @@ public Context(T defaultValue, [CallerMemberName] string? name = null)
}

internal override object? DefaultValueBoxed => DefaultValue;

internal override bool CurrentValueEquals(ContextScope scope, object? lastBoxed)
{
var current = scope.Read(this); // strongly-typed read — no boxing of the current value
if (current is null)
return lastBoxed is null;
if (lastBoxed is null)
return false;
// For two non-null operands, object.Equals(current, lastBoxed) is exactly
// ReferenceEquals(current, lastBoxed) || current.Equals(lastBoxed)
// Reproduce that without boxing the freshly-read value (perf #25):
// * reference types — keep the ReferenceEquals fast-path: it covers the common
// "same instance, unchanged" context value AND the only case where a
// non-reflexive Equals override would otherwise diverge from object.Equals.
// The fall-back virtual Equals(object) preserves reference-equality change
// detection for a type implementing IEquatable<T> WITHOUT an object.Equals
// override (a value-equality collapse there would skip a required rerender).
// * value types — the constrained call binds to the Equals(object) override, so
// primitives and Nullable<T> compare without a box; a boxed value of a DIFFERENT
// type returns false rather than throwing, exactly as the prior boxed
// object.Equals did. ReferenceEquals is skipped — distinct boxes are never
// reference-equal, and testing it would needlessly box the current value.
// Routing value types through EqualityComparer<T>.Default would instead dispatch to
// IEquatable<T>, diverging from object.Equals for any struct whose IEquatable<T>.Equals
// disagrees with its object.Equals override.
if (!typeof(T).IsValueType && ReferenceEquals(current, lastBoxed))
return true;
return current.Equals(lastBoxed);
}
}
10 changes: 10 additions & 0 deletions src/Reactor/Core/ContextScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ internal void Push(IReadOnlyDictionary<ContextBase, object?> values)
_version++;
}

/// <summary>
/// Single-pair push — avoids allocating a one-entry dictionary on the common
/// typed-context push path (perf #29).
/// </summary>
internal void Push(ContextBase context, object? value)
{
_stack.Add((context, value));
_version++;
}

internal void Pop(int count)
{
_stack.RemoveRange(_stack.Count - count, count);
Expand Down
23 changes: 17 additions & 6 deletions src/Reactor/Core/Reconciler.KeyedItemsBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,20 @@ internal void BindKeyedItemsSource<TItem>(
ArgumentNullException.ThrowIfNull(buildItemView);
ArgumentNullException.ThrowIfNull(requestRerender);

// Refresh the stashed view source on every call. The closure
// captures the *current* `items` + `buildItemView` references, so
// realizations + container refreshes after this point see the
// updated data. Cheap allocation; happens at most once per render.
var viewSource = new ClosureItemViewSource<TItem>(items, buildItemView);
SetItemViewSource(control, viewSource);
// Refresh the stashed view source. The closure captures the *current*
// `items` + `buildItemView` references, so realizations + container
// refreshes after this point see the updated data. When neither
// reference changed since the last bind (parent re-rendered but this
// list's data + builder are stable) the previously stashed source is
// still exact — reuse it and skip both the allocation and the COM
// attached-DP stash write (perf #39).
ClosureItemViewSource<TItem>? viewSource =
GetItemViewSource(control) as ClosureItemViewSource<TItem>;
if (viewSource is null || !viewSource.Matches(items, buildItemView))
{
viewSource = new ClosureItemViewSource<TItem>(items, buildItemView);
SetItemViewSource(control, viewSource);
}

switch (control)
{
Expand Down Expand Up @@ -581,5 +589,8 @@ public ClosureItemViewSource(IReadOnlyList<TItem> items, Func<TItem, int, Elemen

public int ItemCount => _items.Count;
public Element BuildItemView(int index) => _buildItemView(_items[index], index);

public bool Matches(IReadOnlyList<TItem> items, Func<TItem, int, Element> buildItemView)
=> ReferenceEquals(_items, items) && ReferenceEquals(_buildItemView, buildItemView);
}
}
17 changes: 10 additions & 7 deletions src/Reactor/Core/Reconciler.Mount.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ public sealed partial class Reconciler

// Issue #345 — one-time debug warning for HStack/VStack collapsing to 0×0 inside a
// Grid Auto track. Gated so Release builds with the flag off pay a single flag read.
if (global::Microsoft.UI.Reactor.Diagnostics.LayoutFootgunDetector.AlwaysOnInDebug || ReactorFeatureFlags.WarnLayoutFootguns)
// Inspect() is a no-op for non-GridElement, so pre-filter on type to skip the call
// (and its redundant internal flag re-check) for every non-Grid element (perf #40).
if ((global::Microsoft.UI.Reactor.Diagnostics.LayoutFootgunDetector.AlwaysOnInDebug || ReactorFeatureFlags.WarnLayoutFootguns)
&& element is GridElement)
global::Microsoft.UI.Reactor.Diagnostics.LayoutFootgunDetector.Inspect(element);

// Apply inline modifiers after mounting
Expand Down Expand Up @@ -625,13 +628,13 @@ private UIElement MountComponent(ComponentElement compElement, Action requestRer
var node = new ComponentNode
{
Component = component, RenderedElement = null, Element = compElement,
PreviousProps = compElement.Props,
PreviousProps = compElement.Props, Control = wrapper,
};
_componentNodes[wrapper] = node;

// Pass the component's own wrapped rerender to children so that child state
// changes propagate SelfTriggered up through all component ancestors.
var componentRerender = CreateComponentRerender(node, requestRerender);
var componentRerender = GetOrCreateComponentRerender(node, requestRerender);

Element childElement;
try
Expand All @@ -658,13 +661,13 @@ private UIElement MountFuncComponent(FuncElement funcElement, Action requestRere
var wrapper = new Border();
var node = new ComponentNode
{
Context = ctx, RenderedElement = null, Element = funcElement,
Context = ctx, RenderedElement = null, Element = funcElement, Control = wrapper,
};
_componentNodes[wrapper] = node;

// Pass the component's own wrapped rerender to children so that child state
// changes propagate SelfTriggered up through all component ancestors.
var componentRerender = CreateComponentRerender(node, requestRerender);
var componentRerender = GetOrCreateComponentRerender(node, requestRerender);

Element childElement;
try
Expand Down Expand Up @@ -692,13 +695,13 @@ private UIElement MountMemoComponent(MemoElement memoElement, Action requestRere
var node = new ComponentNode
{
Context = ctx, RenderedElement = null, Element = memoElement,
MemoDependencies = memoElement.Dependencies,
MemoDependencies = memoElement.Dependencies, Control = wrapper,
};
_componentNodes[wrapper] = node;

// Pass the component's own wrapped rerender to children so that child state
// changes propagate SelfTriggered up through all component ancestors.
var componentRerender = CreateComponentRerender(node, requestRerender);
var componentRerender = GetOrCreateComponentRerender(node, requestRerender);

Element childElement;
try
Expand Down
Loading
Loading