Refactor ItemContainerGenerator API

[grokys]

The ItemContainerGenerator API needs a rework; in particular its materialized events are rather confused and don't really fire when they should.

We need to decide a few things:

• Should we follow the WPF API for ItemContainerGenerator? Personally I've always found this API confusing as it splits container generation between the ItemsControl and the generator, with the generator just being used to signal that it's creating "batches" and the actual containers being created by the ItemsControl itself with a bunch of overridable methods there
• The UWP API seems to be slightly simpler but I'm unfamiliar with it
• The WinUI ElementFactory API is much nicer though it still has things I don't like much - mainly the fact that one needs to create a new ElementFactoryGetArgs object for each container generated. In particular though the ItemsRepeater events for prepared/cleared/index changed are much nicer than having to override methods in WPF/UWP

We probably want a hybrid of these APIs, which leave the main part of the API unchanged from 0.10.x for the simple use-case of defining a new container type.

[robloo]

Should Items vs ItemsSource be considered again as well: #7553

[amwx]

As an experiment, I ported of the WPF ICG a while back (which is the same in UWP, with a couple internal changes) so I've learned quite a bit about how they run the cycle of container generation (the port isn't public). After doing that, and seeing how they wire everything up, I think I actually prefer the WPF method more - although its certainly not perfect. The way I look at in WPF/WinUI, the ItemContainerGenerator is only responsible for maintaining the collection of generated items and the generation logic cycle, while the ItemsControl deals with what kind of containers to generate and how to initialize them for the UI. In Avalonia, the ICG handles almost everything, the ItemsControl just hosts the ICG. To me, the WPF way makes more sense and is less overhead in work since you never have to subclass the ICG. (BTW, I'm not trying to imply we port their ICG. Just laying out the differences for comparison since they split the workload up differently - as indicated above that's too much of a breaking change)

The main differences:

