amwx / FluentAvalonia

Control library focused on fluent design and bringing more WinUI controls into Avalonia
MIT License
1.05k stars 97 forks source link

NavigationView / ItemsRepeater Crash with FluentAvalonia 2.1.0-preview4 #564

Closed robloo closed 3 months ago

robloo commented 6 months ago

Describe the bug

The preview NavigationView crashes in the ItemsRepeater ItemsRepeater.OnElementPrepared / ViewportManager.OnElementPrepared code. This was previously working in preview2 and earlier versions of FluentAvalonia 2.0+.

Screenshots

image

Desktop/Platform (please complete the following information):

Additional context

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=FluentAvalonia
  StackTrace:

    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ViewportManager.OnElementPrepared(Avalonia.Controls.Control element, FluentAvalonia.UI.Controls.VirtualizationInfo vInfo) Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ItemsRepeater.OnElementPrepared(Avalonia.Controls.Control element, int index, FluentAvalonia.UI.Controls.VirtualizationInfo vInfo)    Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ViewManager.GetElementFromElementFactory(int index)   Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ViewManager.GetElement(int index, bool forceCreate, bool suppressAutoRecycle) Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ItemsRepeater.GetElementImpl(int index, bool forceCreate, bool suppressAutoRecycle)   Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.RepeaterLayoutContext.GetOrCreateElementAtCore(int index, FluentAvalonia.UI.Controls.ElementRealizationOptions options)   Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.VirtualizingLayoutContext.GetOrCreateElementAt(int index, FluentAvalonia.UI.Controls.ElementRealizationOptions options)   Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.FlowLayoutAlgorithm.GetAnchorIndex(Avalonia.Size availableSize, bool isWrapping, double minItemSpacing, bool disableVirtualization, string layoutId)  Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.FlowLayoutAlgorithm.Measure(Avalonia.Size availableSize, FluentAvalonia.UI.Controls.VirtualizingLayoutContext context, bool isWrapping, double minItemSpacing, double lineSpacing, int maxItemsPerLine, FluentAvalonia.UI.Controls.ScrollOrientation orientation, bool disableVirtualization, string layoutId)    Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.StackLayout.MeasureOverride(FluentAvalonia.UI.Controls.VirtualizingLayoutContext context, Avalonia.Size availableSize)    Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.Layout.Measure(FluentAvalonia.UI.Controls.LayoutContext context, Avalonia.Size availableSize) Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.ItemsRepeater.MeasureOverride(Avalonia.Size availableSize)    Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Grid.MeasureCell(int cell, bool forceInfinityV) Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Grid.MeasureCellsGroup(int cellsHead, Avalonia.Size referenceSize, bool ignoreDesiredSizeU, bool forceInfinityV, out bool hasDesiredSizeUChanged)   Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Grid.MeasureCellsGroup(int cellsHead, Avalonia.Size referenceSize, bool ignoreDesiredSizeU, bool forceInfinityV)    Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Grid.MeasureOverride(Avalonia.Size constraint)  Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.LayoutHelper.MeasureChild(Avalonia.Layout.Layoutable control, Avalonia.Size availableSize, Avalonia.Thickness padding)    Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.NavigationView.MeasureTopNavigationViewDesiredWidth(Avalonia.Size availableSize)  Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.NavigationView.HandleTopNavigationMeasureOverrideNormal(Avalonia.Size availableSize)  Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.NavigationView.HandleTopNavigationMeasureOverride(Avalonia.Size availableSize)    Unknown
    FluentAvalonia.dll!FluentAvalonia.UI.Controls.NavigationView.MeasureOverride(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Grid.MeasureOverride(Avalonia.Size constraint)  Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.LayoutHelper.MeasureChild(Avalonia.Layout.Layoutable control, Avalonia.Size availableSize, Avalonia.Thickness padding, Avalonia.Thickness borderThickness)    Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Presenters.ContentPresenter.MeasureOverride(Avalonia.Size availableSize)    Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.LayoutHelper.MeasureChild(Avalonia.Layout.Layoutable control, Avalonia.Size availableSize, Avalonia.Thickness padding)    Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Decorator.MeasureOverride(Avalonia.Size availableSize)  Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Primitives.VisualLayerManager.MeasureOverride(Avalonia.Size availableSize)  Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureOverride(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureCore(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.MeasureOverride(Avalonia.Size availableSize)   Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Window.MeasureOverride(Avalonia.Size availableSize) Unknown
    Avalonia.Controls.dll!Avalonia.Controls.WindowBase.MeasureCore(Avalonia.Size availableSize) Unknown
    Avalonia.Base.dll!Avalonia.Layout.Layoutable.Measure(Avalonia.Size availableSize)   Unknown
    Avalonia.Base.dll!Avalonia.Layout.LayoutManager.Measure(Avalonia.Layout.Layoutable control) Unknown
    Avalonia.Base.dll!Avalonia.Layout.LayoutManager.ExecuteInitialLayoutPass()  Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Window.ShowCore(Avalonia.Controls.Window owner) Unknown
    Avalonia.Controls.dll!Avalonia.Controls.Window.Show()   Unknown
    Avalonia.Controls.dll!Avalonia.Controls.ApplicationLifetimes.ClassicDesktopStyleApplicationLifetime.ShowMainWindow()    Unknown
    Avalonia.Controls.dll!Avalonia.Controls.ApplicationLifetimes.ClassicDesktopStyleApplicationLifetime.Start(string[] args)    Unknown
    Avalonia.Controls.dll!Avalonia.ClassicDesktopStyleApplicationLifetimeExtensions.StartWithClassicDesktopLifetime(Avalonia.AppBuilder builder, string[] args, System.Action<Avalonia.Controls.ApplicationLifetimes.IClassicDesktopStyleApplicationLifetime> lifetimeBuilder)  Unknown
amwx commented 6 months ago

Do you have a repro, or if not an actual project at least a description of how the NavView is set up so I can repro this?

robloo commented 6 months ago

I was afraid you were going to say that. Reducing down to a repro in a full app is always tricky. Might be faster to debug with a local build of FluentAvalonia itself. Will spend more time on this later in the week.

amwx commented 6 months ago

If you can't get a repro, does the error message have the line number - I didn't see it above.

Also, populating the NavView, are you binding or using raw NavigationViewItems? If binding, what does your data template look like and what is the source collection type? Is SelectedItem bound? Is it hierarchical? PaneDisplayMode? Basically anything you can tell me about how its set up and I can try to repro it myself. Does it crash upon load or do you have to do anything?

All I know is just from a quick look at the code and stepping back from how that final method is called, nothing should be null at that point. The only thing I can think of is maybe GetElement is being called too early from a binding (like SelectedItem being set) and layout hasn't run so the ScrollViewer isn't available yet, but that should repro in more cases if that's the case.

robloo commented 6 months ago

Edit: Somehow it submitted while I was typing... so I finished it.

@amwx I'm running with FluentAvalonia source directly now so can step into the code.

Also, populating the NavView, are you binding or using raw NavigationViewItems?

NavigationViewItem are added directly in XAML -- there is no binding here.

Is SelectedItem bound?

No

Is it hierarchical?

No

PaneDisplayMode?

Top

Does it crash upon load or do you have to do anything?

It crashes on initial load. It's crashing as soon as Show() is called on the Window and the first layout/arrange pass is running. This is before the Window is even loaded and before and more complex code is run. As far as I can tell it's still constructing the visual tree and not even running real app code yet.

ScrollViewer isn't available yet

That is indeed what is happening. I notice most usages of _scroller have a null check as well. The area of the crash there is no check.

image

robloo commented 6 months ago

@amwx The below does work around the issue. App starts up and runs normally after this change adding the null checks. Somehow the code path is getting run before EnsureScroller() or something. Will look further.

    public void OnElementPrepared(Control element, VirtualizationInfo vInfo)
    {
        // If we have an anchor element, we do not want the
        // scroll anchor provider to start anchoring some other element.
        vInfo.CanBeScrollAnchor = true;
-       _scroller.RegisterAnchorCandidate(element);
+       _scroller?.RegisterAnchorCandidate(element);
    }

    public void OnElementCleared(Control element)
    {
        // Unlike OnElementPrepared, we can't make the change and add the virt info
        // as the caller of this (ItemsRepeater.ClearElementImpl) doesn't have a ref
        // and that method has multiple refs which don't have a the virt info either
        ItemsRepeater.GetVirtualizationInfo(element).CanBeScrollAnchor = false;
-       _scroller.UnregisterAnchorCandidate(element);
+       _scroller?.UnregisterAnchorCandidate(element);
    }
amwx commented 6 months ago

Ok I've added in the null checks and stopped this crash. Everything is actually running ok, but in Top pane mode there isn't a ScrollViewer in the tree and so it was never set. This code is slightly different from WinUI because Avalonia handles scroll anchor candidates slightly differently. So adding the null check won't hurt anything since if there's no ScrollViewer, there's no need to register anchor candidates.

2.1 branch has been updated with this fix along with a couple other crashes I found. If you find any other issues, just tack them on here.