diff --git a/src/Reactor/Core/Element.cs b/src/Reactor/Core/Element.cs index 92918ea50..fdcbd4bec 100644 --- a/src/Reactor/Core/Element.cs +++ b/src/Reactor/Core/Element.cs @@ -276,13 +276,65 @@ public Microsoft.UI.Reactor.Elements.ResourceOverrides? ResourceOverrides /// internal Element SetAttached(object data) { - var dict = Attached is not null - ? new Dictionary(Attached) - : new Dictionary(); - dict[data.GetType()] = data; + var key = data.GetType(); + var existing = Attached; + // #155 — the overwhelmingly common case is a single attached value per + // element (e.g. one .Grid(row, column) per cell, re-applied every render). + // Store it in a tiny single-entry dictionary instead of allocating a full + // Dictionary (bucket + entry arrays) for every cell, every + // frame. A real Dictionary is only materialized when an element actually + // carries two or more distinct attached types (rare). + if (existing is null || (existing.Count == 1 && existing.ContainsKey(key))) + { + return this with { Attached = new SingleAttachedDictionary(key, data) }; + } + var dict = new Dictionary(existing.Count + 1); + foreach (var kv in existing) dict[kv.Key] = kv.Value; + dict[key] = data; return this with { Attached = dict }; } + /// + /// Minimal immutable single-entry + /// used by for the common one-attached-value case + /// (spec 047 §4.4 hot path). Avoids the per-cell + /// allocation while remaining a drop-in for every consumer + /// (, ), which only ever + /// call Count, TryGetValue, ContainsKey, or enumerate. + /// + internal sealed class SingleAttachedDictionary : IReadOnlyDictionary + { + private readonly Type _key; + private readonly object _value; + internal SingleAttachedDictionary(Type key, object value) + { + _key = key; + _value = value; + } + public object this[Type key] => key == _key ? _value : throw new KeyNotFoundException(); + internal Type SingleKey => _key; + internal object SingleValue => _value; + public IEnumerable Keys { get { yield return _key; } } + public IEnumerable Values { get { yield return _value; } } + public int Count => 1; + public bool ContainsKey(Type key) => key == _key; + public bool TryGetValue(Type key, out object value) + { + if (key == _key) + { + value = _value; + return true; + } + value = null!; + return false; + } + public IEnumerator> GetEnumerator() + { + yield return new KeyValuePair(_key, _value); + } + global::System.Collections.IEnumerator global::System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + /// /// Convenience: implicitly convert a string to a TextBlockElement. /// Allows writing: VStack("Hello", "World") instead of VStack(Text("Hello"), Text("World")) @@ -374,7 +426,7 @@ internal static bool ShallowEquals(Element a, Element b) && ta.Weight == tb.Weight && ta.FontStyle == tb.FontStyle && ta.HorizontalAlignment == tb.HorizontalAlignment - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), // Callbacks (OnClick, OnChanged, etc.) are intentionally not compared: // dispatch goes through the Tag trampoline, and the Update.cs skip path @@ -386,26 +438,26 @@ internal static bool ShallowEquals(Element a, Element b) && ba.IsEnabled == bb.IsEnabled && ba.ContentElement is null && bb.ContentElement is null && CommandBindings.CommandsEqual(ba.Command, bb.Command) - && ReferenceEquals(ba.Setters, bb.Setters), + && SettersEqual(ba.Setters, bb.Setters), (HyperlinkButtonElement ha, HyperlinkButtonElement hb) => ha.Content == hb.Content && ha.NavigateUri == hb.NavigateUri && CommandBindings.CommandsEqual(ha.Command, hb.Command) - && ReferenceEquals(ha.Setters, hb.Setters), + && SettersEqual(ha.Setters, hb.Setters), (RepeatButtonElement ra, RepeatButtonElement rb) => ra.Label == rb.Label && ra.Delay == rb.Delay && ra.Interval == rb.Interval && CommandBindings.CommandsEqual(ra.Command, rb.Command) - && ReferenceEquals(ra.Setters, rb.Setters), + && SettersEqual(ra.Setters, rb.Setters), (ToggleButtonElement ta, ToggleButtonElement tb) => ta.Label == tb.Label && ta.IsChecked == tb.IsChecked && CommandBindings.CommandsEqual(ta.Command, tb.Command) - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), // issue #153 (L1) — extend the command fast-path arm to Split / ToggleSplit so all six // command-capable buttons memoize consistently. Flyout uses reference-equality (matches @@ -415,14 +467,14 @@ internal static bool ShallowEquals(Element a, Element b) sa.Label == sb.Label && ReferenceEquals(sa.Flyout, sb.Flyout) && CommandBindings.CommandsEqual(sa.Command, sb.Command) - && ReferenceEquals(sa.Setters, sb.Setters), + && SettersEqual(sa.Setters, sb.Setters), (ToggleSplitButtonElement tsa, ToggleSplitButtonElement tsb) => tsa.Label == tsb.Label && tsa.IsChecked == tsb.IsChecked && ReferenceEquals(tsa.Flyout, tsb.Flyout) && CommandBindings.CommandsEqual(tsa.Command, tsb.Command) - && ReferenceEquals(tsa.Setters, tsb.Setters), + && SettersEqual(tsa.Setters, tsb.Setters), (SliderElement sa, SliderElement sb) => sa.Value == sb.Value @@ -430,27 +482,27 @@ internal static bool ShallowEquals(Element a, Element b) && sa.Max == sb.Max && sa.StepFrequency == sb.StepFrequency && sa.Header == sb.Header - && ReferenceEquals(sa.Setters, sb.Setters), + && SettersEqual(sa.Setters, sb.Setters), (ToggleSwitchElement ta, ToggleSwitchElement tb) => ta.IsOn == tb.IsOn && ta.OnContent == tb.OnContent && ta.OffContent == tb.OffContent && ta.Header == tb.Header - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), (CheckBoxElement ca, CheckBoxElement cb) => ca.IsChecked == cb.IsChecked && ca.Label == cb.Label && ca.IsThreeState == cb.IsThreeState && ca.CheckedState == cb.CheckedState - && ReferenceEquals(ca.Setters, cb.Setters), + && SettersEqual(ca.Setters, cb.Setters), (RadioButtonElement ra, RadioButtonElement rb) => ra.Label == rb.Label && ra.IsChecked == rb.IsChecked && ra.GroupName == rb.GroupName - && ReferenceEquals(ra.Setters, rb.Setters), + && SettersEqual(ra.Setters, rb.Setters), (ComboBoxElement ca, ComboBoxElement cb) => ReferenceEquals(ca.Items, cb.Items) @@ -459,7 +511,7 @@ internal static bool ShallowEquals(Element a, Element b) && ca.Header == cb.Header && ca.IsEditable == cb.IsEditable && ReferenceEquals(ca.ItemElements, cb.ItemElements) - && ReferenceEquals(ca.Setters, cb.Setters), + && SettersEqual(ca.Setters, cb.Setters), (TextBoxElement ta, TextBoxElement tb) => ta.Value == tb.Value @@ -470,7 +522,7 @@ internal static bool ShallowEquals(Element a, Element b) && ta.TextWrapping == tb.TextWrapping && ta.SelectionStart == tb.SelectionStart && ta.SelectionLength == tb.SelectionLength - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), (NumberBoxElement na, NumberBoxElement nb) => na.Value == nb.Value @@ -481,12 +533,12 @@ internal static bool ShallowEquals(Element a, Element b) && na.Header == nb.Header && na.PlaceholderText == nb.PlaceholderText && na.SpinButtonPlacement == nb.SpinButtonPlacement - && ReferenceEquals(na.Setters, nb.Setters), + && SettersEqual(na.Setters, nb.Setters), (PasswordBoxElement pa, PasswordBoxElement pb) => pa.Password == pb.Password && pa.PlaceholderText == pb.PlaceholderText - && ReferenceEquals(pa.Setters, pb.Setters), + && SettersEqual(pa.Setters, pb.Setters), (ProgressElement pa, ProgressElement pb) => pa.Value == pb.Value @@ -494,24 +546,24 @@ internal static bool ShallowEquals(Element a, Element b) && pa.Maximum == pb.Maximum && pa.ShowError == pb.ShowError && pa.ShowPaused == pb.ShowPaused - && ReferenceEquals(pa.Setters, pb.Setters), + && SettersEqual(pa.Setters, pb.Setters), (ProgressRingElement pa, ProgressRingElement pb) => pa.Value == pb.Value && pa.Minimum == pb.Minimum && pa.Maximum == pb.Maximum && pa.IsActive == pb.IsActive - && ReferenceEquals(pa.Setters, pb.Setters), + && SettersEqual(pa.Setters, pb.Setters), (ImageElement ia, ImageElement ib) => ia.Source == ib.Source - && ReferenceEquals(ia.Setters, ib.Setters), + && SettersEqual(ia.Setters, ib.Setters), (RectangleElement ra, RectangleElement rb) => - ReferenceEquals(ra.Setters, rb.Setters), + SettersEqual(ra.Setters, rb.Setters), (EllipseElement ea, EllipseElement eb) => - ReferenceEquals(ea.Setters, eb.Setters), + SettersEqual(ea.Setters, eb.Setters), // Chart primitives — emitted in bulk by D3Charts. Without these arms, // every Path/Line in a chart falls through to UpdatePath/UpdateLine on @@ -559,7 +611,7 @@ internal static bool ShallowEquals(Element a, Element b) && ra.IsColorFontEnabled == rb.IsColorFontEnabled && ra.OpticalMarginAlignment == rb.OpticalMarginAlignment && BrushesEqual(ra.SelectionHighlightColor, rb.SelectionHighlightColor) - && ReferenceEquals(ra.Setters, rb.Setters), + && SettersEqual(ra.Setters, rb.Setters), // Container elements: compare own props + children by reference. // Same children reference = truly unchanged subtree = safe to skip entirely. @@ -570,7 +622,7 @@ internal static bool ShallowEquals(Element a, Element b) && sa.HorizontalAlignment == sb.HorizontalAlignment && sa.VerticalAlignment == sb.VerticalAlignment && ReferenceEquals(sa.Children, sb.Children) - && ReferenceEquals(sa.Setters, sb.Setters), + && SettersEqual(sa.Setters, sb.Setters), (BorderElement ba, BorderElement bb) => BrushesEqual(ba.Background, bb.Background) @@ -578,7 +630,7 @@ internal static bool ShallowEquals(Element a, Element b) && ba.CornerRadius == bb.CornerRadius && ba.BorderThickness == bb.BorderThickness && ReferenceEquals(ba.Child, bb.Child) - && ReferenceEquals(ba.Setters, bb.Setters), + && SettersEqual(ba.Setters, bb.Setters), // ItemContainer is the ItemsView item-root wrapper. Selection // state is framework-driven (so the Reactor element's @@ -588,14 +640,14 @@ internal static bool ShallowEquals(Element a, Element b) (ItemContainerElement ica, ItemContainerElement icb) => ica.IsSelected == icb.IsSelected && ReferenceEquals(ica.Child, icb.Child) - && ReferenceEquals(ica.Setters, icb.Setters), + && SettersEqual(ica.Setters, icb.Setters), (GridElement ga, GridElement gb) => ga.RowSpacing == gb.RowSpacing && ga.ColumnSpacing == gb.ColumnSpacing && ReferenceEquals(ga.Definition, gb.Definition) && ReferenceEquals(ga.Children, gb.Children) - && ReferenceEquals(ga.Setters, gb.Setters), + && SettersEqual(ga.Setters, gb.Setters), (ScrollViewerElement sva, ScrollViewerElement svb) => sva.Orientation == svb.Orientation @@ -605,7 +657,7 @@ internal static bool ShallowEquals(Element a, Element b) && sva.VerticalScrollMode == svb.VerticalScrollMode && sva.ZoomMode == svb.ZoomMode && ReferenceEquals(sva.Child, svb.Child) - && ReferenceEquals(sva.Setters, svb.Setters), + && SettersEqual(sva.Setters, svb.Setters), (ScrollViewElement sva, ScrollViewElement svb) => sva.ContentOrientation == svb.ContentOrientation @@ -619,7 +671,7 @@ internal static bool ShallowEquals(Element a, Element b) && sva.HorizontalAnchorRatio == svb.HorizontalAnchorRatio && sva.VerticalAnchorRatio == svb.VerticalAnchorRatio && ReferenceEquals(sva.Child, svb.Child) - && ReferenceEquals(sva.Setters, svb.Setters), + && SettersEqual(sva.Setters, svb.Setters), (FlexElement fa, FlexElement fb) => fa.Direction == fb.Direction @@ -631,7 +683,7 @@ internal static bool ShallowEquals(Element a, Element b) && fa.RowGap == fb.RowGap && fa.FlexPadding == fb.FlexPadding && ReferenceEquals(fa.Children, fb.Children) - && ReferenceEquals(fa.Setters, fb.Setters), + && SettersEqual(fa.Setters, fb.Setters), (CanvasElement ca, CanvasElement cb) => ca.Width == cb.Width @@ -676,13 +728,13 @@ internal static bool OwnPropsEqual(Element a, Element b) && sa.Spacing == sb.Spacing && sa.HorizontalAlignment == sb.HorizontalAlignment && sa.VerticalAlignment == sb.VerticalAlignment - && ReferenceEquals(sa.Setters, sb.Setters), + && SettersEqual(sa.Setters, sb.Setters), (Core.GridElement ga, Core.GridElement gb) => ga.RowSpacing == gb.RowSpacing && ga.ColumnSpacing == gb.ColumnSpacing && ReferenceEquals(ga.Definition, gb.Definition) - && ReferenceEquals(ga.Setters, gb.Setters), + && SettersEqual(ga.Setters, gb.Setters), (BorderElement ba, BorderElement bb) => BrushesEqual(ba.Background, bb.Background) @@ -690,7 +742,7 @@ internal static bool OwnPropsEqual(Element a, Element b) && ba.CornerRadius == bb.CornerRadius && ba.Padding == bb.Padding && ba.BorderThickness == bb.BorderThickness - && ReferenceEquals(ba.Setters, bb.Setters), + && SettersEqual(ba.Setters, bb.Setters), // ItemContainer: own props (excluding Child) match when // IsSelected and Setters agree. The reconcile-highlight gate @@ -698,7 +750,7 @@ internal static bool OwnPropsEqual(Element a, Element b) // when the only changes are inside the user-supplied subtree. (ItemContainerElement ica, ItemContainerElement icb) => ica.IsSelected == icb.IsSelected - && ReferenceEquals(ica.Setters, icb.Setters), + && SettersEqual(ica.Setters, icb.Setters), (ScrollViewerElement sva, ScrollViewerElement svb) => sva.Orientation == svb.Orientation @@ -707,7 +759,7 @@ internal static bool OwnPropsEqual(Element a, Element b) && sva.HorizontalScrollMode == svb.HorizontalScrollMode && sva.VerticalScrollMode == svb.VerticalScrollMode && sva.ZoomMode == svb.ZoomMode - && ReferenceEquals(sva.Setters, svb.Setters), + && SettersEqual(sva.Setters, svb.Setters), (ScrollViewElement sva, ScrollViewElement svb) => sva.ContentOrientation == svb.ContentOrientation @@ -720,7 +772,7 @@ internal static bool OwnPropsEqual(Element a, Element b) && sva.MaxZoomFactor == svb.MaxZoomFactor && sva.HorizontalAnchorRatio == svb.HorizontalAnchorRatio && sva.VerticalAnchorRatio == svb.VerticalAnchorRatio - && ReferenceEquals(sva.Setters, svb.Setters), + && SettersEqual(sva.Setters, svb.Setters), (FlexElement fa, FlexElement fb) => fa.Direction == fb.Direction @@ -731,23 +783,23 @@ internal static bool OwnPropsEqual(Element a, Element b) && fa.ColumnGap == fb.ColumnGap && fa.RowGap == fb.RowGap && fa.FlexPadding == fb.FlexPadding - && ReferenceEquals(fa.Setters, fb.Setters), + && SettersEqual(fa.Setters, fb.Setters), (CanvasElement ca, CanvasElement cb) => - ReferenceEquals(ca.Setters, cb.Setters), + SettersEqual(ca.Setters, cb.Setters), (WrapGridElement wa, WrapGridElement wb) => wa.Orientation == wb.Orientation && wa.ItemWidth == wb.ItemWidth && wa.ItemHeight == wb.ItemHeight && wa.MaximumRowsOrColumns == wb.MaximumRowsOrColumns - && ReferenceEquals(wa.Setters, wb.Setters), + && SettersEqual(wa.Setters, wb.Setters), (RelativePanelElement ra, RelativePanelElement rb) => - ReferenceEquals(ra.Setters, rb.Setters), + SettersEqual(ra.Setters, rb.Setters), (ViewboxElement va, ViewboxElement vb) => - ReferenceEquals(va.Setters, vb.Setters), + SettersEqual(va.Setters, vb.Setters), // Structural wrappers that only contain children (NavigationHostElement, NavigationHostElement) => true, @@ -765,7 +817,7 @@ internal static bool OwnPropsEqual(Element a, Element b) && ta.IsBackButtonVisible == tb.IsBackButtonVisible && ta.IsBackButtonEnabled == tb.IsBackButtonEnabled && ta.IsPaneToggleButtonVisible == tb.IsPaneToggleButtonVisible - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), // Pure composition wrappers — they never write their own WinUI // properties; their rendered output is diffed separately. Returning @@ -796,56 +848,56 @@ internal static bool OwnPropsEqual(Element a, Element b) && ca.PlaceholderText == cb.PlaceholderText && ca.Header == cb.Header && ca.IsEditable == cb.IsEditable - && ReferenceEquals(ca.Setters, cb.Setters), + && SettersEqual(ca.Setters, cb.Setters), (ListViewElement la, ListViewElement lb) => la.SelectedIndex == lb.SelectedIndex && la.SelectionMode == lb.SelectionMode && la.Header == lb.Header - && ReferenceEquals(la.Setters, lb.Setters), + && SettersEqual(la.Setters, lb.Setters), (GridViewElement ga, GridViewElement gb) => ga.SelectedIndex == gb.SelectedIndex && ga.SelectionMode == gb.SelectionMode && ga.Header == gb.Header - && ReferenceEquals(ga.Setters, gb.Setters), + && SettersEqual(ga.Setters, gb.Setters), (FlipViewElement fa, FlipViewElement fb) => fa.SelectedIndex == fb.SelectedIndex - && ReferenceEquals(fa.Setters, fb.Setters), + && SettersEqual(fa.Setters, fb.Setters), (PivotElement pa, PivotElement pb) => pa.SelectedIndex == pb.SelectedIndex && pa.Title == pb.Title - && ReferenceEquals(pa.Setters, pb.Setters), + && SettersEqual(pa.Setters, pb.Setters), (TabViewElement ta, TabViewElement tb) => ta.SelectedIndex == tb.SelectedIndex && ta.IsAddTabButtonVisible == tb.IsAddTabButtonVisible - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), (TreeViewElement ta, TreeViewElement tb) => ta.SelectionMode == tb.SelectionMode && ta.CanDragItems == tb.CanDragItems && ta.AllowDrop == tb.AllowDrop && ta.CanReorderItems == tb.CanReorderItems - && ReferenceEquals(ta.Setters, tb.Setters), + && SettersEqual(ta.Setters, tb.Setters), (SelectorBarElement sa, SelectorBarElement sb) => sa.SelectedIndex == sb.SelectedIndex - && ReferenceEquals(sa.Setters, sb.Setters), + && SettersEqual(sa.Setters, sb.Setters), (ListBoxElement la, ListBoxElement lb) => la.SelectedIndex == lb.SelectedIndex - && ReferenceEquals(la.Setters, lb.Setters), + && SettersEqual(la.Setters, lb.Setters), (RadioButtonsElement ra, RadioButtonsElement rb) => ra.SelectedIndex == rb.SelectedIndex && ra.Header == rb.Header - && ReferenceEquals(ra.Setters, rb.Setters), + && SettersEqual(ra.Setters, rb.Setters), (BreadcrumbBarElement ba, BreadcrumbBarElement bb) => - ReferenceEquals(ba.Setters, bb.Setters), + SettersEqual(ba.Setters, bb.Setters), // Templated (data-driven) collections: own props are the WinUI // properties UpdateTemplatedXxx writes back. Items + ViewBuilder @@ -878,8 +930,8 @@ internal static bool OwnPropsEqual(Element a, Element b) la.Orientation == lb.Orientation && la.Spacing == lb.Spacing && la.EstimatedItemSize == lb.EstimatedItemSize - && ReferenceEquals(la.ScrollViewerSetters, lb.ScrollViewerSetters) - && ReferenceEquals(la.RepeaterSetters, lb.RepeaterSetters), + && SettersEqual(la.ScrollViewerSetters, lb.ScrollViewerSetters) + && SettersEqual(la.RepeaterSetters, lb.RepeaterSetters), // Non-container / leaf types: return false → always captured _ => false, @@ -1020,52 +1072,89 @@ private static bool FontFamiliesEqual(Microsoft.UI.Xaml.Media.FontFamily? a, Mic return a.Source == b.Source; } + /// + /// Conservative equality for two Setters arrays (the imperative + /// .Set(x => …) escape hatch). Returns true only when the arrays are the + /// same instance (a memoized/unchanged element) or both empty + /// (no imperative writes to apply). It deliberately does not compare + /// element-wise by delegate identity. + /// + /// Why not element-wise identity: a setter is an apply-time imperative write + /// whose effect can read mutable external state (statics, singletons, captured + /// mutable objects). Non-capturing lambdas and method groups are cached by the + /// C# compiler to a single static delegate, so identity-equality across renders + /// does not imply the write is unnecessary. Skipping a reference-stable + /// setter would strand a stale value on the control — a regression versus the + /// documented contract that .Set re-applies on every update. Setters + /// therefore keep the element on the Update path whenever any are present + /// (mirroring the !HasSetters guards used by the templated/lazy element + /// arms). Handler modifiers differ and are safe to compare by identity + /// (FLAGSHIP-1): they dispatch later through + /// ModifierEventHandlerState.Current* and read fresh state at fire time, + /// rather than being re-applied imperatively during Update. + /// + /// + internal static bool SettersEqual(T[]? a, T[]? b) where T : class + { + if (ReferenceEquals(a, b)) return true; + return (a is null || a.Length == 0) && (b is null || b.Length == 0); + } + /// /// Compare two ElementModifiers for rendering equivalence. /// Brushes and FontFamily are compared structurally because fluent helpers /// (.Background("#color"), .FontFamily("Segoe UI")) allocate /// fresh instances on every render even when the underlying values match. - /// Ignores OnMountAction and OnUpdateAction (side-effect hooks, not render state). + /// Ignores OnMountAction (fires once at mount, gated on oldM is null, so a later + /// change is inert), but compares OnUnmountAction by reference — the latest + /// teardown is re-registered on every Update and fires at unmount, so a changed + /// teardown closure must decline the skip. /// internal static bool ModifiersEqual(ElementModifiers? a, ElementModifiers? b) { if (ReferenceEquals(a, b)) return true; if (a is null || b is null) return false; - return a.Margin == b.Margin - && a.Padding == b.Padding - && a.Width == b.Width - && a.Height == b.Height - && a.MinWidth == b.MinWidth - && a.MinHeight == b.MinHeight - && a.MaxWidth == b.MaxWidth - && a.MaxHeight == b.MaxHeight - && a.HorizontalAlignment == b.HorizontalAlignment - && a.VerticalAlignment == b.VerticalAlignment - && a.Opacity == b.Opacity - && a.IsVisible == b.IsVisible - && a.IsEnabled == b.IsEnabled - && a.CornerRadius == b.CornerRadius - && a.BorderThickness == b.BorderThickness + // #159 — hoist the Layout/Visual sub-record reads once instead of + // dereferencing a.Layout?.X / a.Visual?.X ~20 times through the shim + // properties. A null bucket is normalized to a shared empty sentinel so + // the field-level semantics match the historical per-shim comparison + // (a null Layout reads as all-null fields). + var al = a.Layout ?? _emptyLayout; + var bl = b.Layout ?? _emptyLayout; + // LayoutModifiers is entirely value-typed, so the synthesized record + // value-equality is exact — and also covers the logical-inset / + // RequestedTheme fields an explicit field list historically forgot, + // keeping the skip path faithful to a full Update. + if (!ReferenceEquals(al, bl) && !al.Equals(bl)) return false; + + // Visual holds Brush fields that must compare by value (Color/Opacity), + // not by record reference-equality, so it is compared field-by-field. + if (!VisualModifiersEqual(a.Visual, b.Visual)) return false; + + return a.IsEnabled == b.IsEnabled && a.ElementSoundMode == b.ElementSoundMode && a.ToolTip == b.ToolTip && a.AutomationName == b.AutomationName && a.AutomationId == b.AutomationId - && BrushesEqual(a.Background, b.Background) - && BrushesEqual(a.Foreground, b.Foreground) - && BrushesEqual(a.BorderBrush, b.BorderBrush) && a.FontSize == b.FontSize && a.FontWeight == b.FontWeight && FontFamiliesEqual(a.FontFamily, b.FontFamily) - // Skip OnMountAction — only runs at mount time - // Skip OnUpdateAction — side-effect hook, fired each update from ApplyModifiers - // Skip event handlers — delegate comparison is unreliable, conservative false - && a.OnSizeChanged is null && b.OnSizeChanged is null - && a.OnPointerPressed is null && b.OnPointerPressed is null - && a.OnPointerMoved is null && b.OnPointerMoved is null - && a.OnPointerReleased is null && b.OnPointerReleased is null - && a.OnTapped is null && b.OnTapped is null - && a.OnKeyDown is null && b.OnKeyDown is null + // FLAGSHIP-1 — modifier event handlers dispatch through the + // reconciler's per-element ModifierEventHandlerState.Current* fields, + // which are refreshed ONLY on the non-skip Update path + // (ApplyEventHandlers). Skipping is therefore safe exactly when every + // callback-bearing slot is reference-equal: the stale Current* IS the + // new delegate, so dispatch is identical. Reference-stable handlers + // (non-capturing lambdas, method groups, memoized closures) now take + // the skip path; per-render capturing closures still force Update so + // their fresh captures are wired. This replaces the old blanket + // "any-handler-present ⇒ never skip" rule and also closes the latent + // hole where non-listed routed handlers (PointerEntered, GotFocus, …) + // were ignored entirely. Gesture / drag-drop slots are deliberately + // NOT compared here (see ModifierCallbacksEqual) so behavior matches + // the historical diff exactly. + && ModifierCallbacksEqual(a, b) // Skip RichToolTip, AttachedFlyout, ContextFlyout — rare, conservative false && a.RichToolTip is null && b.RichToolTip is null && a.AttachedFlyout is null && b.AttachedFlyout is null @@ -1083,6 +1172,18 @@ internal static bool ModifiersEqual(ElementModifiers? a, ElementModifiers? b) // must force Update so ApplyModifiers clears the old cell and sets the // new one — otherwise the shallow-skip path strands a stale ElementRef. && ReferenceEquals(a.Ref, b.Ref) + // Imperative teardown slot (.OnUnmount). ApplyModifiers re-registers the + // latest OnUnmountAction (Reconciler._onUnmountActions) on every non-skip + // Update, and that captured delegate is what fires at unmount — so a + // changed teardown closure must decline the skip or the stale (first-render) + // action is stranded and runs in place of the current one. Unlike + // OnMountAction (fired once at mount, gated on oldM is null, so a later + // change is inert and is intentionally ignored), OnUnmountAction has live + // update-path semantics. ReferenceEquals mirrors the handler/.Ref treatment: + // both null ⇒ equal (no teardown either render ⇒ zero skip-rate cost for + // plain leaves), a reference-stable teardown still skips, a fresh closure + // forces Update so the latest teardown is registered. + && ReferenceEquals(a.OnUnmountAction, b.OnUnmountAction) // Accessibility Tier 2/3. AccessibilityModifiers is a record of // scalar/string fields, but every fluent helper (.AccessibilityView, // .LiveRegion, .ItemStatus, …) allocates a fresh instance per render @@ -1094,6 +1195,80 @@ internal static bool ModifiersEqual(ElementModifiers? a, ElementModifiers? b) && AccessibilityEqual(a.Accessibility, b.Accessibility); } + // Shared empty buckets so a null Layout/Visual can be compared field-for-field + // against a set one without allocating (and without 20+ null-conditional shims). + private static readonly LayoutModifiers _emptyLayout = new(); + private static readonly VisualModifiers _emptyVisual = new(); + + /// + /// Compare the Visual buckets by value. Brush fields (Background / Foreground / + /// BorderBrush) are compared with (Color + Opacity) + /// because fluent helpers allocate a fresh per + /// render; everything else is a value type compared with ==. + /// + private static bool VisualModifiersEqual(VisualModifiers? a, VisualModifiers? b) + { + if (ReferenceEquals(a, b)) return true; + a ??= _emptyVisual; + b ??= _emptyVisual; + return a.Opacity == b.Opacity + && a.CornerRadius == b.CornerRadius + && a.BorderThickness == b.BorderThickness + && a.Scale == b.Scale + && a.Rotation == b.Rotation + && a.Translation == b.Translation + && a.CenterPoint == b.CenterPoint + && BrushesEqual(a.Background, b.Background) + && BrushesEqual(a.Foreground, b.Foreground) + && BrushesEqual(a.BorderBrush, b.BorderBrush); + } + + /// + /// FLAGSHIP-1 — true when every routed-input event-handler slot is + /// reference-equal between and . + /// These 21 handlers dispatch through the reconciler's + /// ModifierEventHandlerState.Current* fields, refreshed only on the + /// non-skip Update path, so the skip path is sound iff the delegate identity + /// is unchanged. Reference equality (not presence) is the safe predicate: + /// a stale Current* that equals the new delegate dispatches identically, + /// while a freshly captured closure forces Update so its captures are wired. + /// + /// The gesture (Pan/Pinch/Rotate/LongPress) and drag-drop (DragSource/ + /// DropTarget) slots are deliberately NOT compared here. They dispatch through + /// separate per-element gesture/drag state, and the historical diff never + /// compared them — so a per-render gesture closure stays skip-eligible exactly + /// as before. Comparing them would force Update where the framework previously + /// skipped, re-arming an in-flight gesture mid-interaction (e.g. re-registering + /// a long-press handler between its Began and Ended phases, so the released + /// callback fires against a refreshed closure and double-dispatches). Excluding + /// them preserves observable behavior exactly; grid cells use only routed + /// handlers, so the skip-path perf lever is unaffected. + /// + private static bool ModifierCallbacksEqual(ElementModifiers a, ElementModifiers b) + { + return ReferenceEquals(a.OnSizeChanged, b.OnSizeChanged) + && ReferenceEquals(a.OnPointerPressed, b.OnPointerPressed) + && ReferenceEquals(a.OnPointerMoved, b.OnPointerMoved) + && ReferenceEquals(a.OnPointerReleased, b.OnPointerReleased) + && ReferenceEquals(a.OnPointerEntered, b.OnPointerEntered) + && ReferenceEquals(a.OnPointerExited, b.OnPointerExited) + && ReferenceEquals(a.OnPointerCanceled, b.OnPointerCanceled) + && ReferenceEquals(a.OnPointerCaptureLost, b.OnPointerCaptureLost) + && ReferenceEquals(a.OnPointerWheelChanged, b.OnPointerWheelChanged) + && ReferenceEquals(a.OnTapped, b.OnTapped) + && ReferenceEquals(a.OnDoubleTapped, b.OnDoubleTapped) + && ReferenceEquals(a.OnRightTapped, b.OnRightTapped) + && ReferenceEquals(a.OnHolding, b.OnHolding) + && ReferenceEquals(a.OnKeyDown, b.OnKeyDown) + && ReferenceEquals(a.OnKeyUp, b.OnKeyUp) + && ReferenceEquals(a.OnPreviewKeyDown, b.OnPreviewKeyDown) + && ReferenceEquals(a.OnPreviewKeyUp, b.OnPreviewKeyUp) + && ReferenceEquals(a.OnCharacterReceived, b.OnCharacterReceived) + && ReferenceEquals(a.OnGotFocus, b.OnGotFocus) + && ReferenceEquals(a.OnLostFocus, b.OnLostFocus) + && ReferenceEquals(a.OnAccessKeyDisplayRequested, b.OnAccessKeyDisplayRequested); + } + private static bool AccessibilityEqual(AccessibilityModifiers? a, AccessibilityModifiers? b) { if (ReferenceEquals(a, b)) return true; @@ -1112,6 +1287,18 @@ internal static bool AttachedEqual(IReadOnlyDictionary? a, IReadOn if (a is null || b is null) return false; if (a.Count != b.Count) return false; + // #155 hot path — one attached value per element (a single .Grid per cell, + // re-applied each render ⇒ SingleAttachedDictionary on both sides). Compare + // the lone slot directly: `foreach` over the IReadOnlyDictionary would + // allocate an IEnumerator every diff (SingleAttachedDictionary + // enumerates via `yield`), reintroducing the per-cell allocation #155 exists + // to remove. Count is already known equal, so a single TryGetValue on the + // other side is a complete comparison. + if (a is SingleAttachedDictionary sa) + return b.TryGetValue(sa.SingleKey, out var sav) && Equals(sa.SingleValue, sav); + if (b is SingleAttachedDictionary sb) + return a.TryGetValue(sb.SingleKey, out var sbv) && Equals(sb.SingleValue, sbv); + foreach (var (key, valA) in a) { if (!b.TryGetValue(key, out var valB)) return false; diff --git a/src/Reactor/Elements/BrushHelper.cs b/src/Reactor/Elements/BrushHelper.cs index 7ad51ff0f..3795add41 100644 --- a/src/Reactor/Elements/BrushHelper.cs +++ b/src/Reactor/Elements/BrushHelper.cs @@ -6,22 +6,36 @@ namespace Microsoft.UI.Reactor; /// /// Color and brush parsing utilities. /// Supports named colors, hex (#RRGGBB, #AARRGGBB), and direct Color values. -/// Colors are cached by string; a fresh SolidColorBrush is created per call -/// because DependencyObjects have thread affinity and cannot be safely shared. +/// Parsed colors are cached by string (an immutable value type, safe to share), +/// but a fresh is created per call: a brush is a +/// DependencyObject with thread affinity and mutable Color/Opacity, so a single +/// instance cannot be safely shared across controls. /// public static class BrushHelper { - private static readonly ConcurrentDictionary _colorCache = new(); + // OrdinalIgnoreCase so equivalent colors that differ only in casing + // ("Red"/"red", "#FF0000"/"#ff0000") share one cache entry instead of + // creating duplicates. Safe because ParseColor lowercases named colors + // before matching and ParseHex is case-insensitive, so case-folded keying + // yields identical results to the prior case-sensitive keying. + private static readonly ConcurrentDictionary _colorCache = + new(global::System.StringComparer.OrdinalIgnoreCase); /// - /// Parses a color string into a SolidColorBrush. - /// Supports named colors (red, green, blue, white, black, gray, lightgray, transparent) - /// and hex codes (#RRGGBB or #AARRGGBB). - /// Color parsing is cached; a new brush is created each call (thread-safe). + /// Parses a color string into a fresh owned by the + /// caller. Supports named colors (red, green, blue, white, black, gray, lightgray, + /// transparent) and hex codes (#RRGGBB or #AARRGGBB). The parsed color is cached, + /// but a new brush instance is returned on every call, so callers may safely mutate + /// it and a brush is never shared between controls. /// - public static SolidColorBrush Parse(string color) - { - var parsed = _colorCache.GetOrAdd(color, static c => + public static SolidColorBrush Parse(string color) => new(ParseColor(color)); + + /// + /// Parses a color string into a . + /// The result is cached by string so repeated parses are allocation-free. + /// + internal static global::Windows.UI.Color ParseColor(string color) => + _colorCache.GetOrAdd(color, static c => c.ToLowerInvariant() switch { "red" => global::Windows.UI.Color.FromArgb(255, 255, 0, 0), @@ -35,8 +49,6 @@ public static SolidColorBrush Parse(string color) _ when c.StartsWith('#') => ParseHex(c), _ => global::Windows.UI.Color.FromArgb(255, 128, 128, 128), }); - return new SolidColorBrush(parsed); - } internal static global::Windows.UI.Color ParseHex(string hex) { diff --git a/src/Reactor/Elements/Dsl.cs b/src/Reactor/Elements/Dsl.cs index bbe804838..570683aec 100644 --- a/src/Reactor/Elements/Dsl.cs +++ b/src/Reactor/Elements/Dsl.cs @@ -36,6 +36,12 @@ namespace Microsoft.UI.Reactor; /// public static partial class Factories { + // Shared single-Star track array for the cross-axis of UniformGrid / + // InterspersedGrid. Safe to share: GridDefinition immediately converts + // GridSize[] tracks to string[] (read-only consumption), so the array is + // never retained or mutated by callers, and GridSize is a value struct. + private static readonly GridSize[] s_oneStar = { GridSize.Star() }; + private static Optional ToOptionalSelectedIndex(int? selectedIndex) => selectedIndex.HasValue ? Optional.Of(selectedIndex.Value) : Optional.Unset; @@ -796,33 +802,38 @@ public static GridElement InterspersedGrid( throw new ArgumentOutOfRangeException(nameof(proportions), $"proportions[{i}] must be a non-negative number, got {proportions[i]}"); } - var sizes = new List(); - var children = new List(); + var oneStar = s_oneStar; bool isHorizontal = orientation == Orientation.Horizontal; + // #172 — exact track/child count is known up front (one track per item + // plus a separator between each pair), so fill pre-sized arrays directly + // instead of growing two Lists and copying them with ToArray(). + int trackCount = items.Length * 2 - 1; + var sizes = new GridSize[trackCount]; + var children = new Element[trackCount]; + for (int i = 0; i < items.Length; i++) { var starValue = proportions[i]; - sizes.Add(GridSize.Star(starValue)); + sizes[i * 2] = GridSize.Star(starValue); - children.Add(isHorizontal + children[i * 2] = isHorizontal ? items[i].Grid(row: 0, column: i * 2) - : items[i].Grid(row: i * 2, column: 0)); + : items[i].Grid(row: i * 2, column: 0); if (i < items.Length - 1) { - sizes.Add(GridSize.Px(separatorSize)); + sizes[i * 2 + 1] = GridSize.Px(separatorSize); var sep = separatorFactory(i); - children.Add(isHorizontal + children[i * 2 + 1] = isHorizontal ? sep.Grid(row: 0, column: i * 2 + 1) - : sep.Grid(row: i * 2 + 1, column: 0)); + : sep.Grid(row: i * 2 + 1, column: 0); } } - var oneStar = new[] { GridSize.Star() }; return isHorizontal - ? Grid(sizes.ToArray(), oneStar, children.ToArray()) - : Grid(oneStar, sizes.ToArray(), children.ToArray()); + ? Grid(sizes, oneStar, children) + : Grid(oneStar, sizes, children); } /// @@ -834,20 +845,34 @@ public static GridElement UniformGrid(Orientation orientation, params Element?[] var filtered = FilterChildren(items); if (filtered.Length == 0) return Grid(global::System.Array.Empty(), global::System.Array.Empty()); - var sizes = Enumerable.Repeat(GridSize.Star(), filtered.Length).ToArray(); - var oneStar = new[] { GridSize.Star() }; + // #171 — fill the equal-Star track array with a pre-sized loop instead + // of Enumerable.Repeat(...).ToArray() (which allocates a LINQ iterator on + // top of the array). GridSize is a value struct, so every slot is the + // same Star value. + var sizes = new GridSize[filtered.Length]; + var star = GridSize.Star(); + for (int i = 0; i < sizes.Length; i++) + sizes[i] = star; + var oneStar = s_oneStar; bool isHorizontal = orientation == Orientation.Horizontal; + // Position each cell into its OWN array — never write back into + // `filtered`. FilterChildren's fast path returns the caller's array + // aliased (no copy), so mutating it in place would corrupt a + // caller-supplied `items` array with .Grid(...) wrappers. This matches + // the historical behavior, where FilterChildren always returned a fresh + // owned array that was safe to fill. + var positioned = new Element[filtered.Length]; for (int i = 0; i < filtered.Length; i++) { - filtered[i] = isHorizontal + positioned[i] = isHorizontal ? filtered[i].Grid(row: 0, column: i) : filtered[i].Grid(row: i, column: 0); } return isHorizontal - ? Grid(sizes, oneStar, filtered) - : Grid(oneStar, sizes, filtered); + ? Grid(sizes, oneStar, positioned) + : Grid(oneStar, sizes, positioned); } // ── Navigation ────────────────────────────────────────────────── @@ -1317,15 +1342,36 @@ public static Element Expr(Func render) /// Map a list to elements (like .map() in React JSX): /// ForEach(items, item => TextBlock(item.Name)) /// - public static Element ForEach(IEnumerable items, Func render) => - new GroupElement(items.Select(render).ToArray()); + public static Element ForEach(IEnumerable items, Func render) + { + // #170 — IReadOnlyList fast-path: pre-size the Element[] and index + // directly, skipping the Select iterator + closure allocations that + // dominate when re-rendering large data-bound collections every frame. + if (items is IReadOnlyList list) + { + var arr = new Element[list.Count]; + for (int i = 0; i < arr.Length; i++) + arr[i] = render(list[i]); + return new GroupElement(arr); + } + return new GroupElement(items.Select(render).ToArray()); + } /// /// Map with index: /// ForEach(items, (item, i) => TextBlock($"{i}: {item}")) /// - public static Element ForEach(IEnumerable items, Func render) => - new GroupElement(items.Select((item, i) => render(item, i)).ToArray()); + public static Element ForEach(IEnumerable items, Func render) + { + if (items is IReadOnlyList list) + { + var arr = new Element[list.Count]; + for (int i = 0; i < arr.Length; i++) + arr[i] = render(list[i], i); + return new GroupElement(arr); + } + return new GroupElement(items.Select((item, i) => render(item, i)).ToArray()); + } /// /// Groups elements without introducing a layout container (like React's Fragment). @@ -1900,8 +1946,29 @@ private static Element[] FilterChildren(Element?[] children) } if (!needsExpansion) return (Element[])(object)children; - // Flatten GroupElements and remove nulls - var result = new List(); + // #173 — slow path: two passes (count, then fill an exactly-sized array) + // instead of growing a List and copying it with ToArray(). Flattens one + // level of GroupElement and drops null / EmptyElement, matching the prior + // behavior exactly. + int count = 0; + for (int i = 0; i < children.Length; i++) + { + if (children[i] is GroupElement group) + { + foreach (var gc in group.Children) + { + if (gc is not null and not EmptyElement) + count++; + } + } + else if (children[i] is not null and not EmptyElement) + { + count++; + } + } + + var result = new Element[count]; + int idx = 0; for (int i = 0; i < children.Length; i++) { if (children[i] is GroupElement group) @@ -1909,14 +1976,14 @@ private static Element[] FilterChildren(Element?[] children) foreach (var gc in group.Children) { if (gc is not null and not EmptyElement) - result.Add(gc); + result[idx++] = gc; } } else if (children[i] is not null and not EmptyElement) { - result.Add(children[i]!); + result[idx++] = children[i]!; } } - return result.ToArray(); + return result; } } diff --git a/src/Reactor/Elements/ElementExtensions.cs b/src/Reactor/Elements/ElementExtensions.cs index cce7063f7..53e84a405 100644 --- a/src/Reactor/Elements/ElementExtensions.cs +++ b/src/Reactor/Elements/ElementExtensions.cs @@ -40,14 +40,14 @@ public static partial class ElementExtensions // ════════════════════════════════════════════════════════════════ public static T Margin(this T el, double uniform) where T : Element => - Modify(el, new ElementModifiers { Margin = new Thickness(uniform) }); + ModifyLayout(el, new LayoutModifiers { Margin = new Thickness(uniform) }); // Two-argument spacing follows the same order everywhere in Reactor: // horizontal FIRST, then vertical. This keeps `.Margin(...)`, // `.Padding(...)`, and `.FlexPadding(...)` aligned and matches the mental // model of Thickness(left/right, top/bottom). public static T Margin(this T el, double horizontal, double vertical) where T : Element => - Modify(el, new ElementModifiers { Margin = new Thickness(horizontal, vertical, horizontal, vertical) }); + ModifyLayout(el, new LayoutModifiers { Margin = new Thickness(horizontal, vertical, horizontal, vertical) }); // Default values on the per-side overload let callers name only the sides // they want (`.Margin(top: 10)`, `.Margin(left: 8, right: 8)`). Existing @@ -57,68 +57,68 @@ public static T Margin(this T el, double horizontal, double vertical) where T // (fewer parameters → preferred match). Mirrors WPF Thickness defaults: // unspecified sides are zero. public static T Margin(this T el, double left = 0.0, double top = 0.0, double right = 0.0, double bottom = 0.0) where T : Element => - Modify(el, new ElementModifiers { Margin = new Thickness(left, top, right, bottom) }); + ModifyLayout(el, new LayoutModifiers { Margin = new Thickness(left, top, right, bottom) }); public static T Padding(this T el, double uniform) where T : Element => - Modify(el, new ElementModifiers { Padding = new Thickness(uniform) }); + ModifyLayout(el, new LayoutModifiers { Padding = new Thickness(uniform) }); // Same ordering as Margin above — horizontal first, then vertical. public static T Padding(this T el, double horizontal, double vertical) where T : Element => - Modify(el, new ElementModifiers { Padding = new Thickness(horizontal, vertical, horizontal, vertical) }); + ModifyLayout(el, new LayoutModifiers { Padding = new Thickness(horizontal, vertical, horizontal, vertical) }); // Same defaulting story as Margin above — `.Padding(top: 8)` etc. are // valid; existing 1-arg / 2-arg / 4-arg positional call shapes still bind // to the more-specific overloads. public static T Padding(this T el, double left = 0.0, double top = 0.0, double right = 0.0, double bottom = 0.0) where T : Element => - Modify(el, new ElementModifiers { Padding = new Thickness(left, top, right, bottom) }); + ModifyLayout(el, new LayoutModifiers { Padding = new Thickness(left, top, right, bottom) }); // ── Logical (BiDi-aware) layout modifiers ─────────────────────── // InlineStart = left in LTR, right in RTL. InlineEnd = right in LTR, left in RTL. // Resolved at mount/update time based on FlowDirection. public static T MarginInlineStart(this T el, double value) where T : Element => - Modify(el, new ElementModifiers { MarginInlineStart = value }); + ModifyLayout(el, new LayoutModifiers { MarginInlineStart = value }); public static T MarginInlineEnd(this T el, double value) where T : Element => - Modify(el, new ElementModifiers { MarginInlineEnd = value }); + ModifyLayout(el, new LayoutModifiers { MarginInlineEnd = value }); public static T PaddingInlineStart(this T el, double value) where T : Element => - Modify(el, new ElementModifiers { PaddingInlineStart = value }); + ModifyLayout(el, new LayoutModifiers { PaddingInlineStart = value }); public static T PaddingInlineEnd(this T el, double value) where T : Element => - Modify(el, new ElementModifiers { PaddingInlineEnd = value }); + ModifyLayout(el, new LayoutModifiers { PaddingInlineEnd = value }); public static T BorderInlineStart(this T el, Thickness thickness) where T : Element => - Modify(el, new ElementModifiers { BorderInlineStart = thickness }); + ModifyLayout(el, new LayoutModifiers { BorderInlineStart = thickness }); public static T Width(this T el, double width) where T : Element => - Modify(el, new ElementModifiers { Width = width }); + ModifyLayout(el, new LayoutModifiers { Width = width }); public static T Height(this T el, double height) where T : Element => - Modify(el, new ElementModifiers { Height = height }); + ModifyLayout(el, new LayoutModifiers { Height = height }); public static T Size(this T el, double width, double height) where T : Element => - Modify(el, new ElementModifiers { Width = width, Height = height }); + ModifyLayout(el, new LayoutModifiers { Width = width, Height = height }); public static T MinWidth(this T el, double w) where T : Element => - Modify(el, new ElementModifiers { MinWidth = w }); + ModifyLayout(el, new LayoutModifiers { MinWidth = w }); public static T MinHeight(this T el, double h) where T : Element => - Modify(el, new ElementModifiers { MinHeight = h }); + ModifyLayout(el, new LayoutModifiers { MinHeight = h }); public static T MaxWidth(this T el, double w) where T : Element => - Modify(el, new ElementModifiers { MaxWidth = w }); + ModifyLayout(el, new LayoutModifiers { MaxWidth = w }); public static T MaxHeight(this T el, double h) where T : Element => - Modify(el, new ElementModifiers { MaxHeight = h }); + ModifyLayout(el, new LayoutModifiers { MaxHeight = h }); // ── Alignment ─────────────────────────────────────────────────── public static T HAlign(this T el, HorizontalAlignment alignment) where T : Element => - Modify(el, new ElementModifiers { HorizontalAlignment = alignment }); + ModifyLayout(el, new LayoutModifiers { HorizontalAlignment = alignment }); public static T VAlign(this T el, VerticalAlignment alignment) where T : Element => - Modify(el, new ElementModifiers { VerticalAlignment = alignment }); + ModifyLayout(el, new LayoutModifiers { VerticalAlignment = alignment }); // Long-form aliases matching the WinUI/WPF FrameworkElement property names. // The agent's first-instinct write is `.HorizontalAlignment(...)`; making @@ -126,13 +126,13 @@ public static T VAlign(this T el, VerticalAlignment alignment) where T : Elem // Parameter types are fully qualified because the method names shadow the // enum names in this scope. public static T HorizontalAlignment(this T el, Microsoft.UI.Xaml.HorizontalAlignment alignment) where T : Element => - Modify(el, new ElementModifiers { HorizontalAlignment = alignment }); + ModifyLayout(el, new LayoutModifiers { HorizontalAlignment = alignment }); public static T VerticalAlignment(this T el, Microsoft.UI.Xaml.VerticalAlignment alignment) where T : Element => - Modify(el, new ElementModifiers { VerticalAlignment = alignment }); + ModifyLayout(el, new LayoutModifiers { VerticalAlignment = alignment }); public static T Center(this T el) where T : Element => - Modify(el, new ElementModifiers + ModifyLayout(el, new LayoutModifiers { HorizontalAlignment = Microsoft.UI.Xaml.HorizontalAlignment.Center, VerticalAlignment = Microsoft.UI.Xaml.VerticalAlignment.Center, @@ -151,13 +151,13 @@ public static T Center(this T el) where T : Element => /// /// public static T RequestedTheme(this T el, ElementTheme theme) where T : Element => - Modify(el, new ElementModifiers { RequestedTheme = theme }); + ModifyLayout(el, new LayoutModifiers { RequestedTheme = theme }); // ── Visibility ────────────────────────────────────────────────── /// Sets whether the element is visible or collapsed. public static T IsVisible(this T el, bool isVisible) where T : Element => - Modify(el, new ElementModifiers { IsVisible = isVisible }); + ModifyLayout(el, new LayoutModifiers { IsVisible = isVisible }); /// [Obsolete("Use IsVisible() — aligns with WinUI boolean modifier naming (see #268).")] @@ -165,19 +165,19 @@ public static T Visible(this T el, bool isVisible) where T : Element => el.IsVisible(isVisible); public static T Opacity(this T el, double opacity) where T : Element => - Modify(el, new ElementModifiers { Opacity = opacity }); + ModifyVisual(el, new VisualModifiers { Opacity = opacity }); public static T Scale(this T el, global::System.Numerics.Vector3 scale) where T : Element => - Modify(el, new ElementModifiers { Scale = scale }); + ModifyVisual(el, new VisualModifiers { Scale = scale }); public static T Scale(this T el, float uniform) where T : Element => - Modify(el, new ElementModifiers { Scale = new global::System.Numerics.Vector3(uniform, uniform, 1f) }); + ModifyVisual(el, new VisualModifiers { Scale = new global::System.Numerics.Vector3(uniform, uniform, 1f) }); public static T Rotation(this T el, float degrees) where T : Element => - Modify(el, new ElementModifiers { Rotation = degrees }); + ModifyVisual(el, new VisualModifiers { Rotation = degrees }); public static T CenterPoint(this T el, global::System.Numerics.Vector3 center) where T : Element => - Modify(el, new ElementModifiers { CenterPoint = center }); + ModifyVisual(el, new VisualModifiers { CenterPoint = center }); // ── Typography (any Control or TextBlock) ───────────────────── // These set font properties via ElementModifiers, so they work on ANY element @@ -542,7 +542,34 @@ public static T WithToolTip(this T el, Element tooltip) where T : Element => /// Usage: Text("Hello").ApplyStyle("BodyTextBlockStyle") /// public static T ApplyStyle(this T el, string styleName) where T : Element => - el.OnMount(fe => fe.Style = (Style)Application.Current.Resources[styleName]); + el.OnMount(StyleApplier(styleName)); + + // #174 — cache the OnMount delegate per style name so an unchanged + // `.ApplyStyle("X")` chain reuses one delegate instance instead of + // allocating a fresh capturing closure + display class on every render. + // (OnMountAction is excluded from the diff equality — it runs at mount + // only — so this is a pure allocation cut, not a skip-path change.) + // Style names are a small, finite, app-defined set, so the cache is + // bounded in practice. As defense-in-depth against a pathological + // data-driven caller that passes unbounded distinct style names, the + // cache stops growing past StyleApplierCacheCap and falls back to a + // per-call delegate (the pre-#174 behavior) — correctness is unchanged, + // only the allocation optimization stops applying beyond the cap. + private const int StyleApplierCacheCap = 256; + private static readonly global::System.Collections.Concurrent.ConcurrentDictionary> _styleApplierCache = new(); + + private static Action StyleApplier(string styleName) + { + // Steady-state hit: lock-free read returns the cached delegate with no + // allocation (preserves #174). Count is only touched on a miss, which + // happens at most once per distinct style name. + if (_styleApplierCache.TryGetValue(styleName, out var cached)) + return cached; + if (_styleApplierCache.Count >= StyleApplierCacheCap) + return fe => fe.Style = (Style)Application.Current.Resources[styleName]; + return _styleApplierCache.GetOrAdd(styleName, + static name => fe => fe.Style = (Style)Application.Current.Resources[name]); + } // ════════════════════════════════════════════════════════════════ // Sugar extensions (typed, return concrete element type) @@ -1076,14 +1103,17 @@ el with // ── Background (Panel, Control, Border) ──────────────────────── /// - /// Sets the background from a color string. Allocates a new SolidColorBrush per call. - /// On hot render paths, prefer the overload with a cached brush. + /// Sets the background from a color string (named color or #RRGGBB / #AARRGGBB). + /// Returns a fresh, caller-independent brush per call (see + /// — the parsed color is cached, the brush is + /// not); the diff compares brushes structurally (color + opacity). Use the + /// overload to supply an explicit brush. /// public static T Background(this T el, string color) where T : Element => - Modify(el, new ElementModifiers { Background = BrushHelper.Parse(color) }); + ModifyVisual(el, new VisualModifiers { Background = BrushHelper.Parse(color) }); public static T Background(this T el, Brush brush) where T : Element => - Modify(el, new ElementModifiers { Background = brush }); + ModifyVisual(el, new VisualModifiers { Background = brush }); /// /// Sets the background from a WinUI theme resource. Resolves at render time @@ -1096,14 +1126,17 @@ public static T Background(this T el, ThemeRef theme) where T : Element => // ── Foreground (Control, TextBlock) ────────────────────────── /// - /// Sets the foreground from a color string. Allocates a new SolidColorBrush per call. - /// On hot render paths, prefer the overload with a cached brush. + /// Sets the foreground from a color string (named color or #RRGGBB / #AARRGGBB). + /// Returns a fresh, caller-independent brush per call (see + /// — the parsed color is cached, the brush is + /// not); the diff compares brushes structurally (color + opacity). Use the + /// overload to supply an explicit brush. /// public static T Foreground(this T el, string color) where T : Element => - Modify(el, new ElementModifiers { Foreground = BrushHelper.Parse(color) }); + ModifyVisual(el, new VisualModifiers { Foreground = BrushHelper.Parse(color) }); public static T Foreground(this T el, Brush brush) where T : Element => - Modify(el, new ElementModifiers { Foreground = brush }); + ModifyVisual(el, new VisualModifiers { Foreground = brush }); /// /// Sets the foreground from a WinUI theme resource. Resolves at render time @@ -1116,22 +1149,25 @@ public static T Foreground(this T el, ThemeRef theme) where T : Element => // ── CornerRadius (on Control and Border) ──────────────────────── public static T CornerRadius(this T el, double radius) where T : Element => - Modify(el, new ElementModifiers { CornerRadius = new Microsoft.UI.Xaml.CornerRadius(radius) }); + ModifyVisual(el, new VisualModifiers { CornerRadius = new Microsoft.UI.Xaml.CornerRadius(radius) }); public static T CornerRadius(this T el, double topLeft, double topRight, double bottomRight, double bottomLeft) where T : Element => - Modify(el, new ElementModifiers { CornerRadius = new Microsoft.UI.Xaml.CornerRadius(topLeft, topRight, bottomRight, bottomLeft) }); + ModifyVisual(el, new VisualModifiers { CornerRadius = new Microsoft.UI.Xaml.CornerRadius(topLeft, topRight, bottomRight, bottomLeft) }); // ── Border brush/thickness (on Control and Border) ───────────── /// - /// Sets the border from a color string. Allocates a new SolidColorBrush per call. - /// On hot render paths, prefer the overload with a cached brush. + /// Sets the border from a color string (named color or #RRGGBB / #AARRGGBB). + /// Returns a fresh, caller-independent brush per call (see + /// — the parsed color is cached, the brush is + /// not); the diff compares brushes structurally (color + opacity). Use the + /// overload to supply an explicit brush. /// public static T WithBorder(this T el, string color, double thickness = 1) where T : Element => - Modify(el, new ElementModifiers { BorderBrush = BrushHelper.Parse(color), BorderThickness = new Thickness(thickness) }); + ModifyVisual(el, new VisualModifiers { BorderBrush = BrushHelper.Parse(color), BorderThickness = new Thickness(thickness) }); public static T WithBorder(this T el, Brush brush, double thickness = 1) where T : Element => - Modify(el, new ElementModifiers { BorderBrush = brush, BorderThickness = new Thickness(thickness) }); + ModifyVisual(el, new VisualModifiers { BorderBrush = brush, BorderThickness = new Thickness(thickness) }); /// /// Sets the border from a WinUI theme resource. Resolves at render time @@ -2729,7 +2765,7 @@ public static SemanticElement Semantics(this T el, /// Routes through AnimationHelper so WithAnimation scopes animate the change. /// public static T Translation(this T el, float x, float y, float z) where T : Element => - Modify(el, new ElementModifiers { Translation = new global::System.Numerics.Vector3(x, y, z) }); + ModifyVisual(el, new VisualModifiers { Translation = new global::System.Numerics.Vector3(x, y, z) }); // ════════════════════════════════════════════════════════════════ // Compositor property animation (.Animate() modifier) @@ -2835,6 +2871,33 @@ public static T ScrollLinked(this T el, private static T Modify(T el, ElementModifiers mods) where T : Element => el with { Modifiers = el.Modifiers is not null ? el.Modifiers.Merge(mods) : mods }; + // #165/#157 — bucket-level merge entry points for pure-layout / pure-visual + // fluent modifiers. Routing through these avoids allocating a throwaway parent + // ElementModifiers (and the bucket sub-record its init shim builds) on every + // chained call: `.Margin().Width().Foreground()` merges the delta straight into + // the Layout / Visual slot instead of constructing a temporary ElementModifiers + // per step. Semantics are identical to Modify(el, new ElementModifiers { }): + // ElementModifiers.Merge copies every non-bucket field as `other.X ?? X` (i.e. + // unchanged) when only a bucket is supplied, and merges the bucket with the same + // `other ?? this` precedence used here. + private static T ModifyLayout(T el, LayoutModifiers delta) where T : Element + { + var mods = el.Modifiers; + if (mods is null) + return el with { Modifiers = new ElementModifiers { Layout = delta } }; + var merged = mods.Layout is not null ? mods.Layout.Merge(delta) : delta; + return el with { Modifiers = mods with { Layout = merged } }; + } + + private static T ModifyVisual(T el, VisualModifiers delta) where T : Element + { + var mods = el.Modifiers; + if (mods is null) + return el with { Modifiers = new ElementModifiers { Visual = delta } }; + var merged = mods.Visual is not null ? mods.Visual.Merge(delta) : delta; + return el with { Modifiers = mods with { Visual = merged } }; + } + private static T ModifyA11y(T el, AccessibilityModifiers a11y) where T : Element { var existing = el.Modifiers?.Accessibility; diff --git a/tests/Reactor.Tests/BrushHelperTests.cs b/tests/Reactor.Tests/BrushHelperTests.cs index 888f1efa3..d2c898982 100644 --- a/tests/Reactor.Tests/BrushHelperTests.cs +++ b/tests/Reactor.Tests/BrushHelperTests.cs @@ -1,3 +1,4 @@ +using System; using Xunit; namespace Microsoft.UI.Reactor.Tests; @@ -135,4 +136,53 @@ public void ParseHex_MixedCase() Assert.Equal(0xBB, color.G); Assert.Equal(0xCC, color.B); } + + // ── Case-insensitive color cache (Fix A) ───────────────────── + + [Fact] + public void ParseColor_IsCaseInsensitive() + { + // Named colors and hex resolve identically regardless of input casing. + Assert.Equal(BrushHelper.ParseColor("Red"), BrushHelper.ParseColor("red")); + Assert.Equal(BrushHelper.ParseColor("RED"), BrushHelper.ParseColor("red")); + Assert.Equal(BrushHelper.ParseColor("#FF0000"), BrushHelper.ParseColor("#ff0000")); + Assert.Equal(BrushHelper.ParseColor("TRANSPARENT"), BrushHelper.ParseColor("transparent")); + } + + [Fact] + public void ParseColor_Cache_Dedupes_Across_Casing() + { + // Teeth for Fix A: _colorCache uses OrdinalIgnoreCase, so the many + // casings of one color name collapse to a SINGLE cache entry. Value + // equality alone can't prove this (ParseColor lowercases before the + // switch, so even a case-sensitive cache would return equal colors) — + // the observable difference is the dedupe, which we prove by allocation: + // once one casing is cached, every other casing is a hit ⇒ no new entry + // and no factory ToLowerInvariant alloc. Case-sensitive keying would + // instead insert one entry per distinct casing (tens of KB here). + const string word = "transparent"; // 11 letters ⇒ >500 distinct casings + var variants = new string[500]; + for (int i = 0; i < variants.Length; i++) variants[i] = CasingOf(word, i); + + BrushHelper.ParseColor(word); // warm the single shared entry + foreach (var v in variants) BrushHelper.ParseColor(v); // also warm + JIT + + var before = GC.GetAllocatedBytesForCurrentThread(); + foreach (var v in variants) BrushHelper.ParseColor(v); // all cache hits + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + + Assert.True(allocated < 10_000, + $"case-insensitive ParseColor over 500 distinct casings allocated {allocated} bytes " + + "(expected ~0 — all should dedupe to one entry; case-sensitive keying would be far larger)"); + } + + private static string CasingOf(string word, int mask) + { + var chars = word.ToCharArray(); + for (int b = 0; b < chars.Length && b < 31; b++) + chars[b] = ((mask >> b) & 1) == 1 + ? char.ToUpperInvariant(chars[b]) + : char.ToLowerInvariant(chars[b]); + return new string(chars); + } } diff --git a/tests/Reactor.Tests/DeclarativeModifierTests.cs b/tests/Reactor.Tests/DeclarativeModifierTests.cs index 386ad3b42..c8fc928f0 100644 --- a/tests/Reactor.Tests/DeclarativeModifierTests.cs +++ b/tests/Reactor.Tests/DeclarativeModifierTests.cs @@ -155,12 +155,51 @@ public void Event_Handlers_Merge_Overwrites() } [Fact] - public void ModifiersEqual_Returns_False_When_Event_Handlers_Present() + public void ModifiersEqual_True_When_Same_Handler_Reference() { + // FLAGSHIP-1 — a reference-stable handler (memoized / non-capturing / + // method group) now takes the skip fast-path: the reconciler's Current* + // dispatch field already holds this exact delegate, so skipping Update is + // safe. Previously ANY handler present forced a full Update every render. Action handler = (s, e) => { }; var a = new ElementModifiers { OnTapped = handler }; var b = new ElementModifiers { OnTapped = handler }; - // Conservative: event handlers always cause update (delegate comparison unreliable) + Assert.True(Element.ModifiersEqual(a, b)); + } + + [Fact] + public void ModifiersEqual_False_When_Different_Handler_Reference() + { + // A freshly-captured closure each render is a different delegate instance, + // so the skip path is correctly declined and Update re-wires Current*. + Action handler1 = (s, e) => { }; + Action handler2 = (s, e) => { }; + var a = new ElementModifiers { OnTapped = handler1 }; + var b = new ElementModifiers { OnTapped = handler2 }; + Assert.False(Element.ModifiersEqual(a, b)); + } + + [Fact] + public void ModifiersEqual_False_When_Handler_Presence_Differs() + { + // null → non-null transition must force Update so the trampoline subscribes. + Action handler = (s, e) => { }; + var a = new ElementModifiers { OnTapped = handler }; + var b = new ElementModifiers { }; + Assert.False(Element.ModifiersEqual(a, b)); + Assert.False(Element.ModifiersEqual(b, a)); + } + + [Fact] + public void ModifiersEqual_False_When_Unlisted_Handler_Identity_Differs() + { + // Regression guard: handlers outside the historical 6-handler null-check + // (e.g. PointerEntered) are now compared too, closing the latent hole + // where a stale closure could be stranded on the skip path. + Action h1 = (s, e) => { }; + Action h2 = (s, e) => { }; + var a = new ElementModifiers { OnPointerEntered = h1 }; + var b = new ElementModifiers { OnPointerEntered = h2 }; Assert.False(Element.ModifiersEqual(a, b)); } diff --git a/tests/Reactor.Tests/PerfDiffSkipPathTests.cs b/tests/Reactor.Tests/PerfDiffSkipPathTests.cs new file mode 100644 index 000000000..92e1be80b --- /dev/null +++ b/tests/Reactor.Tests/PerfDiffSkipPathTests.cs @@ -0,0 +1,721 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.Input; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Input; +using static Microsoft.UI.Reactor.Factories; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests; + +/// +/// Regression coverage for the perf "restore diff skip-path + cut DSL/element +/// per-render allocations" work. These are pure C# record/equality tests — no +/// WinUI thread, and intentionally brush-free (constructing a SolidColorBrush +/// requires a UI thread). BrushHelper caches only the parsed Color (an +/// immutable value), exercised by the headless ParseColor tests below; the +/// public Parse still returns a fresh, caller-owned SolidColorBrush per +/// call, so there is no shared brush instance to validate on a real thread. +/// +/// The headline behaviour being locked down: an interactive grid cell whose only +/// per-render churn is handler identity / appended setters / re-applied attached +/// data must once again hit the skip path, +/// while any change that actually affects rendering still declines it. +/// +public class PerfDiffSkipPathTests +{ + // ════════════════════════════════════════════════════════════════ + // FLAGSHIP-1 — interactive elements regain the skip fast-path + // ════════════════════════════════════════════════════════════════ + + // A reference-stable handler (hoisted/memoized/method-group). The reconciler's + // ModifierEventHandlerState.Current* already holds this exact delegate, so the + // skip path dispatches identically — skipping is safe. + private static readonly Action SharedTap = (s, e) => { }; + + // The other half of the DataGrid cell shape — cells carry OnPointerPressed + + // OnTapped, every other routed slot null. Reference-stable across renders. + private static readonly Action SharedPointerPressed = (s, e) => { }; + + [Fact] + public void ShallowEquals_True_For_Interactive_Element_With_Stable_Handler() + { + var a = TextBlock("AAPL").OnTapped(SharedTap); + var b = TextBlock("AAPL").OnTapped(SharedTap); + // Pre-fix this returned false (any handler present ⇒ never skip), forcing a + // full Update on every selectable/interactive cell every render. + Assert.True(Element.ShallowEquals(a, b)); + } + + [Fact] + public void ShallowEquals_False_When_Handler_Identity_Changes() + { + Action h1 = (s, e) => { }; + Action h2 = (s, e) => { }; + var a = TextBlock("AAPL").OnTapped(h1); + var b = TextBlock("AAPL").OnTapped(h2); + // Distinct delegate instances (e.g. a freshly captured closure each render) + // must decline the skip so Update re-wires Current* to the new capture. + Assert.False(Element.ShallowEquals(a, b)); + } + + [Fact] + public void ShallowEquals_False_When_Handler_Added_Or_Removed() + { + var plain = TextBlock("AAPL"); + var interactive = TextBlock("AAPL").OnTapped(SharedTap); + // null → non-null presence change must run Update so the trampoline subscribes. + Assert.False(Element.ShallowEquals(plain, interactive)); + Assert.False(Element.ShallowEquals(interactive, plain)); + } + + [Fact] + public void ShallowEquals_False_When_OnUnmount_Changes_With_Stable_Handler() + { + // FLAGSHIP-1 lets a stable .OnTapped element take the skip path. The + // imperative .OnUnmount teardown is re-registered ONLY on the non-skip Update + // path (the reconciler's _onUnmountActions), and the latest registered + // delegate is what fires at unmount — so a *changed* teardown closure must + // decline the skip, or the stale (first-render) action is stranded and runs + // in place of the current one. Pre-FLAGSHIP-1 this was masked because any + // handler present forced Update every render; the per-slot ref-equality skip + // re-exposed it, so OnUnmountAction is compared by reference in ModifiersEqual. + Action teardown1 = _ => { }; + Action teardown2 = _ => { }; + + var changedA = TextBlock("AAPL").OnTapped(SharedTap).OnUnmount(teardown1); + var changedB = TextBlock("AAPL").OnTapped(SharedTap).OnUnmount(teardown2); + Assert.False(Element.ShallowEquals(changedA, changedB)); + + // A reference-stable teardown (same delegate instance) stays skip-eligible. + var stableA = TextBlock("AAPL").OnTapped(SharedTap).OnUnmount(teardown1); + var stableB = TextBlock("AAPL").OnTapped(SharedTap).OnUnmount(teardown1); + Assert.True(Element.ShallowEquals(stableA, stableB)); + + // Presence change (add / remove the teardown) must also decline the skip. + var withTeardown = TextBlock("AAPL").OnTapped(SharedTap).OnUnmount(teardown1); + var withoutTeardown = TextBlock("AAPL").OnTapped(SharedTap); + Assert.False(Element.ShallowEquals(withTeardown, withoutTeardown)); + Assert.False(Element.ShallowEquals(withoutTeardown, withTeardown)); + } + + [Fact] + public void ModifierCallbacksEqual_Every_Slot_Participates_By_Reference() + { + // FLAGSHIP-1 compares 21 routed-input handler slots by per-slot + // ReferenceEquals. This sweeps EACH slot to prove it participates: setting a + // slot to the same instance on both sides stays skip-eligible, while changing + // that one slot's instance (or adding/removing it) declines the skip. Guards + // against a forgotten slot or a mis-paired comparison (e.g. a.X vs a.X). One + // distinct instance pair per delegate type; only one slot is set per case, so + // a sibling mis-pair (a.OnKeyDown vs b.OnKeyUp) is caught too. Gesture and + // drag-drop slots are intentionally NOT in this set — see the companion + // ModifierCallbacksEqual_Ignores_Gesture_And_DragDrop_Slots_To_Match_Main. + Action sc1 = (s, e) => { }, sc2 = (s, e) => { }; + Action p1 = (s, e) => { }, p2 = (s, e) => { }; + Action tap1 = (s, e) => { }, tap2 = (s, e) => { }; + Action dt1 = (s, e) => { }, dt2 = (s, e) => { }; + Action rt1 = (s, e) => { }, rt2 = (s, e) => { }; + Action hd1 = (s, e) => { }, hd2 = (s, e) => { }; + Action k1 = (s, e) => { }, k2 = (s, e) => { }; + Action cr1 = (s, e) => { }, cr2 = (s, e) => { }; + Action f1 = (s, e) => { }, f2 = (s, e) => { }; + Action ak1 = (s, e) => { }, ak2 = (s, e) => { }; + + // (name, equalA, equalB [same instance as A], diff [different instance]) + var cases = new (string Name, ElementModifiers A, ElementModifiers B, ElementModifiers Diff)[] + { + ("OnSizeChanged", new() { OnSizeChanged = sc1 }, new() { OnSizeChanged = sc1 }, new() { OnSizeChanged = sc2 }), + ("OnPointerPressed", new() { OnPointerPressed = p1 }, new() { OnPointerPressed = p1 }, new() { OnPointerPressed = p2 }), + ("OnPointerMoved", new() { OnPointerMoved = p1 }, new() { OnPointerMoved = p1 }, new() { OnPointerMoved = p2 }), + ("OnPointerReleased", new() { OnPointerReleased = p1 }, new() { OnPointerReleased = p1 }, new() { OnPointerReleased = p2 }), + ("OnPointerEntered", new() { OnPointerEntered = p1 }, new() { OnPointerEntered = p1 }, new() { OnPointerEntered = p2 }), + ("OnPointerExited", new() { OnPointerExited = p1 }, new() { OnPointerExited = p1 }, new() { OnPointerExited = p2 }), + ("OnPointerCanceled", new() { OnPointerCanceled = p1 }, new() { OnPointerCanceled = p1 }, new() { OnPointerCanceled = p2 }), + ("OnPointerCaptureLost", new() { OnPointerCaptureLost = p1 }, new() { OnPointerCaptureLost = p1 }, new() { OnPointerCaptureLost = p2 }), + ("OnPointerWheelChanged",new() { OnPointerWheelChanged = p1 }, new() { OnPointerWheelChanged = p1 }, new() { OnPointerWheelChanged = p2 }), + ("OnTapped", new() { OnTapped = tap1 }, new() { OnTapped = tap1 }, new() { OnTapped = tap2 }), + ("OnDoubleTapped", new() { OnDoubleTapped = dt1 }, new() { OnDoubleTapped = dt1 }, new() { OnDoubleTapped = dt2 }), + ("OnRightTapped", new() { OnRightTapped = rt1 }, new() { OnRightTapped = rt1 }, new() { OnRightTapped = rt2 }), + ("OnHolding", new() { OnHolding = hd1 }, new() { OnHolding = hd1 }, new() { OnHolding = hd2 }), + ("OnKeyDown", new() { OnKeyDown = k1 }, new() { OnKeyDown = k1 }, new() { OnKeyDown = k2 }), + ("OnKeyUp", new() { OnKeyUp = k1 }, new() { OnKeyUp = k1 }, new() { OnKeyUp = k2 }), + ("OnPreviewKeyDown", new() { OnPreviewKeyDown = k1 }, new() { OnPreviewKeyDown = k1 }, new() { OnPreviewKeyDown = k2 }), + ("OnPreviewKeyUp", new() { OnPreviewKeyUp = k1 }, new() { OnPreviewKeyUp = k1 }, new() { OnPreviewKeyUp = k2 }), + ("OnCharacterReceived", new() { OnCharacterReceived = cr1 }, new() { OnCharacterReceived = cr1 }, new() { OnCharacterReceived = cr2 }), + ("OnGotFocus", new() { OnGotFocus = f1 }, new() { OnGotFocus = f1 }, new() { OnGotFocus = f2 }), + ("OnLostFocus", new() { OnLostFocus = f1 }, new() { OnLostFocus = f1 }, new() { OnLostFocus = f2 }), + ("OnAccessKeyDisplayRequested", new() { OnAccessKeyDisplayRequested = ak1 }, new() { OnAccessKeyDisplayRequested = ak1 }, new() { OnAccessKeyDisplayRequested = ak2 }), + }; + + var empty = new ElementModifiers(); + foreach (var (name, a, b, diff) in cases) + { + Assert.True(Element.ModifiersEqual(a, b), $"{name}: same handler instance ⇒ skip-eligible"); + Assert.False(Element.ModifiersEqual(a, diff), $"{name}: changed handler instance ⇒ must Update"); + Assert.False(Element.ModifiersEqual(a, empty), $"{name}: adding handler ⇒ must Update"); + Assert.False(Element.ModifiersEqual(empty, a), $"{name}: removing handler ⇒ must Update"); + } + + // Tracks the routed-handler slot count in Element.ModifierCallbacksEqual; + // bump both together. Gesture / drag-drop slots are intentionally excluded — + // see ModifierCallbacksEqual_Ignores_Gesture_And_DragDrop_Slots_To_Match_Main. + Assert.Equal(21, cases.Length); + } + + [Fact] + public void ModifierCallbacksEqual_Ignores_Gesture_And_DragDrop_Slots_To_Match_Main() + { + // BEHAVIOR-PRESERVATION GUARD (companion to the routed-slot sweep above). + // The historical diff never compared the gesture (Pan/Pinch/Rotate/LongPress) + // or drag-drop (DragSource/DropTarget) slots: they dispatch through separate + // per-element gesture/drag state, NOT ModifierEventHandlerState.Current*. + // Adding them to ModifierCallbacksEqual would force Update where the framework + // previously skipped, re-arming an in-flight gesture mid-interaction — the + // long-press Began+Ended double-dispatch regression (GestureTests + // .Interactive_OnLongPress_FiresAfterHold). So a gesture/drag closure that + // changes identity every render MUST stay skip-eligible, exactly as before. + // Do NOT "complete" the sweep by adding these slots — that reintroduces the bug. + PanGestureConfig pan1 = new(_ => { }), pan2 = new(_ => { }); + PinchGestureConfig pin1 = new(_ => { }), pin2 = new(_ => { }); + RotateGestureConfig rot1 = new(_ => { }), rot2 = new(_ => { }); + LongPressGestureConfig lp1 = new(_ => { }), lp2 = new(_ => { }); + DragSourceConfig ds1 = new(() => default!), ds2 = new(() => default!); + DropTargetConfig dp1 = new(), dp2 = new(); + + var empty = new ElementModifiers(); + var pairs = new (string Name, ElementModifiers One, ElementModifiers Changed)[] + { + ("Pan", new() { Pan = pan1 }, new() { Pan = pan2 }), + ("Pinch", new() { Pinch = pin1 }, new() { Pinch = pin2 }), + ("Rotate", new() { Rotate = rot1 }, new() { Rotate = rot2 }), + ("LongPress", new() { LongPress = lp1 }, new() { LongPress = lp2 }), + ("DragSource", new() { DragSource = ds1 }, new() { DragSource = ds2 }), + ("DropTarget", new() { DropTarget = dp1 }, new() { DropTarget = dp2 }), + }; + + foreach (var (name, one, changed) in pairs) + { + Assert.True(Element.ModifiersEqual(one, changed), $"{name}: changed closure must stay skip-eligible (matches main)"); + Assert.True(Element.ModifiersEqual(one, empty), $"{name}: present-vs-absent must stay skip-eligible (matches main)"); + Assert.True(Element.ModifiersEqual(empty, one), $"{name}: absent-vs-present must stay skip-eligible (matches main)"); + } + } + + [Fact] + public void ShallowEquals_GridCell_Stable_Primary_Plus_Changing_Secondary_Declines_Skip() + { + // Combined FLAGSHIP-1 regression at the ShallowEquals/element level, mirroring + // the realistic DataGrid cell shape (OnPointerPressed + OnTapped, every other + // routed slot null). The per-slot sweep above proves each slot participates in + // isolation; this locks the combined case the coordinator called out: a cell + // with reference-STABLE primary handlers must STILL decline the skip the instant + // ANY secondary routed slot (OnDoubleTapped/OnGotFocus/OnLostFocus/…) changes + // identity — otherwise that secondary's ModifierEventHandlerState.Current* + // (refreshed only on the non-skip Update path) would dispatch a stale closure. + static Element Cell( + Action? dbl = null, + Action? got = null, + Action? lost = null) + { + var cell = TextBlock("AAPL").OnPointerPressed(SharedPointerPressed).OnTapped(SharedTap); + if (dbl is not null) cell = cell.OnDoubleTapped(dbl); + if (got is not null) cell = cell.OnGotFocus(got); + if (lost is not null) cell = cell.OnLostFocus(lost); + return cell; + } + + // GRID WIN INTACT — the actual cell shape (stable primary handlers, all + // secondaries null) stays reference-stable across renders ⇒ skips. + Assert.True(Element.ShallowEquals(Cell(), Cell())); + + // STALE-DISPATCH GUARDS — stable primaries, but a per-render (changing) + // secondary closure ⇒ must NOT skip so the secondary's Current* is refreshed. + Action dt1 = (s, e) => { }, dt2 = (s, e) => { }; + Assert.False(Element.ShallowEquals(Cell(dbl: dt1), Cell(dbl: dt2))); + + Action g1 = (s, e) => { }, g2 = (s, e) => { }; + Assert.False(Element.ShallowEquals(Cell(got: g1), Cell(got: g2))); + + Action l1 = (s, e) => { }, l2 = (s, e) => { }; + Assert.False(Element.ShallowEquals(Cell(lost: l1), Cell(lost: l2))); + + // A reference-STABLE secondary (same instance both renders) keeps the cell + // skip-eligible — the win is not over-narrowed to "any secondary ⇒ never skip". + Assert.True(Element.ShallowEquals(Cell(dbl: dt1), Cell(dbl: dt1))); + } + + // ════════════════════════════════════════════════════════════════ + // Setters (.Set) — the imperative escape hatch always re-applies on + // Update; only the no-setter case is skip-eligible. A reference-stable + // setter is NOT skippable: it is an apply-time imperative write that can + // read mutable external state (statics/singletons), and non-capturing + // lambdas are compiler-cached to a stable identity — so identity-equality + // must not be mistaken for "the write is unnecessary". (Safety counterpart + // to FLAGSHIP-1, where handlers dispatch later via Current* and are safe.) + // ════════════════════════════════════════════════════════════════ + + // Same source location, non-capturing lambda → the compiler caches a single + // static delegate, so both renders' Setters arrays hold the identical instance. + private static Element BuildWithStaticSetter() => + TextBlock("Hdr").Set(tb => tb.FontSize = 12); + + // Captures `size` → a fresh display-class + delegate per call. + private static Element BuildWithCapturingSetter(double size) => + TextBlock("Hdr").Set(tb => tb.FontSize = size); + + [Fact] + public void ShallowEquals_True_When_Neither_Element_Has_Setters() + { + // Both Setters arrays are the empty singleton → no imperative writes to + // re-apply → the cell stays skip-eligible. + Assert.True(Element.ShallowEquals(TextBlock("Hdr"), TextBlock("Hdr"))); + } + + [Fact] + public void ShallowEquals_False_For_Stable_Set_Chain() + { + // Even a cached, reference-stable (non-capturing) setter must decline the + // skip: `.Set` is an apply-time imperative write whose effect can depend on + // mutable external state, so the reconciler has to re-apply it every update. + Assert.False(Element.ShallowEquals(BuildWithStaticSetter(), BuildWithStaticSetter())); + } + + [Fact] + public void ShallowEquals_False_For_Capturing_Set_Chain() + { + // A capturing setter allocates a new delegate per render and likewise + // declines the fast-path. + Assert.False(Element.ShallowEquals(BuildWithCapturingSetter(12), BuildWithCapturingSetter(12))); + } + + [Fact] + public void SettersEqual_True_For_Same_Instance_Or_Both_Empty() + { + Action a = () => { }; + var arr = new[] { a }; + Assert.True(Element.SettersEqual(arr, arr)); // same instance + Assert.True(Element.SettersEqual(Array.Empty(), new Action[0])); // both empty + Assert.True(Element.SettersEqual(null, null)); // both null + Assert.True(Element.SettersEqual(Array.Empty(), null)); // empty + null + } + + [Fact] + public void SettersEqual_False_When_Setters_Present_In_Distinct_Arrays() + { + Action shared = () => { }; + // Distinct arrays holding the IDENTICAL (cached/stable) delegate must still + // decline — the imperative write has to re-run every update. + Assert.False(Element.SettersEqual(new[] { shared }, new[] { shared })); + Assert.False(Element.SettersEqual(new[] { shared }, Array.Empty())); + } + + // ════════════════════════════════════════════════════════════════ + // #159 — ModifiersEqual hoists Layout/Visual sub-records (value compare) + // ════════════════════════════════════════════════════════════════ + + [Fact] + public void ModifiersEqual_True_For_Identical_Layout_And_Visual_Chain() + { + var a = TextBlock("x").Width(120).Margin(8).Opacity(0.5).Scale(1.5f); + var b = TextBlock("x").Width(120).Margin(8).Opacity(0.5).Scale(1.5f); + Assert.True(Element.ModifiersEqual(a.Modifiers, b.Modifiers)); + Assert.True(Element.ShallowEquals(a, b)); + } + + [Fact] + public void ModifiersEqual_False_When_Layout_Field_Differs() + { + var a = TextBlock("x").Width(120); + var b = TextBlock("x").Width(121); + Assert.False(Element.ModifiersEqual(a.Modifiers, b.Modifiers)); + } + + [Fact] + public void ModifiersEqual_False_When_RequestedTheme_Differs() + { + // RequestedTheme lives in LayoutModifiers; the hoisted record value-equality + // now covers it (the historical explicit field list omitted it). + var a = TextBlock("x").RequestedTheme(ElementTheme.Dark); + var b = TextBlock("x").RequestedTheme(ElementTheme.Light); + Assert.False(Element.ModifiersEqual(a.Modifiers, b.Modifiers)); + } + + [Fact] + public void ModifiersEqual_False_When_Visual_Opacity_Differs() + { + var a = TextBlock("x").Opacity(1.0); + var b = TextBlock("x").Opacity(0.25); + Assert.False(Element.ModifiersEqual(a.Modifiers, b.Modifiers)); + } + + [Fact] + public void ModifiersEqual_False_When_Visual_Scale_Differs() + { + // Scale lives in VisualModifiers and was historically omitted from the + // per-field compare (a latent skip bug); it must now be honored. + var a = TextBlock("x").Scale(1.0f); + var b = TextBlock("x").Scale(2.0f); + Assert.False(Element.ModifiersEqual(a.Modifiers, b.Modifiers)); + } + + // ════════════════════════════════════════════════════════════════ + // #155 — Attached single-slot dictionary (per-cell .Grid alloc cut) + // ════════════════════════════════════════════════════════════════ + + [Fact] + public void Grid_Attached_RoundTrips_Through_SingleAttachedDictionary() + { + var el = TextBlock("cell").Grid(2, 3); + Assert.NotNull(el.Attached); + Assert.Single(el.Attached!); + var ga = el.GetAttached(); + Assert.NotNull(ga); + Assert.Equal(2, ga!.Row); + Assert.Equal(3, ga.Column); + Assert.True(el.Attached.ContainsKey(typeof(GridAttached))); + } + + [Fact] + public void Grid_Reapplied_Same_Position_Is_AttachedEqual_And_Skips() + { + var a = TextBlock("cell").Grid(2, 3); + var b = TextBlock("cell").Grid(2, 3); + Assert.True(Element.AttachedEqual(a.Attached, b.Attached)); + Assert.True(Element.ShallowEquals(a, b)); + } + + [Fact] + public void Grid_Different_Position_Is_Not_AttachedEqual() + { + var a = TextBlock("cell").Grid(2, 3); + var b = TextBlock("cell").Grid(4, 5); + Assert.False(Element.AttachedEqual(a.Attached, b.Attached)); + Assert.False(Element.ShallowEquals(a, b)); + } + + [Fact] + public void Second_Distinct_Attached_Type_Promotes_To_Full_Dictionary() + { + // The single-slot dictionary is an optimization for the common one-value + // case; a second distinct attached type must still materialize correctly. + var el = TextBlock("cell").Grid(1, 1).WrapGridRowSpan(2); + Assert.Equal(2, el.Attached!.Count); + Assert.NotNull(el.GetAttached()); + Assert.NotNull(el.GetAttached()); + Assert.Equal(2, el.GetAttached()!.RowSpan); + } + + [Fact] + public void AttachedEqual_SingleEntry_FastPath_Matches_Semantics() + { + // The #155 single-slot fast path in AttachedEqual must return exactly what the + // foreach it replaces would, including the asymmetric branches. + var grid23 = TextBlock("c").Grid(2, 3); + var grid23b = TextBlock("c").Grid(2, 3); + var grid45 = TextBlock("c").Grid(4, 5); + var wrap = TextBlock("c").WrapGridRowSpan(2); // Single, different key + + Assert.True(Element.AttachedEqual(grid23.Attached, grid23b.Attached)); // same key+value + Assert.False(Element.AttachedEqual(grid23.Attached, grid45.Attached)); // same key, diff value + Assert.False(Element.AttachedEqual(grid23.Attached, wrap.Attached)); // diff key (a is Single) + Assert.False(Element.AttachedEqual(wrap.Attached, grid23.Attached)); // diff key (other order) + + // Non-Single (raw Dictionary, Count 1) vs Single exercises the `b is Single` branch. + var ga = grid23.GetAttached()!; + var rawSameKey = new Dictionary { [typeof(GridAttached)] = ga }; + Assert.True(Element.AttachedEqual(rawSameKey, grid23b.Attached)); + var rawDiffVal = new Dictionary { [typeof(GridAttached)] = grid45.GetAttached()! }; + Assert.False(Element.AttachedEqual(rawDiffVal, grid23.Attached)); + } + + [Fact] + public void AttachedEqual_SingleEntry_DoesNotAllocate() + { + // Copilot review (a6adfdd8): `foreach` over the IReadOnlyDictionary interface + // made SingleAttachedDictionary's yield-based enumerator allocate an iterator + // per diff, partially defeating #155's per-cell allocation cut. The single-slot + // fast path must be allocation-free. (Old foreach path: ~10k iterator objects.) + var a = TextBlock("cell").Grid(2, 3); + var b = TextBlock("cell").Grid(2, 3); + Element.AttachedEqual(a.Attached, b.Attached); // JIT + warm up + + var before = GC.GetAllocatedBytesForCurrentThread(); + var acc = true; + for (int i = 0; i < 10_000; i++) acc &= Element.AttachedEqual(a.Attached, b.Attached); + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + + Assert.True(acc); + Assert.True(allocated < 2_000, $"single-entry AttachedEqual allocated {allocated} bytes over 10k calls (expected ~0)"); + } + + // ════════════════════════════════════════════════════════════════ + // Dsl micro-opts — behaviour preserved (#170 / #171 / #172 / #173) + // ════════════════════════════════════════════════════════════════ + + [Fact] + public void ForEach_IReadOnlyList_FastPath_Preserves_Order_And_Content() + { + var items = new List { "a", "b", "c" }; + var group = Assert.IsType(ForEach(items, s => TextBlock(s))); + Assert.Equal( + new[] { "a", "b", "c" }, + group.Children.Select(c => Assert.IsType(c).Content)); + } + + [Fact] + public void ForEach_NonList_Enumerable_Matches_FastPath() + { + // A Where-iterator is IEnumerable but not IReadOnlyList → exercises the + // fallback; result must match the fast-path exactly (same order/content). + IEnumerable seq = new List { "a", "b", "c" }.Where(_ => true); + var group = Assert.IsType(ForEach(seq, s => TextBlock(s))); + Assert.Equal( + new[] { "a", "b", "c" }, + group.Children.Select(c => Assert.IsType(c).Content)); + } + + [Fact] + public void ForEach_Indexed_Overload_Preserves_Index_And_Content() + { + var items = new List { "x", "y" }; + var group = Assert.IsType(ForEach(items, (s, i) => TextBlock($"{i}:{s}"))); + Assert.Equal( + new[] { "0:x", "1:y" }, + group.Children.Select(c => Assert.IsType(c).Content)); + } + + [Fact] + public void ForEach_Empty_Input_Produces_Empty_Group() + { + // Both the IReadOnlyList fast-path and the indexed overload must yield an + // empty child set (no off-by-one, no phantom element) for empty input. + var list = Assert.IsType(ForEach(new List(), s => TextBlock(s))); + Assert.Empty(list.Children); + + var indexed = Assert.IsType(ForEach(new List(), (s, i) => TextBlock(s))); + Assert.Empty(indexed.Children); + + IEnumerable emptySeq = new List().Where(_ => true); + var seq = Assert.IsType(ForEach(emptySeq, s => TextBlock(s))); + Assert.Empty(seq.Children); + } + + [Fact] + public void FilterChildren_Drops_Nulls_And_Flattens_One_Group_Level() + { + // #173 two-pass exact-array path (triggered by the null) drops nulls. + var filtered = Assert.IsType(Group(TextBlock("a"), null, TextBlock("b"))); + Assert.Equal(2, filtered.Children.Length); + + // Nested GroupElement is flattened one level. + var nested = Assert.IsType( + Group(Group(TextBlock("a"), TextBlock("b")), TextBlock("c"))); + Assert.Equal(3, nested.Children.Length); + } + + [Fact] + public void UniformGrid_Builds_One_Star_Track_Per_Item() + { + var grid = UniformGrid(Orientation.Horizontal, TextBlock("a"), TextBlock("b"), TextBlock("c")); + Assert.Equal(3, grid.Children.Length); + Assert.Equal(3, grid.Definition.Columns.Length); + Assert.Single(grid.Definition.Rows); + } + + [Fact] + public void UniformGrid_Does_Not_Mutate_Caller_Items_Array() + { + // FilterChildren's #173 fast path returns a null-free input array aliased + // (no copy). UniformGrid must position cells into its OWN array — writing + // .Grid(...) wrappers back into `items` would corrupt a caller-supplied, + // reusable array. Lock that in: the source array's elements must be + // unchanged (and un-positioned) after the call. + var cellA = TextBlock("a"); + var cellB = TextBlock("b"); + var items = new Element?[] { cellA, cellB }; + + var grid = UniformGrid(Orientation.Vertical, items); + + // Source array untouched — same instances, no Grid attached-data added. + Assert.Same(cellA, items[0]); + Assert.Same(cellB, items[1]); + Assert.Null(((Element)items[0]!).GetAttached()); + Assert.Null(((Element)items[1]!).GetAttached()); + + // The grid's own children ARE positioned. + Assert.Equal(2, grid.Children.Length); + Assert.NotNull(grid.Children[0].GetAttached()); + Assert.Equal(1, grid.Children[1].GetAttached()!.Row); + } + + [Fact] + public void InterspersedGrid_Builds_Item_And_Separator_Tracks() + { + var grid = InterspersedGrid( + Orientation.Horizontal, + new Element[] { TextBlock("a"), TextBlock("b"), TextBlock("c") }, + new double[] { 1, 1, 1 }, + 6, + _ => TextBlock("|")); + // 3 items + 2 separators = 5 tracks/children. + Assert.Equal(5, grid.Children.Length); + Assert.Equal(5, grid.Definition.Columns.Length); + Assert.Single(grid.Definition.Rows); + } + + // ════════════════════════════════════════════════════════════════ + // #168 — BrushHelper.ParseColor cache. The string→Color parse is + // cached (an immutable value, safe to share); the public Parse still + // returns a FRESH SolidColorBrush per call (a brush is a mutable, + // thread-affine DependencyObject and must not be shared across + // controls). These headless tests lock down the cached color parse. + // ════════════════════════════════════════════════════════════════ + + [Theory] + [InlineData("red", 255, 255, 0, 0)] + [InlineData("transparent", 0, 0, 0, 0)] + [InlineData("#FF8800", 255, 255, 0x88, 0)] + [InlineData("#8000FF00", 0x80, 0, 255, 0)] + public void ParseColor_Resolves_Named_And_Hex(string input, int a, int r, int g, int b) + { + var color = BrushHelper.ParseColor(input); + Assert.Equal((byte)a, color.A); + Assert.Equal((byte)r, color.R); + Assert.Equal((byte)g, color.G); + Assert.Equal((byte)b, color.B); + } + + [Fact] + public void ParseColor_Is_Deterministic_Across_Calls() + { + // Repeated parses (served from the string cache on the second call) must + // yield the same Color value. + Assert.Equal(BrushHelper.ParseColor("#123456"), BrushHelper.ParseColor("#123456")); + Assert.Equal(BrushHelper.ParseColor("blue"), BrushHelper.ParseColor("blue")); + } + + // ════════════════════════════════════════════════════════════════ + // #165 / #157 — ModifyLayout / ModifyVisual fluent helpers merge the + // delta straight into the Layout / Visual bucket (skipping a throwaway + // parent ElementModifiers + shim-built bucket per chained call). The + // resulting ElementModifiers must be value-identical to the historical + // `Modify(el, new ElementModifiers { })` path so the diff and + // reconciler see no behavioural change — only fewer allocations. + // ════════════════════════════════════════════════════════════════ + + [Fact] + public void Fluent_Layout_Chain_Equals_HandBuilt_Modifiers() + { + var fluent = TextBlock("x").Margin(8).Width(120).Height(40).MinWidth(10); + var handBuilt = new ElementModifiers + { + Margin = new Thickness(8), + Width = 120, + Height = 40, + MinWidth = 10, + }; + Assert.Equal(handBuilt, fluent.Modifiers); + // Every layout field landed in a single Layout bucket; no Visual allocated. + Assert.NotNull(fluent.Modifiers!.Layout); + Assert.Null(fluent.Modifiers.Visual); + } + + [Fact] + public void Fluent_Visual_Chain_Equals_HandBuilt_Modifiers() + { + var fluent = TextBlock("x").Opacity(0.5).Scale(2f).Rotation(45f) + .CornerRadius(4); + var handBuilt = new ElementModifiers + { + Opacity = 0.5, + Scale = new global::System.Numerics.Vector3(2f, 2f, 1f), + Rotation = 45f, + CornerRadius = new Microsoft.UI.Xaml.CornerRadius(4), + }; + Assert.Equal(handBuilt, fluent.Modifiers); + Assert.NotNull(fluent.Modifiers!.Visual); + Assert.Null(fluent.Modifiers.Layout); + } + + [Fact] + public void Fluent_Mixed_Chain_Populates_Both_Buckets_Like_HandBuilt() + { + var fluent = TextBlock("x").Margin(8).Opacity(0.5).Width(120).Rotation(90f); + var handBuilt = new ElementModifiers + { + Margin = new Thickness(8), + Width = 120, + Opacity = 0.5, + Rotation = 90f, + }; + Assert.Equal(handBuilt, fluent.Modifiers); + } + + [Fact] + public void ModifyLayout_Preserves_Existing_NonBucket_Field() + { + // IsEnabled is a top-level (non-bucketed) ElementModifiers field. Routing the + // following .Margin() through ModifyLayout must not drop it — the bucket merge + // only touches the Layout slot and copies every other field unchanged. + var fluent = TextBlock("x").IsEnabled(false).Margin(8); + var handBuilt = new ElementModifiers { IsEnabled = false, Margin = new Thickness(8) }; + Assert.Equal(handBuilt, fluent.Modifiers); + Assert.False(fluent.Modifiers!.IsEnabled); + Assert.Equal(new Thickness(8), fluent.Modifiers.Margin); + } + + [Fact] + public void ModifyVisual_Preserves_Existing_Layout_Bucket() + { + // A pure-visual modifier applied after a pure-layout one must leave the + // existing Layout bucket intact (regression guard for the `mods with { Visual }` + // merge not clobbering Layout). + var fluent = TextBlock("x").Width(120).Opacity(0.5); + Assert.Equal(120, fluent.Modifiers!.Width); + Assert.Equal(0.5, fluent.Modifiers.Opacity); + Assert.Equal(new ElementModifiers { Width = 120, Opacity = 0.5 }, fluent.Modifiers); + } + + [Fact] + public void Fluent_Layout_Chain_Is_ShallowEquals_Across_Identical_Builds() + { + // The end-to-end skip-path payoff: an unchanged layout/visual chain compares + // equal so the cell takes the fast path. + var a = TextBlock("x").Margin(8).Width(120).Opacity(0.5); + var b = TextBlock("x").Margin(8).Width(120).Opacity(0.5); + Assert.True(Element.ShallowEquals(a, b)); + } + + // ════════════════════════════════════════════════════════════════ + // #174 — ApplyStyle caches the OnMount delegate per style name so an + // unchanged `.ApplyStyle("X")` chain reuses one delegate instance + // instead of allocating a fresh capturing closure + display class every + // render. (OnMountAction is intentionally excluded from the diff + // equality — it runs at mount only — so this is a pure allocation cut, + // not a skip-path change.) + // ════════════════════════════════════════════════════════════════ + + [Fact] + public void ApplyStyle_Same_Name_Produces_ReferenceStable_OnMountAction() + { + var a = TextBlock("x").ApplyStyle("BodyTextBlockStyle"); + var b = TextBlock("x").ApplyStyle("BodyTextBlockStyle"); + // Same cached delegate instance for the same style name — no fresh closure + // allocated on the second (and every subsequent) render. + Assert.Same(a.Modifiers!.OnMountAction, b.Modifiers!.OnMountAction); + } + + [Fact] + public void ApplyStyle_Different_Name_Produces_Distinct_OnMountAction() + { + var a = TextBlock("x").ApplyStyle("BodyTextBlockStyle"); + var b = TextBlock("x").ApplyStyle("TitleTextBlockStyle"); + // Distinct style names map to distinct cached delegates (each applies its + // own resource key). + Assert.NotSame(a.Modifiers!.OnMountAction, b.Modifiers!.OnMountAction); + } +}