microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.34k stars 677 forks source link

[LinedFlowLayout (Maybe)] Crashed when item subscribed EffectiveViewportChanged #9142

Open Poker-sang opened 10 months ago

Poker-sang commented 10 months ago

Describe the bug

With LinedFlowLayout, if we subscribe to the EffectiveViewportChanged event for the item and update the ItemsSource with a ObservableCollection<T>, the application crashes when the new item is at the end of the row or out of the viewport.

Steps to reproduce the bug

See https://github.com/Poker-sang/ItemsViewTest

Expected behavior

The app does not crash

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.4.2: 1.4.231008000

Windows version

Windows Insider Build (xxxxx)

Additional context

No response

ranjeshj commented 10 months ago

@Poker-sang can you explain what you are trying a bit more? What are you trying to achieve by changing the items in EffectiveViewportChanged? That event triggers quite often and changing the items would end up in doing re-layout which could trigger EffectiveViewportChanged on the items again and potentially causing a layout cycle.

Poker-sang commented 10 months ago

Yes, I want to dispose of the image source after the element leaves the viewport, so I call EffectiveViewportChanged. In addition to this, I found that the LinedFlowLayout often doesn't stretch the elements correctly, and I need to manually adjust the width of the ItemsView to retrigger the layout. Sometimes the LinedFlowLayout also leaves an empty element room in the center. I can provide screenshots later.

Poker-sang commented 10 months ago
<LinedFlowLayout
    ItemsStretch="Fill"
    LineHeight="200"
    LineSpacing="5"
    MinItemSpacing="5" />

image

image

ranjeshj commented 10 months ago

@Poker-sang LinedFlowLayout is a virtualizing layout, so items get recycled and I believe if you are using the Image element, those will get released automatically, so you should not need to use effective viewport to do that. The gaps in layout seem wonky. can you share a simple repro ?

Poker-sang commented 10 months ago

so you should not need to use effective viewport to do that.

Because the image is downloaded from the network to the ImageSource, it is separate from the control Image. While the Image will be recycled, the ImageSource will not be released, so I'd like to call the event EffectiveViewportChanged. Is there a better way to free up memory of 'ImageSource'?

can you share a simple repro ?

Sure. This repo is fine: https://github.com/Poker-sang/ItemsViewTest. You can comment the event handler EffectiveViewportChanged. Run the project click the Add button, and then you can see the gap.

ranjeshj commented 10 months ago

Another option would be to hook into the ElementPrepared and ElementClearing event of the underlying ItemsRepeater. You can get to the repeater through ItemsView.ScrollView.Child as ItemsRepeater I think. When elements are getting recycled to be reused, you can perform any cleanup you wish for that item.

Poker-sang commented 10 months ago

You can get to the repeater through ItemsView.ScrollView.Child as ItemsRepeater I think.

Thanks! This looks great. I'll try it.

Poker-sang commented 10 months ago

@ranjeshj I'm grateful. ElementPrepared and ElementClearing works even better and simpler than EffectiveViewportChanged. Looks like there is no need to use this event any more.

But the "gaps" problem still exists, should I rename this issue?