Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ItemsRepeater in ListBox etc #2594

Closed
grokys opened this issue May 29, 2019 · 8 comments
Closed

Use ItemsRepeater in ListBox etc #2594

grokys opened this issue May 29, 2019 · 8 comments

Comments

@grokys
Copy link
Member

grokys commented May 29, 2019

Our current item virtualization code has a few problems:

  1. It doesn't handle differently sized items very well (ListBox virtualization scroll to end with differing item height sometimes fails #2144)
  2. It assumes that 1 item = one scroll unit, meaning that it can't handle layouts that display multiple items per line (i.e. a wrapping panel, a grid)
  3. It doesn't handle smooth scrolling
  4. It doesn't correctly handle ListBoxItem children (Scrolling in ListBox causes stack overflow exception #2936)

One of the main problems with supporting these scenarios is that the materialization of items due to changes in the source collection happens in the CollectionChanged event handler. Beacuse this occurs outside the layout pass, changes made here then trigger a layout pass. When that layout pass occurs, the virtualization algorithm is re-run, possibly with different constraints. This can cause the item that was scrolled to to be scrolled back out of the viewport, causing #2144. This could be worked around by storing temporary state to pass to the coming layout pass, however in some cases that layout pass will not occur meaning that the temporary state will be passed to a subsequent unrelated layout pass, causing other problems.

Another problem with creating the items in the CollectionChanged handler, is that we can potentially be doing much more materialization/dematerialization than required.

Initial attempt to fix this

My initial attempt to fix this involved moving the materialization of items to the layout pass, however the fact that we use a Panel to display the containers made this difficult: you don't know which containers need to be visible until the measure pass has completed, so I started by removing all Children from the container at the start of the measure pass and adding them back in. This unfortunately broke things because as controls are removed from Children they are also removed from the visual/logical tree, causing the focused element to lose focus among other things.

The solution to this would be to modify virtualizing panels to have a "virtual" list of children for the measure pass, which is then synchronized with the real Children on arrange, or to somehow delay visual/logical tree detachment. To me these solutions seem like a hack to get around the real problem: that Panels aren't the right solution for virtualization.

And this is just to fix point 1) above.

ItemsRepeater

UWP seem to have come to the same conclusion and have introduced Attached Layouts. This is a really powerful virtualization framework that would handle all points above, plus more, including nesting and grouping. In addition this is all MIT-licenced code with its source on GitHub.

UWP doesn't use ItemsRepeater for its list boxes, assumedly for back-compat reasons, but we don't have that problem being pre-1.0, and I think it could be used to implement a new ItemsPresenter or similar in Avalonia.

Unfortunately the code is written in C++/CX and there's a quite a lot of it, so porting may not be easy.

ItemsRepeater has now been ported to Avalonia; the next step is to rewrite ItemsPresenter to use it.

@grokys
Copy link
Member Author

grokys commented May 29, 2019

cc: @ahopper

@grokys
Copy link
Member Author

grokys commented Sep 28, 2019

Now that ItemsRepeater is ported from UWP, I plan to rework ItemsPresenter to be an ItemsRepeater in the 0.10 timeline. Currently #3040 is blocking that though.

@grokys grokys changed the title Rethinking Item Virtualization Use ItemsRepeater in ListBox etc Sep 28, 2019
@ekvalizer
Copy link

Just wanna say it would be great to have the feature. Folks are waiting for it! =)

@robloo
Copy link
Contributor

robloo commented Dec 23, 2020

@grokys Commented here

We want to move to using ItemsRepeater for ItemsPresenter in #4779 but that means ItemsPanel will no longer be supported because ItemsRepeater uses attached layouts instead of panels. Should we break compatibility with both WPF/UWP and current Avalonia there? That's a big break but it gives us a single modern way of doing things, whereas UWP for example now has two ways of laying out items (panels in ItemsControl/attached layouts in ItemsRepeater)

Overall, switching to ItemsRepeater and attached layouts is exactly the type of change that I think is good for Avalonia. I think most developers can accept changes as long as they are for the better (I'm assuming all ItemsRepeater issues get solved and I know some solutions are still unknown -- it just hasn't been used enough yet). However, changing to attached layout concepts everywhere instead of panels is a difficult decision. There is a lot of history there. Also a lot of limitations with virtualization.

That said, I've only seen a Panel manually assigned a few times in production code (I bet most code isn't perfect MVVM). Just using higher-level ListBox/ListView/Canvas etc covers a lot of the cases just fine out of the box. There is usually no need to set an ItemsPanel or use an ItemsControl directly. If you decided to switch to attached layouts exclusively it might not be as big of a headache for apps to update as you might think. It is a big architecture change in the framework though.

