MPowerKit / VirtualizeListView

.NET MAUI ListView renderers. Improves performance for MAUI ListView, adds additional behaviors
MIT License
69 stars 10 forks source link

ArgumentOutOfRange_IndexMustBeLess fix #7

Closed MichaelFrenkel closed 1 month ago

MichaelFrenkel commented 1 month ago

fix for https://github.com/MPowerKit/VirtualizeListView/issues/6

MichaelFrenkel commented 1 month ago

we've checked on our end, looks good. @daltzctr could you test it on your end?

Alex-Dobrynin commented 1 month ago

I would very appreciate if you provide some repro project, because i think that this issue is much deeper than just add this if statement. Cannot reproduce by myself

MichaelFrenkel commented 1 month ago

repro/argument_out_of_range

not easy to reproduce, I assume if UI is more complex then it is easier to reproduce

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1[[MPowerKit.VirtualizeListView.VirtualizeListViewItem, MPowerKit.VirtualizeListView, Version=1.1.5.0, Culture=neutral, PublicKeyToken=null]].get_Item(Int32 index)
   at MPowerKit.VirtualizeListView.LinearItemsLayoutManager.ArrangeItem(IReadOnlyList`1 items, VirtualizeListViewItem item, Size availableSpace) in /Users/michaelfrenkel/VirtualizeListView/MPowerKit.VirtualizeListView/LinearItemsLayoutManager.cs:line 51
   at MPowerKit.VirtualizeListView.ItemsLayoutManager.OnItemSizeChanged(VirtualizeListViewItem item) in /Users/michaelfrenkel/VirtualizeListView/MPowerKit.VirtualizeListView/ItemsLayoutManager.cs:line 660
   at MPowerKit.VirtualizeListView.VirtualizeListViewItem.OnCellSizeChanged() in /Users/michaelfrenkel/VirtualizeListView/MPowerKit.VirtualizeListView/VirtualizeListViewItem.cs:line 53
   at MPowerKit.VirtualizeListView.CellHolder.CellHolder_SizeChanged(Object sender, EventArgs e) in /Users/michaelfrenkel/VirtualizeListView/MPowerKit.VirtualizeListView/CellHolder.cs:line 17
   at Microsoft.Maui.Controls.VisualElement.UpdateBoundsComponents(Rect bounds)
   at Microsoft.Maui.Controls.VisualElement.set_Frame(Rect value)
   at Microsoft.Maui.Controls.VisualElement.ArrangeOverride(Rect bounds)
   at Microsoft.Maui.Controls.VisualElement.Microsoft.Maui.IView.Arrange(Rect bounds)
   at Microsoft.Maui.Layouts.AbsoluteLayoutManager.ArrangeChildren(Rect bounds)
   at Microsoft.Maui.Controls.Layout.CrossPlatformArrange(Rect bounds)
   at Microsoft.Maui.Platform.MauiView.CrossPlatformArrange(Rect bounds)
   at Microsoft.Maui.Platform.MauiView.LayoutSubviews()

https://github.com/user-attachments/assets/ac4fcf38-d795-45ca-88aa-7eb212343056

Alex-Dobrynin commented 1 month ago

Is this only on ios, or android too?

MichaelFrenkel commented 1 month ago

only iOS on our side

daltzctr commented 1 month ago

This occurred on Windows for me with the exact same steps as above. I mutate the ItemsSource when filtering a complex list of items.

Alex-Dobrynin commented 1 month ago

@daltzctr I cannot reproduce repro on windows, which is provided by @MichaelFrenkel. can you provide your repro for windows?

Buy the way guys. my suggestion to you is to put some delay between your input and search. I'm using this one

public class SearchBarSearchBehavior : BehaviorBase<SearchBar>
    {
        private Task _search;
        private CancellationTokenSource _cts;

        protected override void OnAttachedTo(SearchBar bindable)
        {
            base.OnAttachedTo(bindable);
            bindable.TextChanged += OnSearchQueryChanged;
        }

        protected override void OnDetachingFrom(SearchBar bindable)
        {
            base.OnDetachingFrom(bindable);
            bindable.TextChanged -= OnSearchQueryChanged;
        }

        void OnSearchQueryChanged(object sender, TextChangedEventArgs e)
        {
            if (!IsEnabled) return;

            if (_search is null)
            {
                StartTimer();
            }
            else if (!_search.IsCompleted)
            {
                _cts.Cancel();
                StartTimer();
            }
        }

        private void StartTimer()
        {
            _cts = new CancellationTokenSource();
            _search = Task.Run(async () =>
            {
                try
                {
                    await Task.Delay(TimeDelay, _cts.Token);
                    _search = null;

                    var trimmed = AssociatedObject.Text?.Trim();
                    if (string.IsNullOrEmpty(trimmed)
                        && !string.IsNullOrEmpty(AssociatedObject.Text)
                        && AssociatedObject.Text.Length > 0) return;

                    RaiseCommand(trimmed);
                }
                catch { }
            });
        }

        [MainThreadAspect]
        private void RaiseCommand(string text)
        {
            if (SearchCommand?.CanExecute(text) is not true) return;

            SearchCommand.Execute(text);
        }

        #region IsEnabled
        public bool IsEnabled
        {
            get => (bool)GetValue(IsEnabledProperty);
            set => SetValue(IsEnabledProperty, value);
        }

        public static readonly BindableProperty IsEnabledProperty =
            BindableProperty.Create(
                nameof(IsEnabled),
                typeof(bool),
                typeof(SearchBarSearchBehavior),
                true);
        #endregion

        #region SearchCommand
        public ICommand SearchCommand
        {
            get => (ICommand)GetValue(SearchCommandProperty);
            set => SetValue(SearchCommandProperty, value);
        }

        public static readonly BindableProperty SearchCommandProperty =
            BindableProperty.Create(
                nameof(SearchCommand),
                typeof(ICommand),
                typeof(SearchBarSearchBehavior));
        #endregion

        #region TimeDelay
        public int TimeDelay
        {
            get => (int)GetValue(TimeDelayProperty);
            set => SetValue(SearchCommandProperty, value);
        }

        public static readonly BindableProperty TimeDelayProperty =
            BindableProperty.Create(
                nameof(TimeDelay),
                typeof(int),
                typeof(SearchBarSearchBehavior),
                1000);
        #endregion
    }
Alex-Dobrynin commented 1 month ago

@MichaelFrenkel does this happen only in release mode, or in debug also?

Alex-Dobrynin commented 1 month ago

@MichaelFrenkel @daltzctr cannot reproduce this repro on ios, android, windows debug/release modes simulators/devices. what can i suggest, we can merge this PR, but i'm sure the main problem is not here. since i cannot reproduce and debug, we will leave this fix as is, but I want to ask you to add comment above your if statement that it this is temporary fix and fixes this issue

MichaelFrenkel commented 1 month ago

@MichaelFrenkel does this happen only in release mode, or in debug also?

we have the crash in release and also I've reproduced in debug on the repro.

Alex-Dobrynin commented 1 month ago

@MichaelFrenkel do you have sync or async search method?

MichaelFrenkel commented 1 month ago

@MichaelFrenkel do you have sync or async search method?

in repro sync in our proj also sync but we update items like this: MainThread.InvokeOnMainThreadAsync(() => Items = items);

MichaelFrenkel commented 1 month ago

@Alex-Dobrynin if you have doubts about the fix, not a problem to fix it on our end as we can inherit LinearItemsLayoutManager and override ArrangeItem let me know if you want to merge the fix

Alex-Dobrynin commented 1 month ago

@MichaelFrenkel just add that comment there and i will merge, but before i merge try to reproduce with this fix on your end. if not fail i will merge

MichaelFrenkel commented 1 month ago

@MichaelFrenkel just add that comment there and i will merge, but before i merge try to reproduce with this fix on your end. if not fail i will merge

added the comment. Our automation team has checked the fix, looks good

Alex-Dobrynin commented 1 month ago

Yes. I've reproduced it finally. And i've found what is causing this crash. I suppose we need to make another fix

Alex-Dobrynin commented 1 month ago

i will close it, since i made fix by myself. thank you guys for your help