• In Avalonia we have multiple ItemContainerGenerator classes, corresponding to each of the major ItemsControls. If a new or custom ItemsControl is created, it requires a new ICG class to support it, including initialization and clearing of containers. WinUI has the IsItemItsOwnContainerOverride and GetContainerForItemOverride methods on ItemsControl to get the container, and the PrepareContainerForItemOverride and ClearContainerForItemOverride to prepare/clear the container and you never have to subclass the ICG because it doesn't care what it stores - it just stores items.
• Avalonia's ICG uses a SortedDictionary to store containers. WinUI uses a doubly-linked list approach of ItemBlock, which can be RealizedItemBlock or UnrealizedItemBlock. How this actually works is quite involved, but it works quite well. The downside to this is that when INCC actions are taken, the RealizedItemBlock has a 16-element array, which can store 'null' values (because it only coalesces the adjacent blocks if they're empty for obvious perf reasons), so it might have higher memory usage. Lookup time is slightly slower too, but is faster than O(n) because of how its setup. On the Avalonia side, the dictionary approach has faster lookup, since the key is the item index, but the downside IMO is that when you need to make changes in the middle, you have to foreach over the dictionary to change all the indices of items after the change.
• In WinUI, the ICG takes a subscription to the Items or ItemsSource. When that is modified, the generator goes through and updates the ItemBlocks. If its an addition, for example, an UnrealizedItemBlock is created where those items should go (or they're merged into an existing one if the indices overlap. ICG then raises its own event ItemsChanged to tell panels (where generation logic is carried out) that changes have been made and this is where they go. When ready, the panel will then call the generator to materialize the items (changing to a RealizedItemBlock), if necessary. In Avalonia, this is all done in one go by the ItemsPresenter.

Also, with newer UWP controls like the ListView/GridView, they rely less on the ItemContainerGenerator and have a slighly different way of running everything to support better virtualization. This newer logic is what the ItemsRepeater was derived from, so its probably somewhat similar to that, though this is all still closed source. Although, I agree, the need for the constant eventargs creation in the ItemsRepeater isn't the best.

A couple other things:

• Replace INCC actions need an optimization. In WPF, the container isn't removed from the panel unless it absolutely needs to. It just switches out the item/DataContext of the container, unless a different container type is needed (probably rare). In Avalonia, the container is always dematerialized and disconnected and then rematerialized and readded to the panel with the new item, which is inefficient.
• I also find the Dematerialize and RemoveRange methods kind of confusing. Looking at the source it took me a bit to figure out Dematerialize is used for replacing items, and RemoveRange is used for fully removing items. IMO, this should just be one method - Dematerialize - and then items can be recycled, removed, or whatever.
• One of the things I don't like about the current ICG in Avalonia is that the Materialize and Dematerialize events and the Dematerialize, RemoveRange, and Clear methods all involve allocating lists of the items involved - even for the Materialize event which is always one item at a time. In WPF, this isn't needed since the ICG's ItemsChanged event just reports indices of where changes are being made and then you work with that info to materialize or dematerialize as you need.

I'd certainly be in favor of a hybrid API though for this. I think the main thing is to not need to subclass the ICG for a new container type. If you don't want to go the WPF route with the override methods in ItemsControl, then I think remove the generic version of the ICG and make ContainerType a settable property and add a Func property for container creation. Materialize could then use that to create the container and you'd never have to write a custom ICG. Something like this: (though IMO I think overrideable methods in ItemsControl is better)

public class ItemContainerGenerator
{
    // Make this a settable property
    Type ContainerType { get; set; }

     // Equivalent to WPF IGeneratorHost.CreateContainerForItem, also replaces the existing CreateContainer method
    Func<Control> CreateContainerFunc { get; set; } 

    public void Materialize(int index, object item)
    {
        if (typeof(item) == ContainerType) // Equivalent to WPF IGeneratorHost.IsItemItsOwnContainer
        {
        }
        
        var container = CreateContainerFunc();
        //... materialize
    }
}

public class MyCustomItemsControl : ItemsControl
{
    protected override IItemContainerGenerator CreateItemContainerGenerator()
    {
        return new ItemContainerGenerator(...) 
        {
             ContainerType = typeof(MyCustomContainer),
             CreateContainerFunc = () => return new MyCustomContainer();
        };
    }
}

On a slightly different but related note/an expansion of this topic though, I'd also like to see generation logic moved to panels, as is in WinUI/WPF. Currently, there is no way to provide custom virtualization logic since everything is handled through ItemsPresenter and that is locked to either No or simple virtualization. Also doing this, if desired, one could completely skip the ItemsPresenter and just let the panel do the work (adding the IsItemsHost property) and have one less item in the tree. The tricky part here is making sure the LogicalParent is set correctly since that should be the parent ItemsControl and not the panel, and Controls doesn't work like UIElementCollection in WPF, but should be manageable. Although I'm not sure how much work this would be with a lot of existing code heavily reliant on the ItemsPresenter doing the the work and the ItemsPresenter having the Items-related properties (which really wouldn't be needed in this method). Perhaps there's a middle ground?

[grokys]

Thanks for the detailed reply @amwx, looks like you know more about the WPF/UWP/WinUI container generation than me!

Without going into the details of the implementation, I think if I were to completely rewrite the Avalonia API from scratch ignoring compatibility with Avalonia itself and WPF/UWP/WinUI I'd not even have the ItemContainerGenerator at all. As you say, it's only really used to track the generated containers, so it feels like it's exposing an implementation detail.

I think the main thing is to not need to subclass the ICG for a new container type. If you don't want to go the WPF route with the override methods in ItemsControl

