Performance issue related to Maui Styles

arivoir commented 1 year ago

Description

I found a performance issue in a grid control that is setting the "Style" property of the cells and found the issue is caused because the OnPropertyChanged("Background") method is called even if the background doesn't change.

I checked the brush set in the old and the new style is the same When the style set is exactly the same object the OnPropertyChanged("Background") is not called (correct), but if the style is a copy of the preview the method is called (Incorrect), OnPropertyChanged("Background") should only be called if the Background changes.

Steps to Reproduce

Create a sample from above description.

Link to public reproduction project repository

No demo

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows, macOS

Affected platform versions

It affects all the platforms

Did you find any workaround?

I can detect whether the styles are the same (despite they are not the same instance), but if another property is different I need to set the style anyway and this will cause the background changed to be triggered.

Relevant log output

No response

arivoir commented 1 year ago

In case anyone needs the partial workaround, I used below code to check before setting a style, it doesn't solve the problem but it helps under certain conditions

public static bool IsSameStyle(Style style1, Style style2)
{
    if(ReferenceEquals(style1, style2)) return true;

    if(style1 == null || style2 == null) return false;

    if(style1.TargetType != style2.TargetType) return false;

    if(style1.Setters.Count != style2.Setters.Count) return false;

    for (int i = 0; i < style1.Setters.Count; i++)
    {
        var setter1 = style1.Setters[i];
        var setter2 = style2.Setters[i];
        if(setter1.Property != setter2.Property) 
            return false;
        if (setter1.Value == null && setter2.Value == null)
            continue;
        if(!setter1.Value.Equals(setter2.Value)) 
            return false;
    }
    return true;
}

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

rachelkang commented 1 year ago

@StephaneDelcroix any thoughts on this one?

MartyIX commented 7 months ago

Was this fixed in .NET 8?

dotnet / maui