Merge pull request #2591 from AvaloniaUI/fixes/2144-differing-height-virtualized-items

Fix scrolling to end with differing height virtualized items

samples/VirtualizationDemo/MainWindow.xaml

@@ -39,6 +39,8 @@
             <Button Command="{Binding RecreateCommand}">Recreate</Button>
             <Button Command="{Binding SelectFirstCommand}">Select First</Button>
             <Button Command="{Binding SelectLastCommand}">Select Last</Button>
+            <Button Command="{Binding RandomizeSize}">Randomize Size</Button>
+            <Button Command="{Binding ResetSize}">Reset Size</Button>
         </StackPanel>
 
         <ListBox Name="listBox" 
@@ -55,7 +57,7 @@
             </ListBox.ItemsPanel>
             <ListBox.ItemTemplate>
                 <DataTemplate>
-                    <TextBlock Text="{Binding Header}" TextWrapping="Wrap"/>
+                    <TextBlock Text="{Binding Header}" Height="{Binding Height}" TextWrapping="Wrap"/>
                 </DataTemplate>
             </ListBox.ItemTemplate>
         </ListBox>

samples/VirtualizationDemo/ViewModels/ItemViewModel.cs

@@ -10,6 +10,7 @@ namespace VirtualizationDemo.ViewModels
     {
         private string _prefix;
         private int _index;
+        private double _height = double.NaN;
 
         public ItemViewModel(int index, string prefix = "Item")
         {
@@ -18,5 +19,11 @@ namespace VirtualizationDemo.ViewModels
         }
 
         public string Header => $"{_prefix} {_index}";
+
+        public double Height
+        {
+            get => _height;
+            set => this.RaiseAndSetIfChanged(ref _height, value);
+        }
     }
 }

samples/VirtualizationDemo/ViewModels/MainWindowViewModel.cs

@@ -98,6 +98,24 @@ namespace VirtualizationDemo.ViewModels
         public ReactiveCommand SelectFirstCommand { get; private set; }
         public ReactiveCommand SelectLastCommand { get; private set; }
 
