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

Refactor ItemsControl container generation and implement smooth virtualization #9677

Merged
merged 70 commits into from
Jan 17, 2023

Conversation

grokys
Copy link
Member

@grokys grokys commented Dec 12, 2022

What does the pull request do?

This PR refactors item container generation for ItemsControl as discussed in #9497 to be more similar to the WPF/UWP model. Given that this required a major refactoring of our virtualization logic, I also rewrote VirtualizingStackPanel to use "smooth" virtualization based on the code from TreeDataGrid. Now all ListBoxes etc use smooth virtualization and handle variable-sized items by default. There should now no longer be a problem with using virtualization in combination with with inline items (#4265 and others).

XCjM39nCrh.mp4

The main changes from the point of view of a regular consumer of ItemsControl-related controls are:

  • ListBox.VirtualizationMode has been removed, the virtualization mode is changed by changing the ItemsPanel
  • Carousel.IsVirtualizing has been removed, there is now only a "virtualizing" panel
  • There is no longer any "simple" stack virtualizing mode:
    • StackPanel should be used if no virtualization is required
    • VirtualizingStackPanel should be used if "smooth" virtualization is required
  • Item container lookup was moved to ItemsControl as in UWP (old methods are left on ItemContainerGenerator marked with [Obsolete]):
    • ItemsControl.ContainerFromIndex(object item)
    • ItemsControl.IndexFromContainer(Control container)

The main changes from the point of view of a virtualizing panel author are:

  • ItemsPresenter is no longer responsible for generating containers, the responsibility is moved to VirtualizingPanel
  • ItemContainerGenerator should no longer be inherited to modify the container type for an ItemsControl, instead override methods on ItemsControl

A more detailed list of changes is listed in the Breaking Changes section.

Now the procedure to create a virtualizing panel is as follows:

  • Derive the panel from VirtualizingPanel and implement the abstract members
  • To create a container use the methods on ItemContainerGenerator from VirtualizingPanel:
    • ItemContainerGenerator.IsItemItsOwnContainer(Control) should first be called if the item is derived from the Control class. If this method returns true then the
      item itself should be used as the container.
    • If IsItemItsOwnContainer(Control) returns false then ItemContainerGenerator.CreateContainer()` should be called to create a new container.
    • The `ItemContainerGenerator.PrepareItemContainer(Control, object?, int) method should be called for the container.
    • The container should then be added to the panel using VirtualizingPanel.AddInternalChild(Control)
    • Finally, ItemContainerGenerator.ItemContainerPrepared(Control, object?, int) should be called.
    • If the index of the container changes then ItemContainerGenerator.ItemContainerIndexChanged(Control container, int oldIndex, int newIndex) should be called
    • When the item is ready to be recycled, ClearItemContainer(Control) should be called if returned false.
  • The realized containers should be stored in the VirtualizingPanel-derived class. ItemContainerGenerator DOES NOT store the realized containers anywhere, unlike WPF/UWP
    • VirtualizingPanel.ContainerFromIndex and VirtualizingPanel.IndexFromContainer should be implemented to query this collection of realized containers
  • The VirtualizingPanel-derived class is also responsible for storing its own list of recyclcable containers and using these when necessary

