Fix `nth-child` styles on virtualized lists (#13770)

* Don't alter state in properties.

The `IValueEntry.HasValue` and `ValueFrame.IsActive` properties could alter state, which meant that when inspecting objects with these properties in a debugger, the state got altered by observing it. Make them methods.

* Deleted unused file.

* Add failing test for #12381.

* Use the index from the event in EvaluateIsActive.

Break the rules in this case.

Fixes #12381

---------

Co-authored-by: Max Katz <maxkatz6@outlook.com>

src/Avalonia.Base/PropertyStore/BindingEntryBase.cs

@@ -49,15 +49,6 @@ namespace Avalonia.PropertyStore
                 _uncommon = new() { _hasDataValidation = true };
         }
 
-        public bool HasValue
-        {
-            get
-            {
-                Start(produceValue: false);
-                return _hasValue;
-            }
-        }
-
         public bool IsSubscribed => _subscription is not null;
         public AvaloniaProperty Property { get; }
         AvaloniaProperty IValueEntry.Property => Property;
@@ -70,6 +61,12 @@ namespace Avalonia.PropertyStore
             BindingCompleted();
         }
 
+        public bool HasValue()
+        {
+            Start(produceValue: false);
+            return _hasValue;
+        }
+
         public TValue GetValue()
         {
             Start(produceValue: false);

src/Avalonia.Base/PropertyStore/IValueEntry.cs

@@ -8,13 +8,16 @@ namespace Avalonia.PropertyStore
     /// </summary>
     internal interface IValueEntry
     {
-        bool HasValue { get; }
-
         /// <summary>
         /// Gets the property that this value applies to.
         /// </summary>
         AvaloniaProperty Property { get; }
 
+        /// <summary>
+        /// Checks whether the entry has a value, starting the entry if necessary.
+        /// </summary>
+        bool HasValue();
+
         /// <summary>
         /// Gets the value associated with the entry.
         /// </summary>

src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs

@@ -19,13 +19,13 @@ namespace Avalonia.PropertyStore
         }
 
         public StyledProperty<T> Property { get; }
-        public bool HasValue => true;
         AvaloniaProperty IValueEntry.Property => Property;
 
         public void Unsubscribe() { }
 
         public void Dispose() => _owner.OnEntryDisposed(this);
 
+        bool IValueEntry.HasValue() => true;
         object? IValueEntry.GetValue() => _value;
         T IValueEntry<T>.GetValue() => _value;
 

src/Avalonia.Base/PropertyStore/ValueFrame.cs

@@ -27,7 +27,7 @@ namespace Avalonia.PropertyStore
         }
 
         public int EntryCount => _index.Count;
-        public bool IsActive => GetIsActive(out _);
+        public bool IsActive() => GetIsActive(out _);
         public ValueStore? Owner => !_isShared ? _owner : 
             throw new AvaloniaInternalException("Cannot get owner for shared ValueFrame");
         public BindingPriority Priority { get; }

src/Avalonia.Base/PropertyStore/ValueStore.cs

@@ -843,7 +843,7 @@ namespace Avalonia.PropertyStore
                     // evaluated last as it can cause bindings to be subscribed.
                     if (foundEntry &&
                         HasHigherPriority(entry!, priority, current, changedValueEntry) && 
-                        entry!.HasValue)
+                        entry!.HasValue())
                     {
                         if (current is not null)
                         {
@@ -911,7 +911,7 @@ namespace Avalonia.PropertyStore
                 {
                     var frame = _frames[i];
 
-                    if (!frame.IsActive)
+                    if (!frame.IsActive())
                         continue;
 
                     var priority = frame.Priority;
@@ -927,7 +927,7 @@ namespace Avalonia.PropertyStore
                         if (!HasHigherPriority(entry, priority, effectiveValue, changedValueEntry))
                             continue;
 
-                        if (!entry.HasValue)
+                        if (!entry.HasValue())
                             continue;
 
                         if (effectiveValue is not null)

src/Avalonia.Base/Styling/Activators/NthChildActivator.cs

@@ -1,4 +1,5 @@
-using Avalonia.LogicalTree;
+using System.Collections.Generic;
+using Avalonia.LogicalTree;
 
 namespace Avalonia.Styling.Activators
 {
@@ -12,7 +13,7 @@ namespace Avalonia.Styling.Activators
         private readonly int _step;
         private readonly int _offset;
         private readonly bool _reversed;
-        private int? _index;
+        private int _index = -1;
 
         public NthChildActivator(
             ILogical control,
@@ -28,7 +29,7 @@ namespace Avalonia.Styling.Activators
 
         protected override bool EvaluateIsActive()
         {
-            var index = _index ?? _provider.GetChildIndex(_control);
+            var index = _index >= 0 ? _index : _provider.GetChildIndex(_control);
             return NthChildSelector.Evaluate(index, _provider, _step, _offset, _reversed).IsMatch;
         }
 
@@ -50,26 +51,25 @@ namespace Avalonia.Styling.Activators
             // 3. We're a reversed (nth-last-child) selector and total count has changed
             switch (e.Action)
             {
-                // We're using the _index field to pass the index of the child to EvaluateIsActive
-                // *only* when the active state is re-evaluated via this event handler. The docs
-                // for EvaluateIsActive say:
+                // The docs for EvaluateIsActive say:
                 //
                 // > This method should read directly from its inputs and not rely on any
                 // > subscriptions to fire in order to be up-to-date.
                 //
                 // Which is good advice in general, however in this case we need to break the rule
-                // and use the value from the event subscription instead of calling
+                // and use the value from the event subscription where possible instead of calling
                 // IChildIndexProvider.GetChildIndex. This is because this event can be fired during
-                // the process of realizing an element of a virtualized list; in this case calling
-                // GetChildIndex may not return the correct index as the element isn't yet realized.
+                // the process of realizing an element of a virtualized list; in this case there may
+                // be more than one `nth-child` style on a the list item and when the other is 
+                // re-evaluated calling GetChildIndex may not return the correct index as the element
+                // isn't yet realized.
                 case ChildIndexChangedAction.ChildIndexChanged when e.Child == _control:
-                    _index = e.Index;
+                    _index = e.Index >= 0 ? e.Index : _provider.GetChildIndex(_control);
                     ReevaluateIsActive();
-                    _index = null;
                     break;
                 case ChildIndexChangedAction.ChildIndexesReset:
                 case ChildIndexChangedAction.TotalCountChanged when _reversed:
-                    _index = null;
+                    _index = _provider.GetChildIndex(_control);
                     ReevaluateIsActive();
                     break;
             }

src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs

@@ -15,9 +15,9 @@ namespace Avalonia.Styling
             Property = property;
         }
 
-        public bool HasValue => true;
         public AvaloniaProperty Property { get; }
 
+        public bool HasValue() => true;
         public object? GetValue() => _value ??= _template.Build();
 
         bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error)

src/Avalonia.Base/Styling/Setter.cs

@@ -58,7 +58,6 @@ namespace Avalonia.Styling
             }
         }
 
-        bool IValueEntry.HasValue => true;
         AvaloniaProperty IValueEntry.Property => EnsureProperty();
 
         public override string ToString() => $"Setter: {Property} = {Value}";
@@ -88,6 +87,7 @@ namespace Avalonia.Styling
                 return this;
         }
 