I think if we're going to break the Avalonia API to that extent then I'd be in favor of essentially deprecating the ItemContainerGenerator - we could keep it around, with just the ContainerFromIndex and IndexFromContainer methods (the most used API I'd imagine) for compatibility with 0.10.x but I'd move the rest of the logic into ItemsControl itself.

On a slightly different but related note/an expansion of this topic though, I'd also like to see generation logic moved to panels, as is in WinUI/WPF. Currently, there is no way to provide custom virtualization logic since everything is handled through ItemsPresenter and that is locked to either No or simple virtualization. Also doing this, if desired, one could completely skip the ItemsPresenter and just let the panel do the work (adding the IsItemsHost property) and have one less item in the tree.

This is a good point, and something that we should try to support; if not initially then we should make sure we bear it in mind during API design.

I guess my question is:

Would people mind us breaking the API in this area for 11.0? How common is it to define a new ItemsControl? I'd imagine quite rare, and the people who have done it would probably appreciate a better API.

If the feedback is that we'd be OK with breaking the API, I'll have a think about what I think my ideal API would look like and post it here for discussion.

[grokys]

Should Items vs ItemsSource be considered again as well: #7553

Yep, this will be addressed before 11.0!

[amwx]

How common is it to define a new ItemsControl? I'd imagine quite rare, and the people who have done it would probably appreciate a better API.

For most people just consuming Avalonia, probably pretty rare. Its more useful if building a control library (which is where my POV comes from).

I think if we're going to break the Avalonia API to that extent then I'd be in favor of essentially deprecating the ItemContainerGenerator - we could keep it around, with just the ContainerFromIndex and IndexFromContainer methods (the most used API I'd imagine) for compatibility with 0.10.x but I'd move the rest of the logic into ItemsControl itself.

This is an interesting thought. Actually if you look at the docs for UWP, they say not to use those IndexFromContainer and similar methods on the ItemContainerGenerator and use the ones directly on the ItemsControl because incorrect results may be returned with the newer panels that rely less on the ICG. So it seems like even MS is trying to get people to not rely on the ICG if it can be helped. And thinking about it, outside of writing a custom control that needs to handle generation, I'm not entirely sure what the real value of having it public really is - aside from maybe those methods. You'd end up in an invalid state if you try materializing or dematerializing containers outside of the layout logic. So implementation detail is probably a good way to describe it, and wrapping it into ItemsControl is probably a good way to modernize the API.

[grokys]

@amwx investigating an API proposal now, but I have a question: any idea why item containers in WPF/UWP are DependencyObjects instead of e.g. UIElements? Is there a use-case you're aware of where the item containers won't be controls?

[grokys]

@amwx another question regarding this:

Also doing this, if desired, one could completely skip the ItemsPresenter and just let the panel do the work (adding the IsItemsHost property) and have one less item in the tree.

Do you have an example of a WPF/UWP control which does this (with source if possible)?

[amwx]

@amwx investigating an API proposal now, but I have a question: any idea why item containers in WPF/UWP are DependencyObjects instead of e.g. UIElements? Is there a use-case you're aware of where the item containers won't be controls?

No clue. Just had a quick browse over DependencyObject and they seal off Equals, so maybe this has something to do with it. But everything else is always done through DependencyObject in their APIs so it may also just be consistency? I think is one of the many areas Avalonia has improved upon WPF/UWP (not using AvaloniaObject everywhere).

@amwx another question regarding this:

Also doing this, if desired, one could completely skip the ItemsPresenter and just let the panel do the work (adding the IsItemsHost property) and have one less item in the tree.

Do you have an example of a WPF/UWP control which does this (with source if possible)?

I thought I saw once that UWP will sometimes skip the ItemsPresenter and just inject the panel straight into its VisualChildren collection, but I can't seem to replicate that now, and I think WPF always uses one. But in both WPF and UWP/WinUI its completely valid to do this and the panel will still populate:

<Setter Property="Template">
    <ControlTemplate TargetType="ItemsControl">
        <StackPanel IsItemsHost="True" />
    </ControlTemplate>
</Setter>

(In UWP, IsItemsHost isn't settable and is automatically calculated)
Of course, this completely ignores the ItemsPanel property, but if the ItemsControl is meant for a specific purpose where the panel should never change, this cleans the tree up a bit.

If you look at the Panel source, it shows how it hooks up to the parent ItemsControl/Generator and generates the items itself.

Trying to get rid of ItemsPresenter isn't all that important, its just one thing WPF/UWP support that isn't currently possible (or at least easy). What's more important is to shift the generation logic into the panels. There's at least 3 reasons I can think of:

1- Logical scrolling. In WPF, you can write a custom panel that implements IScrollInfo and get it to work. In Avalonia, it won't. Because ItemsPresenter implements ILogicalScrollable, the ScrollViewer never finds the panel and is always tied directly to the ItemsPresenter which gets its info from the Virtualizer regardless of the Panel. This IMO is the biggest reason.
2- Virtualization. Aside from allowing external virtualizing panels, different panels need virtualization to run slightly differently depending on how they layout their items (how a StackPanel virtualizes is slightly different to a WrapPanel, for example). While the generation code is the same, knowing when to start/stop is different and this allows that logic to be separated into the panel rather than concatenated in a single class with a bunch of ifs based on panel type. UWP's ListView also supports placeholder items if content is slow to load while scrolling where the panel needs full control over this. Also drag/drop with UI feedback is easier if the generated items are more accessible in the panel.
3- Something like WPF's toolbar. The ToolBarPanel generates the items and stores a list of all generated containers - then decides whether they go in the main panel or the overflow panel. This is much more complicated to achieve if ItemsPresenter handles everything.

[grokys]

But in both WPF and UWP/WinUI its completely valid to do this and the panel will still populate:

Wow, I didn't even know that was possible! I'm not a big fan of putting the code for generating items in simple (non-virtualizing) panels like StackPanel though - it feels to me that panels should just concern themselves with layout, not creating children.

Having said that, in the case of actual virtualizing panels, the reasons for moving generation logic into the panel are good ones.

My initial thoughts on a potential design:

• If the panel is a "simple" panel then ItemsPresenter should generate the items for it
• If the panel opts-in and says "i can create my own items" then ItemsPresenter won't create the items and leave generation to the panel

Perhaps in the last case, we can say that ItemsPresenter isn't needed in the template if we really want to be able to skip that extra control.

[amwx]

I'm not a big fan of putting the code for generating items in simple (non-virtualizing) panels like StackPanel though - it feels to me that panels should just concern themselves with layout, not creating children.

That's a fair point and in-line with how Avalonia generally separates things now, so let me revise my statement: I think its more important to open up the API to external/alternate options, where the generation actually happens an how we get from ItemsControl to panel is mostly irrelevant.

My initial thoughts on a potential design:

* If the panel is a "simple" panel then `ItemsPresenter` should generate the items for it

* If the panel opts-in and says "i can create my own items" then `ItemsPresenter` won't create the items and leave generation to the panel

I was thinking something similar. I thought about this some even before this discussion and my thought to avoid breaking existing code was to have an interface (or base panel class if you want to avoid interfaces, IVirtualizingPanel, for example), or even just a property, that when the ItemsPresenter finds that type of panel, it hands the generation off to the panel and doesn't do anything. But an existing simple panel, it would continue like it currently does. The trickiest part of this is making sure logical scrolling forwards its properties/events properly and ensuring generated items are still logical children of the ItemsControl and not the custom/virtualizing panel, but these should still be mostly trivial to solve

[Tulesha]

Good afternoon.
In our project we use ItemContainerGenerator. In preview-4, we inherited from ItemContainerGenerator<Control> to implement our own containers for various ItemsControls. Depending on the Enum field, we choose which container should be returned for a particular item. In other words, we use data objects as item (for example: item is MenuItemViewModel).

CommandItemContainerGeneratorBase.cs

public abstract class CommandItemContainerGeneratorBase: ItemContainerGenerator<Control>
{
    public CommandItemContainerGeneratorBase( IControl owner, AvaloniaProperty contentProperty, AvaloniaProperty contentTemplateProperty )
        : base( owner, contentProperty, contentTemplateProperty )
    {
    }

    protected sealed override IControl CreateContainer( object item )
    {
        if( item is ICommandItem cmdItem )
        {
	    return  GetItemContainer( cmdItem );
	}
	return null;
    }

    protected abstract Control GetItemContainer( ICommandItem cmdItem );
}

MenuItemContainerGenerator.cs

public class MenuItemContainerGenerator: CommandItemContainerGeneratorBase
{
    public MenuItemContainerGenerator( IControl owner, AvaloniaProperty contentProperty, AvaloniaProperty contentTemplateProperty )
        : base( owner, contentProperty, contentTemplateProperty )
    {
    }

    protected override Control GetItemContainer( ICommandItem commandItem )
    {
        switch( commandItem.Type )
	{
	    case CommandItemType.MenuItem:
	        return new MenuItem();

	    case CommandItemType.Separator:
	        return new MenuItemSeparator();

	    case CommandItemType.Custom:
	        return commandItem.GetItemContainer();

	    default:
		throw new ArgumentException();
        }
    }    
}

Now we want to upgrade to preview-6 version and keep exactly the same logic. However, we can't do this because the ItemContainerGenerator API has changed and the method signature is not what we expected.
We are now overriding the CreateContainerForItemOverride() method. However, this method does not accept an object item argument for which to build a container. Therefore, we cannot create a container dependent on item fields.
An alternative to this solution would be to override the IsItemItsOwnContainerOverride( Control item ) method, however it takes a Control argument and is called only when item is Control.

Avalonia/src/Avalonia.Controls/Presenters/PanelContainerGenerator.cs

Line 113 in ec6cd48

if (item is Control c && generator.IsItemItsOwnContainer(c))

In WPF version of our Application we override IsItemItsOwnContainerOverride( object item ) method, which calls regardless of item type. We are stored this item in local field and use in GetContainerForItemOverride() to build specific container.

MenuEx.cs

public class MenuEx: Menu
{
    private ToolStripItemViewModel m_currentItem;

    protected override bool IsItemItsOwnContainerOverride( object item )
    {
        var currentItem = item as ToolStripItemViewModel;
	if( currentItem != null )
	{
	    m_currentItem = currentItem;
	    return false;
        }

        return base.IsItemItsOwnContainerOverride( item );
    }

    protected override DependencyObject GetContainerForItemOverride()
    {
        if( m_currentItem != null )
	{
	    var item = m_currentItem;
	    m_currentItem = null;

	    switch( item.Type )
	    {
	        case WpfToolStripItemType.MenuItem:
		    return new TopLevelMenuItem();
                case WpfToolStripItemType.Separator:
                    return new MenuItemSeparator();
		default:
		    throw new ArgumentException();
	    }
        }

        return base.GetContainerForItemOverride();
    }
}

Can you please help how we can keep the same logic of preview-4 and WPF Applications in new preview-6 API?

[grokys]

That's a good question @Tulesha and one that's not addressed with the current design (or in WPF, I don't think). It is however a requirement we've discussed with customers.

In WPF version of our Application we override IsItemItsOwnContainerOverride( object item ) method, which calls regardless of item type. We are stored this item in local field and use in GetContainerForItemOverride() to build specific container.

The problem with this is that it won't work with recycling because consider the following:

• IsItemItsOwnContainerOverride(item) returns false
• The container is generated with GetContainerForItemOverride()
• The container goes offscreen and is added to the recycle pool
• A new item is made visible, that requires a different type of container
• IsItemItsOwnContainerOverride(item) returns false
• The panel gets a container from the recycle pool
• This is of the incorrect type

I think to handle this, we need to add an extra step to get a recycle key for an item. So the flow would go something like:

• IsItemItsOwnContainerOverride(item) returns false
• GetRecycleKeyForItem(item) returns a recycle key
• The container is generated with GetContainerForItemOverride(recycleKey)
• The container goes offscreen and is added to the recycle pool for recycleKey
• A new item is made visible, that requires a different type of container
• IsItemItsOwnContainerOverride(item) returns false
• GetRecycleKeyForItem(item) returns a different recycle key
• The container will be recycled from the recycle pool for that container type

Thoughts?

[Tulesha]

@grokys, that sounds good. I would like this thing to work for non-virtual panels. For example, we do not have virtualization task in Menu in our Application.
Also as an option, if GetRecycleKeyForItem is not overridden, then you can take container type as recycleKey by default.

[grokys]

@Tulesha opened a PR to add support for this: #11068