Overall, I think the number of lines of code that will need to change in apps to switch to attached layouts is small in comparison to other things. Additionally, apps targeting WinUI+Avalonia are likely going to need separate XAML source files anyway. If the other differences get addressed in compatiblity packs though this might become bigger concern -- perhaps panels could persist in the compatiblity packs in that case.

I don't follow this area of the framework closely but I think LayoutPanel (preview) is designed to help bridge layouts and panels as well. Perhaps something along these lines would be useful for Avalonia to potentially support both concepts.

Finally, there might actually be three similar concepts.

  1. ItemsRepeater.Layout with Attached Layouts
  2. ItemsControl.ItemsPanel with Panels
  3. ListView.View with ViewBase

Cleaning these up and removing older ideas can only be a good thing long-term. You shouldn't have to keep around Microsoft's old baggage when the number of lines of code that need to change is relatively small (in my limited understanding).

@maxkatz6
Copy link
Member

I've only seen a Panel manually assigned a few times in production code

I use panel/layout properties quite often to change default orientation or spacing. Sometimes I also need to use WrapPanel/WrapLayout. So I would definitely notice that breaking change. Although I understand benefits and agree with that.

Finally, there might actually be three similar concepts.

@grokys is it even possible to create some bridge between Panel and Layout? So user created panels may still work after moving to ItemsRepeater. And could LayoutPanel actually help with it? I also don't follow this control closely, and not sure if it is even planned to be released at some point.

ListView.View with ViewBase

I have never used ViewBase from WPF, but its ListView control is way more complex than ListView in the UWP, so I usually don't want to compare them. It's more like lightweight DataGrid.

@robloo
Copy link
Contributor

robloo commented Dec 23, 2020

@maxkatz6

I use panel/layout properties quite often to change default orientation or spacing. Sometimes I also need to use WrapPanel/WrapLayout.

Ok, that's fair. Still, a lot of cases simply throw a horizontal stackpanel inside a scrollviewer and call it a day. I'm not sure how many apps actually support changing all this from the VM. Maybe you have more modern/advanced apps but a lot of code I've seen follows older windows forms ideas and clean MVVM is never used anywhere close to 100%. This means it's all pretty basic and changing panels or layouts is hardly ever done. Maybe that's just the industry-specific LOB apps I've seen though. Impossible to say how prevalent this actually is. Just a viewpoint difference I think.

Overall, I think the number of times where advanced layouts are used by specifying panels or an attached layout is actually not that much in any given application -- even modern full-MVVM ones. This means the amount of code that actually has to change to switch to attached layouts entirely isn't that much. Which means the cost is low and any maintenance burden keeping an app targeting Avalonia + WPF is also low. My main point is that the change is far more trivial for apps to make than it is for the framework -- assuming attached layouts can do everything panels have done historically. It also simplifies a lot and makes virtualization a first-class citizen in all controls. I'm willing to change over my apps for this but can't speak for others -- this is just my vote.

Sometimes I also need to use WrapPanel/WrapLayout

Edit: Yes, that's along the lines of where I think it's most useful. The last time I used it personally was for a UniformGrid for a color palette

ListView.View with ViewBase

I have never used ViewBase from WPF, but its ListView control is way more complex than ListView in the UWP, so I usually don't want to compare them. It's more like lightweight DataGrid.

I only added that for reference. All my experience with ListView comes from UWP/WinUI as well. In WPF I only recall seeing/using ListBox. Still, it seems it has it's own way of doing layouts in WPF so that shouldn't be forgotten about. I'm fine (if ListView comes to Avalonia) to use the UWP implementation.

@grokys
Copy link
Member Author

grokys commented Aug 23, 2021

Update on this: I've given up hoping that the upstream WinUI ItemsRepeater will ever be fixed enough to use it as a core component of our ItemsControls, in particular these issues are blockers:

microsoft/microsoft-ui-xaml#1829
microsoft/microsoft-ui-xaml#1422

I have a proof of concept for a virtualized stack panel which may fix many of our current problems with ListBox, and also allow smooth scrolling without breaking API compatibility, I hope to get more time to work on it in a few months.

Closing this issue for now.

@grokys grokys closed this as completed Aug 23, 2021
@robloo
Copy link
Contributor

robloo commented Aug 23, 2021

This is unfortunate, but we are all in the same position with WinUI and UWP. Hopefully Microsoft has resources to work on things like this again next year but they are already over a year late with WinUI3 and I kind of doubt they will even manage to get feature parity with UWP in 2022. So its smart not to be dependent on their schedule (or lack thereof).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants