From 27589e87a8033d7060bc20210ea9f0a541256fce Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 3 Jul 2018 23:15:51 +0200 Subject: [PATCH 1/2] Don't create virtualizer before panel. Otherwise, an `ItemVirtualizerNone` is created which can materialize all items, even if `ItemVirtualizationMode.Simple` is set. Hopefully fixes #1707. --- .../Presenters/ItemVirtualizer.cs | 5 +++ .../Presenters/ItemsPresenter.cs | 36 +++++++++++-------- .../ItemsPresenterTests_Virtualization.cs | 9 +++++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizer.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizer.cs index fee326dacc..e293cff211 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizer.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizer.cs @@ -161,6 +161,11 @@ namespace Avalonia.Controls.Presenters /// An . public static ItemVirtualizer Create(ItemsPresenter owner) { + if (owner.Panel == null) + { + return null; + } + var virtualizingPanel = owner.Panel as IVirtualizingPanel; var scrollable = (ILogicalScrollable)owner; ItemVirtualizer result = null; diff --git a/src/Avalonia.Controls/Presenters/ItemsPresenter.cs b/src/Avalonia.Controls/Presenters/ItemsPresenter.cs index 590bfa25ac..f8d62a1cbf 100644 --- a/src/Avalonia.Controls/Presenters/ItemsPresenter.cs +++ b/src/Avalonia.Controls/Presenters/ItemsPresenter.cs @@ -22,7 +22,6 @@ namespace Avalonia.Controls.Presenters nameof(VirtualizationMode), defaultValue: ItemVirtualizationMode.None); - private ItemVirtualizer _virtualizer; private bool _canHorizontallyScroll; private bool _canVerticallyScroll; @@ -76,21 +75,27 @@ namespace Avalonia.Controls.Presenters /// bool ILogicalScrollable.IsLogicalScrollEnabled { - get { return _virtualizer?.IsLogicalScrollEnabled ?? false; } + get { return Virtualizer?.IsLogicalScrollEnabled ?? false; } } /// - Size IScrollable.Extent => _virtualizer.Extent; + Size IScrollable.Extent => Virtualizer?.Extent ?? Size.Empty; /// Vector IScrollable.Offset { - get { return _virtualizer.Offset; } - set { _virtualizer.Offset = CoerceOffset(value); } + get { return Virtualizer?.Offset ?? new Vector(); } + set + { + if (Virtualizer != null) + { + Virtualizer.Offset = CoerceOffset(value); + } + } } /// - Size IScrollable.Viewport => _virtualizer.Viewport; + Size IScrollable.Viewport => Virtualizer?.Viewport ?? Bounds.Size; /// Action ILogicalScrollable.InvalidateScroll { get; set; } @@ -101,6 +106,8 @@ namespace Avalonia.Controls.Presenters /// Size ILogicalScrollable.PageScrollSize => new Size(0, 1); + internal ItemVirtualizer Virtualizer { get; private set; } + /// bool ILogicalScrollable.BringIntoView(IControl target, Rect targetRect) { @@ -110,29 +117,30 @@ namespace Avalonia.Controls.Presenters /// IControl ILogicalScrollable.GetControlInDirection(NavigationDirection direction, IControl from) { - return _virtualizer?.GetControlInDirection(direction, from); + return Virtualizer?.GetControlInDirection(direction, from); } public override void ScrollIntoView(object item) { - _virtualizer?.ScrollIntoView(item); + Virtualizer?.ScrollIntoView(item); } /// protected override Size MeasureOverride(Size availableSize) { - return _virtualizer?.MeasureOverride(availableSize) ?? Size.Empty; + return Virtualizer?.MeasureOverride(availableSize) ?? Size.Empty; } protected override Size ArrangeOverride(Size finalSize) { - return _virtualizer?.ArrangeOverride(finalSize) ?? Size.Empty; + return Virtualizer?.ArrangeOverride(finalSize) ?? Size.Empty; } /// protected override void PanelCreated(IPanel panel) { - _virtualizer = ItemVirtualizer.Create(this); + Virtualizer?.Dispose(); + Virtualizer = ItemVirtualizer.Create(this); ((ILogicalScrollable)this).InvalidateScroll?.Invoke(); if (!Panel.IsSet(KeyboardNavigation.DirectionalNavigationProperty)) @@ -149,7 +157,7 @@ namespace Avalonia.Controls.Presenters protected override void ItemsChanged(NotifyCollectionChangedEventArgs e) { - _virtualizer?.ItemsChanged(Items, e); + Virtualizer?.ItemsChanged(Items, e); } private Vector CoerceOffset(Vector value) @@ -162,8 +170,8 @@ namespace Avalonia.Controls.Presenters private void VirtualizationModeChanged(AvaloniaPropertyChangedEventArgs e) { - _virtualizer?.Dispose(); - _virtualizer = ItemVirtualizer.Create(this); + Virtualizer?.Dispose(); + Virtualizer = ItemVirtualizer.Create(this); ((ILogicalScrollable)this).InvalidateScroll?.Invoke(); } } diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization.cs index 7eb44c5354..5aa6d50425 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization.cs @@ -194,6 +194,15 @@ namespace Avalonia.Controls.UnitTests.Presenters } } + [Fact] + public void Should_Not_Create_Virtualizer_Before_Panel() + { + var target = CreateTarget(); + + Assert.Null(target.Panel); + Assert.Null(target.Virtualizer); + } + [Fact] public void Changing_VirtualizationMode_None_To_Simple_Should_Update_Control() { From 16da122924d6447187b76482fbcba769abb98b5f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 18 Jul 2018 22:21:25 +0200 Subject: [PATCH 2/2] Use WeakSubscribe in ItemsPresenters. This not only prevents memory leaks, but also accidental double-subscribes as we have an `IDisposable` to represent the subscription state. --- .../Presenters/ItemsPresenterBase.cs | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs b/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs index 5a56e52029..e9dc75a236 100644 --- a/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs +++ b/src/Avalonia.Controls/Presenters/ItemsPresenterBase.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Specialized; +using Avalonia.Collections; using Avalonia.Controls.Generators; using Avalonia.Controls.Templates; using Avalonia.Styling; @@ -40,6 +41,7 @@ namespace Avalonia.Controls.Presenters ItemsControl.MemberSelectorProperty.AddOwner(); private IEnumerable _items; + private IDisposable _itemsSubscription; private bool _createdPanel; private IItemContainerGenerator _generator; @@ -63,24 +65,12 @@ namespace Avalonia.Controls.Presenters set { - if (_createdPanel) - { - INotifyCollectionChanged incc = _items as INotifyCollectionChanged; - - if (incc != null) - { - incc.CollectionChanged -= ItemsCollectionChanged; - } - } + _itemsSubscription?.Dispose(); + _itemsSubscription = null; - if (_createdPanel && value != null) + if (_createdPanel && value is INotifyCollectionChanged incc) { - INotifyCollectionChanged incc = value as INotifyCollectionChanged; - - if (incc != null) - { - incc.CollectionChanged += ItemsCollectionChanged; - } + _itemsSubscription = incc.WeakSubscribe(ItemsCollectionChanged); } SetAndRaise(ItemsProperty, ref _items, value); @@ -233,11 +223,9 @@ namespace Avalonia.Controls.Presenters _createdPanel = true; - INotifyCollectionChanged incc = Items as INotifyCollectionChanged; - - if (incc != null) + if (_itemsSubscription == null && Items is INotifyCollectionChanged incc) { - incc.CollectionChanged += ItemsCollectionChanged; + _itemsSubscription = incc.WeakSubscribe(ItemsCollectionChanged); } PanelCreated(Panel); @@ -263,4 +251,4 @@ namespace Avalonia.Controls.Presenters (e.NewValue as IItemsPresenterHost)?.RegisterItemsPresenter(this); } } -} \ No newline at end of file +}