+        bool IValueEntry.HasValue() => true;
         object? IValueEntry.GetValue() => Value;
 
         bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error)

tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs

@@ -1,17 +0,0 @@
-using System;
-using Avalonia.Styling;
-
-namespace Avalonia.Base.UnitTests.Styling
-{
-    public static class TestSelectors
-    {
-        public static Selector SubscribeCheck(this Selector selector)
-        {
-            throw new NotImplementedException();
-            //return new Selector(
-            //    selector,
-            //    control => new SelectorMatch(((TestControlBase)control).SubscribeCheckObservable),
-            //    "");
-        }
-    }
-}

tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

@@ -843,6 +843,64 @@ namespace Avalonia.Controls.UnitTests
             Assert.Equal(1, panel.VisualChildren.Count);
         }
 
+        [Fact]
+        public void Alternating_Backgrounds_Should_Be_Correct_After_Scrolling()
+        {
+            // Issue #12381.
+            static void AssertColors(VirtualizingStackPanel target)
+            {
+                var containers = target.GetRealizedContainers()!
+                    .Cast<ListBoxItem>()
+                    .ToList();
+
+                for (var i = target.FirstRealizedIndex; i <= target.LastRealizedIndex; i++)
+                {
+                    var container = Assert.IsType<ListBoxItem>(target.ContainerFromIndex(i));
+                    var expectedBackground = i % 2 == 0 ? Colors.Green : Colors.Red;
+                    var brush = Assert.IsAssignableFrom<ISolidColorBrush>(container.Background);
+
+                    Assert.Equal(expectedBackground, brush.Color);
+                }
+            }
+
+            using var app = App();
+            var styles = new[]
+            {
+                new Style(x => x.OfType<ListBoxItem>())
+                {
+                    Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.White) },
+                },
+                new Style(x => x.OfType<ListBoxItem>().NthChild(2, 1))
+                {
+                    Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Green) },
+                },
+                new Style(x => x.OfType<ListBoxItem>().NthChild(2, 0))
+                {
+                    Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Red) },
+                },
+            };
+            var (target, scroll, itemsControl) = CreateUnrootedTarget<ListBox>();
+
+            // We need to display an odd number of items to reproduce the issue.
+            var root = CreateRoot(itemsControl, clientSize: new(100, 90), styles: styles);
+            root.LayoutManager.ExecuteInitialLayoutPass();
+
+            var containers = target.GetRealizedContainers()!
+                .Cast<ListBoxItem>()
+                .ToList();
+
+            Assert.Equal(0, target.FirstRealizedIndex);
+            Assert.Equal(8, target.LastRealizedIndex);
+            AssertColors(target);
+
+            scroll.Offset = new Vector(0, 10);
+            target.UpdateLayout();
+
+            Assert.Equal(1, target.FirstRealizedIndex);
+            Assert.Equal(9, target.LastRealizedIndex);
+            AssertColors(target);
+        }
+
         [Fact]
         public void Inserting_Item_Before_Viewport_Preserves_FirstRealizedIndex()
         {
@@ -925,7 +983,7 @@ namespace Avalonia.Controls.UnitTests
                 where T : ItemsControl, new()
         {
             var (target, scroll, itemsControl) = CreateUnrootedTarget<T>(items, itemTemplate);
-            var root = CreateRoot(itemsControl, styles);
+            var root = CreateRoot(itemsControl, styles: styles);
 
             root.LayoutManager.ExecuteInitialLayoutPass();
 
@@ -964,10 +1022,13 @@ namespace Avalonia.Controls.UnitTests
             return (target, scroll, itemsControl);
         }
 
-        private static TestRoot CreateRoot(Control? child, IEnumerable<Style>? styles = null)
+        private static TestRoot CreateRoot(
+            Control? child, 
+            Size? clientSize = null,
+            IEnumerable<Style>? styles = null)
         {
             var root = new TestRoot(true, child);
-            root.ClientSize = new(100, 100);
+            root.ClientSize = clientSize ?? new(100, 100);
 
             if (styles is not null)
                 root.Styles.AddRange(styles);