From 9fa0f0c3666ecd562c86292307e53f784a7c3d55 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Nov 2023 16:31:51 +0100 Subject: [PATCH 1/4] 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. --- .../PropertyStore/BindingEntryBase.cs | 15 ++++++--------- src/Avalonia.Base/PropertyStore/IValueEntry.cs | 7 +++++-- .../PropertyStore/ImmediateValueEntry.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueFrame.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueStore.cs | 6 +++--- .../Styling/PropertySetterTemplateInstance.cs | 2 +- src/Avalonia.Base/Styling/Setter.cs | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index a841803ee10..ef43720537a 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -49,15 +49,6 @@ private BindingEntryBase( _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 @@ public void Dispose() BindingCompleted(); } + public bool HasValue() + { + Start(produceValue: false); + return _hasValue; + } + public TValue GetValue() { Start(produceValue: false); diff --git a/src/Avalonia.Base/PropertyStore/IValueEntry.cs b/src/Avalonia.Base/PropertyStore/IValueEntry.cs index 5898bef491b..7ac7e832762 100644 --- a/src/Avalonia.Base/PropertyStore/IValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/IValueEntry.cs @@ -8,13 +8,16 @@ namespace Avalonia.PropertyStore /// internal interface IValueEntry { - bool HasValue { get; } - /// /// Gets the property that this value applies to. /// AvaloniaProperty Property { get; } + /// + /// Checks whether the entry has a value, starting the entry if necessary. + /// + bool HasValue(); + /// /// Gets the value associated with the entry. /// diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs index 16b96eff5d7..2d136fb1e5c 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueEntry.cs @@ -19,13 +19,13 @@ public ImmediateValueEntry( } public StyledProperty 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.GetValue() => _value; diff --git a/src/Avalonia.Base/PropertyStore/ValueFrame.cs b/src/Avalonia.Base/PropertyStore/ValueFrame.cs index 7a9d1bb13a6..8ef68c651c1 100644 --- a/src/Avalonia.Base/PropertyStore/ValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ValueFrame.cs @@ -27,7 +27,7 @@ protected ValueFrame(BindingPriority priority, FrameType type) } 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; } diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 2047f4d2d09..85adee23864 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -843,7 +843,7 @@ private void ReevaluateEffectiveValue( // 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 @@ private void ReevaluateEffectiveValues(IValueEntry? changedValueEntry = null) { var frame = _frames[i]; - if (!frame.IsActive) + if (!frame.IsActive()) continue; var priority = frame.Priority; @@ -927,7 +927,7 @@ private void ReevaluateEffectiveValues(IValueEntry? changedValueEntry = null) if (!HasHigherPriority(entry, priority, effectiveValue, changedValueEntry)) continue; - if (!entry.HasValue) + if (!entry.HasValue()) continue; if (effectiveValue is not null) diff --git a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs index 7604c26244f..d9a2f55da91 100644 --- a/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs +++ b/src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs @@ -15,9 +15,9 @@ public PropertySetterTemplateInstance(AvaloniaProperty property, ITemplate templ 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) diff --git a/src/Avalonia.Base/Styling/Setter.cs b/src/Avalonia.Base/Styling/Setter.cs index 1ac26c79ecc..f31394d6f7b 100644 --- a/src/Avalonia.Base/Styling/Setter.cs +++ b/src/Avalonia.Base/Styling/Setter.cs @@ -58,7 +58,6 @@ public object? Value } } - bool IValueEntry.HasValue => true; AvaloniaProperty IValueEntry.Property => EnsureProperty(); public override string ToString() => $"Setter: {Property} = {Value}"; @@ -88,6 +87,7 @@ internal override ISetterInstance Instance(IStyleInstance instance, StyledElemen return this; } + bool IValueEntry.HasValue() => true; object? IValueEntry.GetValue() => Value; bool IValueEntry.GetDataValidationState(out BindingValueType state, out Exception? error) From 540cd6991c61ac64175dcac0de36ddeaf062368f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Nov 2023 22:58:23 +0100 Subject: [PATCH 2/4] Deleted unused file. --- .../Styling/TestSelectors.cs | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs diff --git a/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs b/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs deleted file mode 100644 index bd60275d508..00000000000 --- a/tests/Avalonia.Base.UnitTests/Styling/TestSelectors.cs +++ /dev/null @@ -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), - // ""); - } - } -} From 8182e58e97db0858fae0928798edfa358da61b4a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Nov 2023 23:52:15 +0100 Subject: [PATCH 3/4] Add failing test for #12381. --- .../VirtualizingStackPanelTests.cs | 67 ++++++++++++++++++- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs index 4cbf8d2142b..9218559b76c 100644 --- a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs @@ -843,6 +843,64 @@ public void Does_Not_Realize_Items_If_Self_Outside_Viewport() 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() + .ToList(); + + for (var i = target.FirstRealizedIndex; i <= target.LastRealizedIndex; i++) + { + var container = Assert.IsType(target.ContainerFromIndex(i)); + var expectedBackground = i % 2 == 0 ? Colors.Green : Colors.Red; + var brush = Assert.IsAssignableFrom(container.Background); + + Assert.Equal(expectedBackground, brush.Color); + } + } + + using var app = App(); + var styles = new[] + { + new Style(x => x.OfType()) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.White) }, + }, + new Style(x => x.OfType().NthChild(2, 1)) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Green) }, + }, + new Style(x => x.OfType().NthChild(2, 0)) + { + Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Red) }, + }, + }; + var (target, scroll, itemsControl) = CreateUnrootedTarget(); + + // 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() + .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); + } + private static IReadOnlyList GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl) { return target.GetRealizedElements() @@ -903,7 +961,7 @@ private static (VirtualizingStackPanel, ScrollViewer, T) CreateTarget( where T : ItemsControl, new() { var (target, scroll, itemsControl) = CreateUnrootedTarget(items, itemTemplate); - var root = CreateRoot(itemsControl, styles); + var root = CreateRoot(itemsControl, styles: styles); root.LayoutManager.ExecuteInitialLayoutPass(); @@ -942,10 +1000,13 @@ private static (VirtualizingStackPanel, ScrollViewer, T) CreateUnrootedTarget return (target, scroll, itemsControl); } - private static TestRoot CreateRoot(Control? child, IEnumerable