+        public void RandomizeSize()
+        {
+            var random = new Random();
+
+            foreach (var i in Items)
+            {
+                i.Height = random.Next(240) + 10;
+            }
+        }
+
+        public void ResetSize()
+        {
+            foreach (var i in Items)
+            {
+                i.Height = double.NaN;
+            }
+        }
+
         private void ResizeItems(int count)
         {
             if (Items == null)

src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs

@@ -20,6 +20,8 @@ namespace Avalonia.Controls.Presenters
     /// </summary>
     internal class ItemVirtualizerSimple : ItemVirtualizer
     {
+        private int _anchor;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ItemVirtualizerSimple"/> class.
         /// </summary>
@@ -362,7 +364,10 @@ namespace Avalonia.Controls.Presenters
 
             if (panel.OverflowCount > 0)
             {
-                RemoveContainers(panel.OverflowCount);
+                if (_anchor <= FirstIndex)
+                {
+                    RemoveContainers(panel.OverflowCount);
+                }
             }
         }
 
@@ -540,7 +545,9 @@ namespace Avalonia.Controls.Presenters
                 // it means we're running a unit test.
                 if (container != null && layoutManager != null)
                 {
+                    _anchor = index;
                     layoutManager.ExecuteLayoutPass();
+                    _anchor = -1;
 
                     if (newOffset != -1 && newOffset != OffsetValue)
                     {

src/Avalonia.Controls/Primitives/RangeBase.cs

@@ -44,13 +44,13 @@ namespace Avalonia.Controls.Primitives
         /// Defines the <see cref="SmallChange"/> property.
         /// </summary>
         public static readonly StyledProperty<double> SmallChangeProperty =
-            AvaloniaProperty.Register<RangeBase, double>(nameof(SmallChange), 0.1);
+            AvaloniaProperty.Register<RangeBase, double>(nameof(SmallChange), 1);
 
         /// <summary>
         /// Defines the <see cref="LargeChange"/> property.
         /// </summary>
         public static readonly StyledProperty<double> LargeChangeProperty =
-            AvaloniaProperty.Register<RangeBase, double>(nameof(LargeChange), 1);
+            AvaloniaProperty.Register<RangeBase, double>(nameof(LargeChange), 10);
 
         private double _minimum;
         private double _maximum = 100.0;

src/Avalonia.Controls/Primitives/ScrollBar.cs

@@ -216,25 +216,25 @@ namespace Avalonia.Controls.Primitives
 
         private void SmallDecrement()
         {
-            Value = Math.Max(Value - SmallChange * ViewportSize, Minimum);
+            Value = Math.Max(Value - SmallChange, Minimum);
             OnScroll(ScrollEventType.SmallDecrement);
         }
 
         private void SmallIncrement()
         {
-            Value = Math.Min(Value + SmallChange * ViewportSize, Maximum);
+            Value = Math.Min(Value + SmallChange, Maximum);
             OnScroll(ScrollEventType.SmallIncrement);
         }
 
         private void LargeDecrement()
         {
-            Value = Math.Max(Value - LargeChange * ViewportSize, Minimum);
+            Value = Math.Max(Value - LargeChange, Minimum);
             OnScroll(ScrollEventType.LargeDecrement);
         }
 
         private void LargeIncrement()
         {
-            Value = Math.Min(Value + LargeChange * ViewportSize, Maximum);
+            Value = Math.Min(Value + LargeChange, Maximum);
             OnScroll(ScrollEventType.LargeIncrement);
         }
 

src/Avalonia.Controls/Slider.cs

@@ -47,8 +47,6 @@ namespace Avalonia.Controls
             Thumb.DragStartedEvent.AddClassHandler<Slider>(x => x.OnThumbDragStarted, RoutingStrategies.Bubble);
             Thumb.DragDeltaEvent.AddClassHandler<Slider>(x => x.OnThumbDragDelta, RoutingStrategies.Bubble);
             Thumb.DragCompletedEvent.AddClassHandler<Slider>(x => x.OnThumbDragCompleted, RoutingStrategies.Bubble);
-            SmallChangeProperty.OverrideDefaultValue<Slider>(1);
-            LargeChangeProperty.OverrideDefaultValue<Slider>(10);
         }
 
         /// <summary>

tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs

@@ -15,6 +15,7 @@ using Avalonia.Input;
 using Avalonia.Layout;
 using Avalonia.Platform;
 using Avalonia.Rendering;
+using Avalonia.Styling;
 using Avalonia.UnitTests;
 using Xunit;
 
@@ -756,6 +757,80 @@ namespace Avalonia.Controls.UnitTests.Presenters
             Assert.Same(target.Panel.Children[9].DataContext, last);
         }
 
+        [Fact]
+        public void Scrolling_Less_Than_A_Page_Should_Move_Recycled_Items()
+        {
+            var target = CreateTarget();
+            var items = (IList<string>)target.Items;
+
+            target.ApplyTemplate();
+            target.Measure(new Size(100, 100));
+            target.Arrange(new Rect(0, 0, 100, 100));
+
+            var containers = target.Panel.Children.ToList();
+            var scroller = (ScrollContentPresenter)target.Parent;
+
+            scroller.Offset = new Vector(0, 5);
+
+            var scrolledContainers = containers
+                .Skip(5)
+                .Take(5)
+                .Concat(containers.Take(5)).ToList();
+
+            Assert.Equal(new Vector(0, 5), ((ILogicalScrollable)target).Offset);
+            Assert.Equal(scrolledContainers, target.Panel.Children);
+
+            for (var i = 0; i < target.Panel.Children.Count; ++i)
+            {
+                Assert.Equal(items[i + 5], target.Panel.Children[i].DataContext);
+            }
+
+            scroller.Offset = new Vector(0, 0);
+            Assert.Equal(new Vector(0, 0), ((ILogicalScrollable)target).Offset);
+            Assert.Equal(containers, target.Panel.Children);
+
+            var dcs = target.Panel.Children.Select(x => x.DataContext).ToList();
+
+            for (var i = 0; i < target.Panel.Children.Count; ++i)
+            {
+                Assert.Equal(items[i], target.Panel.Children[i].DataContext);
+            }
+        }
+
+        [Fact]
+        public void Scrolling_More_Than_A_Page_Should_Recycle_Items()
+        {
+            var target = CreateTarget(itemCount: 50);
+            var items = (IList<string>)target.Items;
+
+            target.ApplyTemplate();
+            target.Measure(new Size(100, 100));
+            target.Arrange(new Rect(0, 0, 100, 100));
+
+            var containers = target.Panel.Children.ToList();
+            var scroller = (ScrollContentPresenter)target.Parent;
+
+            scroller.Offset = new Vector(0, 20);
+
+            Assert.Equal(new Vector(0, 20), ((ILogicalScrollable)target).Offset);
+            Assert.Equal(containers, target.Panel.Children);
+
+            for (var i = 0; i < target.Panel.Children.Count; ++i)
+            {
+                Assert.Equal(items[i + 20], target.Panel.Children[i].DataContext);
+            }
+
+            scroller.Offset = new Vector(0, 0);
+
+            Assert.Equal(new Vector(0, 0), ((ILogicalScrollable)target).Offset);
+            Assert.Equal(containers, target.Panel.Children);
+
+            for (var i = 0; i < target.Panel.Children.Count; ++i)
+            {
+                Assert.Equal(items[i], target.Panel.Children[i].DataContext);
+            }
+        }
+
         public class Vertical
         {
             [Fact]
@@ -941,86 +1016,8 @@ namespace Avalonia.Controls.UnitTests.Presenters
             }
         }
 