In addition, this PR finally fixes one of our oldest issues (#54): ItemsControl now exposes an ItemsView property which represents the Items wrapped in an ItemsSourceView.

Breaking Changes

ItemsControl

Item container generation/lifecycle API was changed to be similar to WPF/UWP.

  • One no longer has to derive from ItemContainerGenerator to change the container type
    • ItemContainerGenerator cannot be overridden from user code any more
      • There's one internal exception to this: TreeView creates a TreeItemContainerGenerator for backwards compat
  • Add protected virtual methods for container creation/recycling:
    • ItemsControl.IsItemItsOwnContainerOverride
    • ItemsControl.PrepareContainerForItemOverride
    • ItemsControl.PrepareContainerForItemOverride
    • ItemsControl.ContainerIndexChangedOverride
    • ItemsControl.ClearContainerForItemOverride

Item container lookup was moved to ItemsControl as in UWP (old methods are left on ItemContainerGenerator marked with [Obsolete])

  • ItemsControl.ContainerFromIndex(object item)
  • ItemsControl.ContainerFromItem(object item)
  • ItemsControl.IndexFromContainer(Control container)
  • ItemsControl.ItemFromContainer(Control container)

ItemContainerGenerator

  • Exposes the container creation/prepare/clear lifecycle events:
    • IsItemItsOwnContainer
    • CreateContainer
    • PrepareItemContainer
    • ItemContainerPrepared
    • ItemContainerIndexChanged
    • ClearItemContainer
  • Remove ItemContainerGenerator<T> and move creation of containers to protected methods in ItemsControl (above)
  • DOES NOT store the realized containers anywhere, unlike WPF/UWP:
    • It is new the responsibility of the virtualizing panel to store this; the container mapping methods on ItemsControl are forwarded to the VirtualizingPanel
    • Justification: the virtualizing panel will usually need to store this information in a suitable data structure anyway, no point in storing it twice
  • Remove IItemContainerGenerator
  • Remove ItemContainerInfo
  • ItemContainerGenerator.Containers is now ItemsControl.GetRealizedContainers()
  • Deprecate ItemContainerGenerator.ContainerFromIndex and ItemContainerGenerator.IndexFromContainer, which now just call relevant methods on ItemsControl

Questions

  • In WPF, ItemFromContainer returns UnsetValue if the container was not found, in UWP it unhelpfully returns System.__ComObject. Not a big fan of either of these. Currently it's implemented here as returning null but null is actually a valid item. Should we change this method to bool TryGetItemFromContainer(out object? item)?
  • ItemContainerGenerator events are replaced by ItemsControl.PrepareContainerForItemOverride and ItemsControl.ClearContainerForItemOverride
    • There are no longer any materialized/dematerialized events, they don't exist in WPF/UWP either. Do we need them?

ItemsPresenter

  • ItemsPresenter now only generates containers for simple (non-virtualizing) panels
  • Removed IItemsPresenterHost interface
  • Removed IItemsPresenter interface
  • Removed Items property
  • Removed ItemContainerGenerator property
  • ItemsPresenter can now only be used in an ItemsControl template, not standalone (same as in WPF/UWP etc)

VirtualizingPanel`

  • The IVirtualizingPanel interface has been removed. Virtualizing panels should inherit from VirtualizingPanel

TabControl

  • Removed HeaderDisplayMemberBinding: DisplayMemberBinding should select the member for the header (in WPF it's DisplayMemberPath)

Carousel

  • Now implemented with a logical-scrolling Panel; Carousel syncs the horizontal scroll offset with the currently selected item
  • Only VirtualizingCarouselPanel currently implemented

Problems:

  • When using Rotate3DTransition we see the old item flicker up when the transition finishes

ItemsSourceView

Fixed Issues

Fixes #54
Fixes #2177
Fixes #2259
Fixes #2868
Fixes #2936
Fixes #3506
Fixes #4265
Fixes #4701
Fixes #6480
Fixes #7467
#7706 is partially fixed. The stack overflow is fixed, but the ScrollBar gets confused, that part will need to be fixed separately
Fixes #8086
Fixes #8181
Fixes #8344
Fixes #8495
Fixes #9269

A lot still broken, in particular virtualization is completely removed.`ItemsPresenter` now no longer has an `Items` or `ItemTemplate` property; it detects when it's hosted in an `ItemsControl`. `IItemsPresenter` interface removed.
Currently only supports smooth scrolling.
If you want a different virtualization mode, use a different panel.
A bunch of tests still failing, and some code commented out, but outlines the new API shape.
`ListBox` now needs a root as it uses a `VirtualizingStackPanel`.
Use a slightly different approach to the one that was previously there: create an item template that contains `DisplayMemberBinding`. This is the approach that WPF broadly uses too.
When `ItemsControl.DisplayMemberBinding` or `ItemTemplate` changes, refresh the containers.
As `DisplayMemberBinding` should select the member for the header (in WPF it's `DisplayMemberPath`). Also set `TabItem.TabStripPlacement`.
Wasn't finding `RelativeSource` type at times.
@rabbitism
Copy link
Contributor

@grokys Hi, I noticed that after migrating to preview5, ListBox no longer wraps items in container. Is this an expected behavior?
Also every child element is attached to logical tree twice.

image

@GabrielHalvonik
Copy link

There is regression with Data Virtualization library I use, the items are not rendered property in ListBox

Seems to be caused by the fact that the compositing renderer uses floats and the scrollable extent is too large to properly represent. Minimal repro:

  <ScrollViewer>
    <StackPanel>
      <Button Margin="0 1000000000 0 0">0</Button>
      <Button>1</Button>
    </StackPanel>
  </ScrollViewer>

Turning off the compositing renderer fixes it, but note that winui also can't handle this and looks like firefox also has problems with very large coordinates. Probably not something that would cause problems in real-life cases.

May I ask if there is some initiative to change it to higher precision, so virtualised panels with many elements (in millions) have no offset scattering at all? I think I found a way around it, but the implementation is quite difficult and not very nice, so before I spend much time with it I would like to know, if there is not tendency to (re)implement it in some of the coming releases. Thanks

@maxkatz6
Copy link
Member

if there is some initiative to change it to higher precision

No, none as for now.

I think I found a way around it, but the implementation is quite difficult and not very nice

Worth to create a new enhancement issue with details.

@GabrielHalvonik
Copy link

GabrielHalvonik commented Feb 21, 2023

Worth to create a new enhancement issue with details.

My solution is probably not generally applicable so I will describe it there, and if someone find that approach acceptable to be part of framework, he can create proper issue for it. For now I will describe it here.

Basically I kind of overlapped virtualised ItemsControl positioned inside Panel with my own Control (StackPanel of elements of same type and style as ItemTemplate in ItemsControl and that one is replaced with just dummy Decorator with specific Height), so ItemsControl works basically just as positioning calculator (seems like positioning is calculated properly, just render positions are scattered), and those position changes are delegated to my own overlapped StackPanel.
Those items in my overlapped StackPanel are created in advanced and ClippedToBounds with count of following formulae: ((int)Math.Ceiling(ScrollViewer.Viewport.Height / HeightOfRow) + 1)
and are adjusted (either added or removed) as ViewportHight of that hidden ScrollViewer changes.
When ScrollViewer.ScrollChanged event is fired, I retrieve currently realised elements from underlying ItemsControl, grab its object that has been given to that dummy element in FuncDataTemplate process and use it in own inner StackPanel inner control templating. Also I change offset of first child of own overlapped StackPanel with following formulae:

OwnPanel.Children.First().Margin = new (0, top: -(ScrollViewer.Offset.Y - ItemsControlRealisedItems.First().Value.Bounds.Top), 0, 0)
, so scrolling is smooth.
Also scroll event is rerouted from my overlapped Control to underlying ScrollViewer with ItemsControl as its child.

I find it not very elegant way of doing ui (overlapping dummy controls), but at the end it works kind of well from user's perspective (at least in my simple use-case). But more elegant way may probably be to kind of extract virtualized positioning algorithm and omit rendering capability from ItemsControl and kind of bind it to something like described above. (did not dive deeply into how virtualisation is currently done)

Hope I describe it understandably. I would provide minimal repro sample, but I have kind of too bloated project and not enough time to scrape it atm.

Here is also video how it works compared to default positioning:
virtualscroll

[full video]
https://user-images.githubusercontent.com/60801120/220483854-3c90e9c9-3fbe-4bc2-895e-58b053368c9a.mov

@timunie
Copy link
Contributor

timunie commented Feb 22, 2023

@GabrielHalvonik a feature request doesn't mean you need to provide a pull request, even if for sure pull requests are welcome. If the issue is kept here, it will be lost. It would be really nice if you can transfer your idea into a new issue as a feature request.

To your approach in general: I wonder if it would be possible to implement an own VirtualizingStackPanel with the needed calculations inside, as you already suggested above. You can alsways look up the source code of Avalonia to find out how things works internally.

Happy coding
Tim

@danipen
Copy link

danipen commented Feb 22, 2023

Playing with the Virtualization demo in the samples and I noticed that using a big amount of elements (2,000,000) and using random heights, accumulates some error in the measurement...
virtualization-error

@timunie
Copy link
Contributor

timunie commented Feb 22, 2023

I still think it's not wise to discuss it in a merged PR.

@grokys
Copy link
Member Author

grokys commented Feb 22, 2023

Yes, please open an issue to discuss this; please don't try to fix it before its discussed as there are various different ways in which this could be fixed and we need to decide where to fix it first.

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