diff --git a/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayout.cs b/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayout.cs index d87a8364..e8487332 100644 --- a/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayout.cs +++ b/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayout.cs @@ -179,10 +179,6 @@ protected override Size MeasureOverride( false /* disableVirtualization */, LayoutId); - // If after Measure the first item is in the realization rect, then we revoke grid state's ownership, - // and only use the layout when to clear it when it's done. - gridState.EnsureFirstElementOwnership(context); - return desiredSize; } @@ -207,9 +203,6 @@ protected override void OnItemsChangedCore( GetFlowAlgorithm(context).OnItemsSourceChanged(source, args, context); // Always invalidate layout to keep the view accurate. InvalidateLayout(); - - var gridState = GetAsGridState(context.LayoutState); - gridState.ClearElementOnDataSourceChange(context, args); } #region IFlowLayoutAlgorithmDelegates diff --git a/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayoutState.cs b/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayoutState.cs index 9778e4d4..24f78cb6 100644 --- a/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayoutState.cs +++ b/ModernWpf.Controls/Repeater/Layouts/UniformGridLayout/UniformGridLayoutState.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. See LICENSE in the project root for license information. using System; -using System.Collections.Specialized; using System.Windows; using System.Windows.Controls; @@ -21,11 +20,6 @@ internal void InitializeForContext( internal void UninitializeForContext(VirtualizingLayoutContext context) { FlowAlgorithm.UninitializeForContext(context); - - if (m_cachedFirstElement != null) - { - context.RecycleElement(m_cachedFirstElement); - } } internal FlowLayoutAlgorithm FlowAlgorithm { get; } = new FlowLayoutAlgorithm(); @@ -50,36 +44,27 @@ internal void EnsureElementSize( if (context.ItemCount > 0) { - // If the first element is realized we don't need to cache it or to get it from the context + // If the first element is realized we don't need to get it from the context var realizedElement = FlowAlgorithm.GetElementIfRealized(0); if (realizedElement != null) { realizedElement.Measure(availableSize); - SetSize(realizedElement, layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine); - m_cachedFirstElement = null; + SetSize(realizedElement.DesiredSize, layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine); } else { - if (m_cachedFirstElement == null) - { - // we only cache if we aren't realizing it - m_cachedFirstElement = context.GetOrCreateElementAt(0, ElementRealizationOptions.ForceCreate | ElementRealizationOptions.SuppressAutoRecycle); // expensive - } - - m_cachedFirstElement.Measure(availableSize); - SetSize(m_cachedFirstElement, layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine); - - // See if we can move ownership to the flow algorithm. If we can, we do not need a local cache. - bool added = FlowAlgorithm.TryAddElement0(m_cachedFirstElement); - if (added) + // Not realized by flowlayout, so do this now! + if (context.GetOrCreateElementAt(0, ElementRealizationOptions.ForceCreate) is { } firstElement) { - m_cachedFirstElement = null; + firstElement.Measure(availableSize); + SetSize(firstElement.DesiredSize, layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine); + context.RecycleElement(firstElement); } } } } - private void SetSize(UIElement UIElement, + private void SetSize(Size desiredItemSize, double layoutItemWidth, double LayoutItemHeight, Size availableSize, @@ -94,8 +79,8 @@ private void SetSize(UIElement UIElement, maxItemsPerLine = 1; } - EffectiveItemWidth = double.IsNaN(layoutItemWidth) ? UIElement.DesiredSize.Width : layoutItemWidth; - EffectiveItemHeight = double.IsNaN(LayoutItemHeight) ? UIElement.DesiredSize.Height : LayoutItemHeight; + EffectiveItemWidth = double.IsNaN(layoutItemWidth) ? desiredItemSize.Width : layoutItemWidth; + EffectiveItemHeight = double.IsNaN(LayoutItemHeight) ? desiredItemSize.Height : LayoutItemHeight; var availableSizeMinor = orientation == Orientation.Horizontal ? availableSize.Width : availableSize.Height; var minorItemSpacing = orientation == Orientation.Vertical ? minRowSpacing : minColumnSpacing; @@ -146,63 +131,5 @@ private void SetSize(UIElement UIElement, } } } - - // If it's realized then we shouldn't be caching it - internal void EnsureFirstElementOwnership(VirtualizingLayoutContext context) - { - if (m_cachedFirstElement != null && FlowAlgorithm.GetElementIfRealized(0) != null) - { - // We created the element, but then flowlayout algorithm took ownership, so we can clear it and - // let flowlayout algorithm do its thing. - context.RecycleElement(m_cachedFirstElement); - m_cachedFirstElement = null; - } - } - - internal void ClearElementOnDataSourceChange(VirtualizingLayoutContext context, NotifyCollectionChangedEventArgs args) - { - if (m_cachedFirstElement != null) - { - // The first element of UniformGridLayout is special since we use its size to - // determine the size of all the other elements. So if the first item has changed - // we will need to clear it and re-evalauate all the items with the new item size. - bool shouldClear = false; - switch (args.Action) - { - case NotifyCollectionChangedAction.Add: - shouldClear = args.NewStartingIndex == 0; - break; - - case NotifyCollectionChangedAction.Replace: - shouldClear = args.NewStartingIndex == 0 || args.OldStartingIndex == 0; - break; - - case NotifyCollectionChangedAction.Remove: - shouldClear = args.OldStartingIndex == 0; - break; - - case NotifyCollectionChangedAction.Reset: - shouldClear = true; - break; - - case NotifyCollectionChangedAction.Move: - shouldClear = args.NewStartingIndex == 0 || args.OldStartingIndex == 0; - break; - } - - if (shouldClear) - { - context.RecycleElement(m_cachedFirstElement); - m_cachedFirstElement = null; - } - } - } - - // We need to measure the element at index 0 to know what size to measure all other items. - // If FlowlayoutAlgorithm has already realized element 0 then we can use that. - // If it does not, then we need to do context.GetElement(0) at which point we have requested an element and are on point to clear it. - // If we are responsible for clearing element 0 we keep m_cachedFirstElement valid. - // If we are not (because FlowLayoutAlgorithm is holding it for us) then we just null out this field and use the one from FlowLayoutAlgorithm. - private UIElement m_cachedFirstElement = null; } } diff --git a/test/ModernWpfTestApp/ApiTests/RepeaterTests/LayoutTests.cs b/test/ModernWpfTestApp/ApiTests/RepeaterTests/LayoutTests.cs index 78cc01b2..a5fc8356 100644 --- a/test/ModernWpfTestApp/ApiTests/RepeaterTests/LayoutTests.cs +++ b/test/ModernWpfTestApp/ApiTests/RepeaterTests/LayoutTests.cs @@ -38,6 +38,8 @@ using ElementRealizationOptions = ModernWpf.Controls.ElementRealizationOptions; using LayoutContext = ModernWpf.Controls.LayoutContext; using LayoutPanel = ModernWpf.Controls.LayoutPanel; +using System.Windows.Media; +using ModernWpf.Controls; namespace ModernWpf.Tests.MUXControls.ApiTests.RepeaterTests { @@ -254,6 +256,53 @@ public void ValidateStackLayoutDisabledVirtualizationWithItemsRepeater() }); } + [TestMethod] + public void VerifyUniformGridLayoutDoesntCrashWhenTryingToScrollToEnd() + { + + ItemsRepeater repeater = null; + ScrollViewer scrollViewer = null; + RunOnUIThread.Execute(() => + { + repeater = new ItemsRepeater + { + ItemsSource = Enumerable.Range(0, 1000).Select(i => new Border + { + Background = new SolidColorBrush(Colors.Blue), + Child = new TextBlock { Text = "#" + i } + }).ToArray(), + Layout = new UniformGridLayout + { + MinItemWidth = 100, + MinItemHeight = 40, + MinRowSpacing = 10, + MinColumnSpacing = 10 + } + }; + scrollViewer = new ScrollViewer { Content = repeater }; + Content = scrollViewer; + }); + + IdleSynchronizer.Wait(); + + RunOnUIThread.Execute(() => + { + scrollViewer.ChangeView(0, repeater.ActualHeight, null); + }); + + IdleSynchronizer.Wait(); + + RunOnUIThread.Execute(() => + { + scrollViewer.ChangeView(0, 0, null); + }); + + IdleSynchronizer.Wait(); + + // The test guards against an app crash, so this is enough to verify + Verify.IsTrue(true); + } + private ItemsRepeaterScrollHost CreateAndInitializeRepeater( object itemsSource, VirtualizingLayout layout,