-        public class WithContainers
-        {
-            [Fact]
-            public void Scrolling_Less_Than_A_Page_Should_Move_Recycled_Items()
-            {
-                var target = CreateTarget();
-                var items = (IList<string>)target.Items;
-
-                target.ApplyTemplate();
-                target.Measure(new Size(100, 100));
-                target.Arrange(new Rect(0, 0, 100, 100));
-
-                var containers = target.Panel.Children.ToList();
-                var scroller = (ScrollContentPresenter)target.Parent;
-
-                scroller.Offset = new Vector(0, 5);
-
-                var scrolledContainers = containers
-                    .Skip(5)
-                    .Take(5)
-                    .Concat(containers.Take(5)).ToList();
-
-                Assert.Equal(new Vector(0, 5), ((ILogicalScrollable)target).Offset);
-                Assert.Equal(scrolledContainers, target.Panel.Children);
-
-                for (var i = 0; i < target.Panel.Children.Count; ++i)
-                {
-                    Assert.Equal(items[i + 5], target.Panel.Children[i].DataContext);
-                }
-
-                scroller.Offset = new Vector(0, 0);
-                Assert.Equal(new Vector(0, 0), ((ILogicalScrollable)target).Offset);
-                Assert.Equal(containers, target.Panel.Children);
-
-                var dcs = target.Panel.Children.Select(x => x.DataContext).ToList();
-
-                for (var i = 0; i < target.Panel.Children.Count; ++i)
-                {
-                    Assert.Equal(items[i], target.Panel.Children[i].DataContext);
-                }
-            }
-
-            [Fact]
-            public void Scrolling_More_Than_A_Page_Should_Recycle_Items()
-            {
-                var target = CreateTarget(itemCount: 50);
-                var items = (IList<string>)target.Items;
-
-                target.ApplyTemplate();
-                target.Measure(new Size(100, 100));
-                target.Arrange(new Rect(0, 0, 100, 100));
-
-                var containers = target.Panel.Children.ToList();
-                var scroller = (ScrollContentPresenter)target.Parent;
-
-                scroller.Offset = new Vector(0, 20);
-
-                Assert.Equal(new Vector(0, 20), ((ILogicalScrollable)target).Offset);
-                Assert.Equal(containers, target.Panel.Children);
-
-                for (var i = 0; i < target.Panel.Children.Count; ++i)
-                {
-                    Assert.Equal(items[i + 20], target.Panel.Children[i].DataContext);
-                }
-
-                scroller.Offset = new Vector(0, 0);
-
-                Assert.Equal(new Vector(0, 0), ((ILogicalScrollable)target).Offset);
-                Assert.Equal(containers, target.Panel.Children);
-
-                for (var i = 0; i < target.Panel.Children.Count; ++i)
-                {
-                    Assert.Equal(items[i], target.Panel.Children[i].DataContext);
-                }
-            }
-        }
-
         private static ItemsPresenter CreateTarget(
             Orientation orientation = Orientation.Vertical,
-            bool useContainers = true,
             int itemCount = 20,
             bool useAvaloniaList = false)
         {
@@ -1034,11 +1031,11 @@ namespace Avalonia.Controls.UnitTests.Presenters
             {
                 CanHorizontallyScroll = true,
                 CanVerticallyScroll = true,
-                Content = result = new TestItemsPresenter(useContainers)
+                Content = result = new TestItemsPresenter
                 {
                     Items = items,
                     ItemsPanel = VirtualizingPanelTemplate(orientation),
-                    ItemTemplate = ItemTemplate(),
+                    DataTemplates = { StringDataTemplate() },
                     VirtualizationMode = ItemVirtualizationMode.Simple,
                 }
             };
@@ -1047,7 +1044,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
             return result;
         }
 
-        private static IDataTemplate ItemTemplate()
+        private static IDataTemplate StringDataTemplate()
         {
             return new FuncDataTemplate<string>(x => new Canvas
             {
@@ -1065,7 +1062,7 @@ namespace Avalonia.Controls.UnitTests.Presenters
             });
         }
 
-        private class TestScroller : ScrollContentPresenter, IRenderRoot, ILayoutRoot
+        private class TestScroller : ScrollContentPresenter, IRenderRoot, ILayoutRoot, IStyleRoot
         {
             public IRenderer Renderer { get; }
             public Size ClientSize { get; }
@@ -1085,18 +1082,12 @@ namespace Avalonia.Controls.UnitTests.Presenters
 
         private class TestItemsPresenter : ItemsPresenter
         {
-            private bool _useContainers;
-
-            public TestItemsPresenter(bool useContainers)
-            {
-                _useContainers = useContainers;
-            }
-
             protected override IItemContainerGenerator CreateItemContainerGenerator()
             {
-                return _useContainers ?
-                    new ItemContainerGenerator<TestContainer>(this, TestContainer.ContentProperty, null) :
-                    new ItemContainerGenerator(this);
+                return new ItemContainerGenerator<TestContainer>(
+                    this,
+                    TestContainer.ContentProperty,
+                    null);
             }
         }
 
@@ -1104,8 +1095,12 @@ namespace Avalonia.Controls.UnitTests.Presenters
         {
             public TestContainer()
             {
-                Width = 10;
-                Height = 10;
+                Template = new FuncControlTemplate<TestContainer>(parent => new ContentPresenter
+                {
+                    Name = "PART_ContentPresenter",
+                    [~ContentPresenter.ContentProperty] = parent[~ContentControl.ContentProperty],
+                    [~ContentPresenter.ContentTemplateProperty] = parent[~ContentControl.ContentTemplateProperty],
+                });
             }
         }
     }