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

ItemsRepeater breaks when scrolling through variable height items #1829

Open grokys opened 4 years ago

grokys commented 4 years ago

Describe the bug

With a long list of elements of different heights in an ItemsRepeater, scrolling breaks, often showing a blank page.

Steps to reproduce the bug

Edit BasicDemo to contain the following:

        <controls:ItemsRepeaterScrollHost x:Name="tracker" Grid.Row="1">
            <ScrollViewer x:Name="scrollViewer">
                <controls:ItemsRepeater x:Name="repeater" VerticalCacheLength="0" Background="LightBlue">
                    <controls:ItemsRepeater.ItemTemplate>
                        <DataTemplate>
                            <TextBlock Text="{Binding Text}" Height="{Binding Height}"/>
                        </DataTemplate>
                    </controls:ItemsRepeater.ItemTemplate>
                </controls:ItemsRepeater>
            </ScrollViewer>
        </controls:ItemsRepeaterScrollHost>
    public sealed partial class BasicDemo : Page
    {
        public BasicDemo()
        {
            this.InitializeComponent();

            var random = new Random(0);
            repeater.ItemsSource = Enumerable.Range(0, 10000).Select(x => new BasicItem {
                Text = "Item " + x,
                Height = random.Next(240) + 10
            });
        }
    }

    public class BasicItem
    {
        public string Text { get; set; }
        public int Height { get; set; }
    }

Run the TestApp and use scrollbar to scroll through the list of items.

Expected behavior

Items are displayed at all positions of the scrollbar.

Screenshots

uTxNTHiVJo

Version Info

Current master (8f497521)

Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2019 Update (18362) Yes
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Mobile
Xbox
Surface Hub
IoT
ranjeshj commented 4 years ago

This seems to happen only when using mouse thumb drag and when having a large number of elements (>~5000). Works fine for touch. It also fixes itself if you scroll by a little bit after. That suggests that this might be a bug in the disconnected jump logic in stack layout when the extent is particularly large (does not repro for say 1000 items). It is likely the size of the extent that causes it as opposed to the number of items.

agnauck commented 4 years ago

I can easily reproduce this also with a much low number of items (eg. 500). It seems to depend also on the size/height of the individual items. Or on the spread of the different item heights.

I used TextBlocks to reproduce and filled them with random sentences. Here is a screenshot of the items I used. Happy to publish the code when it helps.

2020-04-14 11_44_04-ItemsRepeaterWinUI

ranjeshj commented 4 years ago

@agnauck please do share the repro app. Thanks.

agnauck commented 4 years ago

@ranjeshj I published a demo app here: https://github.com/agnauck/ItemsRepeaterWinUI

here you can adjust the number of items: https://github.com/agnauck/ItemsRepeaterWinUI/blob/7b20d82c5c864ced0a601e069207d8a4cc08427d/MainWindowViewModel.cs#L27

here you can the height of the items by adding more words or sentences to the demo data generator: https://github.com/agnauck/ItemsRepeaterWinUI/blob/master/MainWindowViewModel.cs#L34-L35

grokys commented 4 years ago

I've looked into this and here's what seems to be happening (with relevant parts of logs):

StackLayout:    EffectiveViewportChanged event callback 
StackLayout:    Effective Viewport: (0,29960,1564,1101)->(0,355815,1564,1101). 
StackLayout:    Used Viewport: (0,29960,1564,1101)->(0,355815,1564,1101). 
StackLayout:    Invalidating measure due to viewport change. 
StackLayout:    Disconnected Window 
StackLayout:    Picked anchor:2280 
StackLayout Disconnected Window - throwing away all realized elements 
 GetElement: 2280 Not AutoRecycleCandidate: 
StackLayout:    Layout bounds of anchor 2280 are (0,356159,1564,26). 
StackLayout:    Generating forward from anchor 2280. 
StackLayout:    Extent is (1564,14199796). Based on average 142. 
StackLayout Extent: (0,32399,1564,14199796). 
StackLayout:    Expecting viewport shift of (0,-31920) 
StackLayout:    Layout Updated with pending shift 0 -31920- invalidating measure 
StackLayout:    ArrangeLayout 
StackLayout:    Arranging element 2281 at (0,323902,1564,39). 
StackLayout:    Arranging element 2282 at (0,323941,1564,239). 
StackLayout:    Arranging element 2283 at (0,324180,1564,18). 
StackLayout:    Arranging element 2284 at (0,324198,1564,199). 
StackLayout:    Arranging element 2285 at (0,324397,1564,135). 
StackLayout:    Arranging element 2286 at (0,324532,1564,101). 
StackLayout:    Arranging element 2287 at (0,324633,1564,179). 
StackLayout:    Arranging element 2288 at (0,324812,1564,110). 
StackLayout:    Arranging element 2289 at (0,324922,1564,152). 
StackLayout:    Arranging element 2290 at (0,325074,1564,162). 
StackLayout:    Arranging element 2291 at (0,325236,1564,140). 

I've tried removing the code that sets m_unshiftableShift and that indeed makes scrolling work again, but only for a while: after a while of scrolling we get an unhandled exception:

Layout cycle detected.  Layout could not complete.

In addition, I think that code is needed in certain scenarios:

This can happen in cases where no scrollviewer in the parent chain can scroll in the shift direction.

I'm not sure how to proceed any further than this.

ranjeshj commented 4 years ago

@RBrid to help investigate

ad1Dima commented 4 years ago

I think i got same issue on my custom layout. Or very similar.

Is any dates when you can investigate this?

ranjeshj commented 4 years ago

@ad1Dima If you are seeing a layout cycle crash, it is very much a layout bug. If you are seeing it in your custom layout, then the bug is likely in your custom layout. You will need to debug your layout to root cause it. It is unlikely to be fixed by a change to stack layout which this bug is talking about.

ad1Dima commented 4 years ago

@ranjeshj this is not about layout cycle. this is about scrollview resetting it's offset to 0. This looks similar because it happens when I quickly scroll through my layout and the elements don't have time to draw.

ranjeshj commented 4 years ago

Thanks for clarifying.

danwalmsley commented 3 years ago

@chingucoding does some of your PRs fixing issues with items repeater / layouts fix this?

marcelwgn commented 3 years ago

@danwalmsley I'm afraid not, the issue still persists even with the latest changes made to ItemsRepeater.

@ranjeshj I can take my shot on this if that's fine.

I think a major issue for layouting is just that the layout can't know which item should be rendered where without having rendered all predecessors as the item heights are random. So I'm not sure how much there is to gain without having to render/measure all items before hand.

grokys commented 3 years ago

I think a major issue for layouting is just that the layout can't know which item should be rendered where without having rendered all predecessors as the item heights are random. So I'm not sure how much there is to gain without having to render/measure all items before hand.

Not true - AFAICS it can estimate the height of all items based on the ones currently visible and make a guess as to what to display for any particular scroll position. In fact this is working. As I mentioned in my comment if you remove the code that sets m_unshiftableShift it does work correctly for a while, until a layout cycle happens.

My problem was understanding what the "unshiftable shift" means; I assume it's there for a reason but the code just says:

This can happen in cases where no scrollviewer in the parent chain can scroll in the shift direction.

And I have no idea what that means or what could cause that. I suspect I could fix scrolling through variable height items by just removing the unshiftable shift and fixing the layout cycle problem, but I suspect that would break something else.

Is the maintainer of ItemsRepeater available to come to this thread and explain what the "unshiftable shift" does @ranjeshj ?

marcelwgn commented 3 years ago

Not true - AFAICS it can estimate the height of all items based on the ones currently visible and make a guess as to what to display for any particular scroll position. In fact this is working. As I mentioned in my comment if you remove the code that sets m_unshiftableShift it does work correctly for a while, until a layout cycle happens.

Well it can estimate, yes agreed. But it can never know for sure, and what I noticed with the thumb drag with variable sized items is that the thumb kept jumping up and down and ending up not where the mouse cursor is, which I think is less then ideal.

grokys commented 3 years ago

Yeah that's unavoidable I think. Scroll anchoring fixes it for scrolling small increments, but for big increments #435 would be required to prevent the thumb jumping.

In the meantime though, not displaying a blank list would be an improvement.

ad1Dima commented 3 years ago

But it can never know for sure, and what I noticed with the thumb drag with variable sized items is that the thumb kept jumping up and down and ending up not where the mouse cursor is, which I think is less then ideal.

ListView also works like that.

agnauck commented 3 years ago

I just wanted to kindly ask if there is an update or workaround for this issue. Its currently blocking my development because this issue happens in my application with more complex items a lot. But for performance reasons I need the ItemsRepeater

zkacso-dev commented 2 years ago

I'm running into this issue as well. The child items in my case are other lists, so they are varying in length and it happens so the last list has really few elements, while the ones before it have more. If I try to scroll down to the bottom the scroll position is constantly being put back up.

kokuda commented 1 year ago

Does anyone have a workaround for this issue? I have the same problem and found this issue by searching for ViewportManagerWithPlatformFeatures::OnLayoutUpdated which is where I discovered the m_unshiftableShift causing our issues.

I have a demo app that exhibits the same problem https://github.com/kokuda/ItemsRepeaterShiftedLayoutExample.

The problem fixes itself with some user input, is there some user input we can fake that will break the ItemsRepeater out of the m_unshiftableShift state?

codendone commented 11 months ago

I haven't tested it, but I expect the layout cycle crash should be fixed in 1.5 via the fix for #6218.