ThemeDictionary + ThemeVariantScope

[maxkatz6]

What does the pull request do?

See: https://github.com/AvaloniaUI/Avalonia/discussions/7886

How was the solution implemented (if it's not obvious)?

More detailed descriptions to be added In short, two major differences comparing to proposal discussion from above:

1. Theme property is an inherited property, so we can override it somewhere in the middle of the tree. It's flexible and performance wise I don't think we care about rare theme-switching operations (it's still fast tho).
2. Developers would expect DynamicResource to work with theme dictionaries as well. As for me it doesn't make much more sense to add ThemeResource on top of it (at least not in this PR).
3. Inherited themes are supported with very simple implementation but might be sufficient.

Note that DynamicResource technically has to do one more dictionary lookup per each ResourceDictionary object now. But on practice it should affect only developers who actually use non-empty ThemeDictionary. So, in overall overhead should be minimal for existing applications. Anyway, better to benchmark.

Also, API and feature in general is not exactly the same as in UWP. These differences should be documented. Though it's hard to guess on under-the-hood UWP behavior without knowing its source code.

Checklist

• [x] Added unit tests (if possible)?
• [x] Added XML documentation to any related classes?
• [ ] Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation

Additional TODO

• [ ] Benchmark startup performance, theme switching, compare to master (at least it shouldn't be slower)
• [ ] See if we can move control-specific resources from single Base.xaml to each style file of the control. Should be possible now, but with performance degradation. Need to try and benchmark.
• [ ] Double check if current API is flexible enough to be extended for system themes. We want application to change dark/light mode when it's changed in the system (not a goal of this PR).

Breaking changes

SimpleThemeMode and FluentThemeMode enums/properties were removed (possible to restore tho).

https://user-images.githubusercontent.com/3163374/169640920-7fd91523-7a95-496e-979d-95934a790e50.mp4

[maxkatz6]

First benchmarks...

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.25115 Intel Core i5-9600K CPU 3.70GHz (Coffee Lake), 1 CPU, 6 logical and 6 physical cores .NET SDK=6.0.300 [Host] : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT DefaultJob : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT

ResourceBenchmarks

Master:

Method	Mean	Error	StdDev	Allocated
FindPreResource	536.54 us	5.687 us	5.319 us	-
FindPostResource	22.87 us	0.209 us	0.196 us	-
FindNotExistingResource	567.82 us	10.430 us	14.277 us	-

This PR:

Method	Mean	Error	StdDev	Allocated
FindPreResource	517.46 us	5.802 us	4.845 us	-
FindPostResource	22.40 us	0.360 us	0.337 us	-
FindNotExistingResource	536.93 us	4.226 us	3.953 us	-

FluentBenchmark

Master:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
RepeatButton	234.7 us	4.67 us	7.41 us	14.1602	0.7324	65 KB

This PR:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
RepeatButton	188.2 us	1.37 us	1.22 us	14.4043	0.7324	67 KB

ThemeBenchmark

Master:

Method	themeUri	Mean	Error	StdDev	Median	Allocated
InitDefaultTheme	Dark	6.803 ms	0.1334 ms	0.1248 ms	6.758 ms	1 MB
InitDefaultTheme	Light	5.691 ms	0.2675 ms	0.7889 ms	5.405 ms	1 MB
InitFluentTheme	Dark	12.361 ms	0.2471 ms	0.7008 ms	12.127 ms	2 MB
InitFluentTheme	Light	12.091 ms	0.2283 ms	0.5337 ms	12.035 ms	2 MB

This PR (only Dark):

Method	Mean	Error	StdDev	Allocated
InitDefaultTheme	3.870 ms	0.2114 ms	0.6234 ms	2 MB
InitFluentTheme	5.219 ms	0.2442 ms	0.7007 ms	3 MB

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0020612-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[grokys]

A few more benchmarks, this time ControlsBenchmark:

master

|         Method |        Mean |     Error |    StdDev |    Gen 0 |    Gen 1 |   Gen 2 | Allocated |
|--------------- |------------:|----------:|----------:|---------:|---------:|--------:|----------:|
| CreateCalendar | 21,481.0 us | 130.35 us | 121.93 us | 718.7500 | 312.5000 | 31.2500 |  4,275 KB |
|   CreateButton |    155.2 us |   0.75 us |   0.62 us |  10.0098 |   1.2207 |       - |     41 KB |
|  CreateTextBox |    903.5 us |   8.47 us |   7.92 us |  50.7813 |   9.7656 |       - |    218 KB |

themeresource

|         Method |        Mean |     Error |    StdDev |   Gen 0 |  Gen 1 | Allocated |
|--------------- |------------:|----------:|----------:|--------:|-------:|----------:|
| CreateCalendar | 17,545.7 us | 315.37 us | 705.36 us |       - |      - |  4,436 KB |
|   CreateButton |    149.7 us |   1.05 us |   0.98 us | 10.2539 | 1.2207 |     43 KB |
|  CreateTextBox |    864.6 us |   2.72 us |   2.54 us | 51.7578 | 9.7656 |    221 KB |

So there seems to be quite a decent speedup. More allocated memory, but that's offset by no GC in the calendar benchmark.

[grokys]

I've not fully reviewed this PR, but so far I only have two comments:

• As mentioned in the issue, the term "theme" is overloaded. Maybe something like "theme variant" might be clearer?
• I'm a little concerned about dynamic resources now having to make two subscriptions for each resource

I realise that the second point was done so that {DynamicResource} could be used, with no separate {ThemeResource}, but I wonder if this may hamper optimization in future? However given the benchmarks above, maybe it's not much of an issue.

[maxkatz6]

Internally we agreed on ThemeVariant property name as for this moment. Might be changed depending on ControlTheme PR status. We want to avoid naming conflicts. ThemeControl should also be renamed to define only a scope for theme variant (dark/light...), and not the whole theme (fluent/default). "ThemeDictionary" probably still makes sense as it is now.

[maxkatz6]

We now have Control.Theme property on each control. So, we need some other naming options:

• Control.ThemeVariant (of ThemeVariant type) - after previous discussion this one was prefered
• Control.ThemeMode (of ThemeMode type) - sounds natural as for me
• Control.ThemeScheme (of ThemeScheme type)

Other frameworks:

• UWP - ElementTheme
• GTK - GTK_THEME=theme:variant (i.e. GTK_THEME=Adwaita:dark, similarly how we plan it in Avalonia)
• Qt - it doesn't support it natively as it seems, but developer can provide custom QtPallete
• UIKit/AppKit (apple) - UIUserInterfaceStyle (seems like community uses term theme there a lot)
• SwiftUI (apple) - colorTheme
• Flutter - Brightness (in a context of Theme Brightness), that's unusual one
• Electron - Theme (nativeTheme)
• HTML5 - color-scheme (prefers-color-scheme)
• Xamarin - OSAppTheme

Bottom line, it feels like no matter what we choose, there is no "correct" term for dark/light mode or theme or style or brightness or variant or [put your option here].

[grokys]

Yeah I think I still prefer Control.ThemeVariant. colorTheme from SwiftUI is a possibility, but given that it can control more than just colors I still prefer ThemeVariant. At least there's some precedent from GTK :)

[workgroupengineering]

AppThemeColor?

[maxkatz6]

so far:

master:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
CreateCalendar	15,253.55 us	84.741 us	75.121 us	687.5000	328.1250	46.8750	4,141 KB
CreateCalendarWithLoaded	15,708.73 us	227.136 us	201.350 us	687.5000	343.7500	62.5000	4,167 KB
CreateButton	89.78 us	0.720 us	0.638 us	6.4697	0.2441	-	30 KB
CreateTextBox	858.24 us	3.019 us	2.676 us	48.8281	7.8125	-	228 KB

this PR:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
CreateCalendar	16,500.41 us	257.406 us	228.184 us	687.5000	281.2500	93.7500	4,194 KB
CreateCalendarWithLoaded	16,395.55 us	145.267 us	135.882 us	718.7500	312.5000	93.7500	4,220 KB
CreateButton	93.46 us	0.656 us	0.614 us	6.7139	0.2441	-	31 KB
CreateTextBox	869.42 us	4.784 us	4.241 us	48.8281	7.8125	-	230 KB

master:

Method	Mean	Error	StdDev	Allocated
FindPreResource	543.42 us	7.249 us	6.780 us	-
FindPostResource	21.44 us	0.147 us	0.123 us	-
FindNotExistingResource	537.70 us	4.012 us	3.752 us	-

this PR:

Method	Mean	Error	StdDev	Allocated
FindPreResource	594.15 us	7.474 us	6.992 us	-
FindPostResource	23.96 us	0.114 us	0.095 us	-
FindNotExistingResource	591.75 us	8.747 us	7.754 us	-

master:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
RepeatButton	148.3 us	2.93 us	3.91 us	6.1035	2.1973	38 KB

this PR:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
RepeatButton	136.3 us	2.71 us	5.95 us	6.3477	2.4414	39 KB

master:

Method	mode	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
InitFluentTheme	Light	647.7 us	8.80 us	8.23 us	84.9609	41.9922	525 KB
InitFluentTheme	Dark	629.2 us	11.93 us	11.71 us	84.9609	41.9922	525 KB
InitSimpleTheme	Light	314.9 us	2.60 us	2.30 us	43.9453	14.6484	222 KB
InitSimpleTheme	Dark	310.1 us	6.02 us	5.91 us	42.9688	15.6250	230 KB

this PR:

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
InitFluentTheme	986.3 us	3.66 us	3.42 us	136.7188	68.3594	848 KB
InitSimpleTheme	288.5 us	2.19 us	2.05 us	47.3633	14.1602	229 KB

[timunie]

AppThemeColor?

For me that would conflict with AccentColor for newbies. Just my two cents.

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0022956-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0023189-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[maxkatz6]

@DrWenz sorry to bother you, but I wonder how this PR affects startup time on your low end devices. From my testing with control catalog and dotTrace on my machine (which isn't really a low end), I see something about 5-10% startup time degradation. With R2R there are no measurable difference at all. Meanwhile micro benchmarks from above also show some degradation on resources lookup. But it's hardly reliable, as we don't have automated benchmarks for any big application as for now.

[timunie]

@maxkatz6 did you re-run your benchmark after your last edits? I can imagine that removing unnecessary foreach loops can improve performance unless it was never or only rarely hit.

[DrWenz]

@DrWenz sorry to bother you, but I wonder how this PR affects startup time on your low end devices. From my testing with control catalog and dotTrace on my machine (which isn't really a low end), I see something about 5-10% startup time degradation. With R2R there are no measurable difference at all. Meanwhile micro benchmarks from above also show some degradation on resources lookup. But it's hardly reliable, as we don't have automated benchmarks for any big application as for now.

Sure I can do some tests, do you want a comparison with 0.10.18 or the last master?

[maxkatz6]

@DrWenz master. As we already know that master (and likely this PR) is faster than 0.18.0.

@timunie yes, I did rerun some of the benchmarks

[DrWenz]

@maxkatz6 as I can see, FluentThemeMode is removed and Dark/Light is handled via separate ResourceDictionary. Any other changes I should consider or adjust in my app, in regards to this PR, when I run some tests?

[maxkatz6]

@DrWenz instead of FluentThemeMode there is an Application.ThemeVariant now. Which also can be set on each window independently. While having single resource dictionary for all of these variants.

If you have custom resources for dark/light modes that you might had to manually detach/attach, it can be simplified now with ThemeDictionaries and improve some perf as well. If don't have these existing custom resources will continue to work same way as before, so nothing else needs to be changed.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0023387-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0023672-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[maxkatz6]

To consider https://github.com/microsoft/WindowsAppSDK/issues/41 https://github.com/dotnet/winforms/issues/5166

[maxkatz6]

@AvaloniaUI/core and @amwx @SKProCH as maintainers of biggest third-party themes for Avalonia. This PR is again ready to review, now with updated description which should cover everything (and basically be copy pasted into documentation page). There are some issues with text rendering in Avalonia, unrelated to this PR, but makes harder to review. But at least please take a look at the API surface. From what I checked, there shouldn't be any issues with integrating theme variants and theme dictionaries into third party themes. But just in case.

[amwx]

@maxkatz6 Can you try to fix the build so there's a package with the new API design? It'll be easier for me to test this than just look at the API surface.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0028798-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

[maxkatz6]

@amwx thanks, done.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0028801-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

[amwx]

@maxkatz6 Found one issue so far trying to integrate this with FluentAvalonia. Light and Dark switching works perfectly, but it will not work with other themes (HighContrast specifically). The resource lookups are coming back with transparent. What seems to be happening is (example): Brush: ApplicationPageBackgroundThemeBrush Maps to: SystemColorWindowColor

When the DeferredItem factory runs for the Brush, the lookup for SystemColorWindowColor is done with null ThemeVariant which ends up looking in the Default dictionary (I've followed you so its Light), where that resource isn't defined so it ends up returning transparent. I think the Markup extensions are going to need context/theme awareness so the lookup is done correctly.

Interestingly, when Dark (for example) mode is used, when that Brush initializes in ResourceDictionary.TryGetValue(), when the item factory runs, the very next call to TryGetValue is on the same dark theme resource dictionary. When using the HighContrast theme, the next call to TryGetValue after the factory isn't the theme dictionary, but the main ResourceDictionary on FluentAvaloniaTheme (which is of type Styles). Not really sure what's causing that difference.

Note: that I just kind of threw this PR into FA so it's possible I'm doing something that's messing with it. The only special thing I do is in FATheme I force search the Application ResourceDictionary first for overriding purposes. But I took that out to test and the same behavior occurs. I'll test some more tomorrow

Otherwise, I think the API itself is fine. IMO I'm not a huge fan of the "Default" variant (I don't like it in WinUI either) as it is ambiguous as to its meaning, especially if the meaning can be changed between Avalonia & third parties. I haven't looked closely enough to know if the ThemeVariant.Default member has a purpose, but at least in the ThemeDictionaries IMO I think we should use actual theme names and just implicitly map "Default" to "Light"

[timunie]

maybe FallbackTheme is a better naming than Default? At least for me it would be easier to understand what is meant by this, a fallback to search if the other lookup was not successful.

[maxkatz6]

The problem is, "ThemeVariant.Default" is used in two different ways depending on the context. RequestedTheme="Default" will reset variant on the current control. Basically, sets it to "Unset". ResourceDictionary x:Key="Default" will work as a fallback resource.

[SKProCH]

@maxkatz6 in Material.Avalonia we have BaseThemeMode (Light and Dark actually) and also Primary and Secondary colors (which may be any color). Do I understand correctly that this change only covers BaseThemeMode in my case?

[maxkatz6]

@SKProCH yes, only dark/light mode, I don't think there is any meaningful way to abstract app accent colors across the app. As different themes might have one or many accent colors. Like material 3 on android which has three accent color.

[amwx]

@maxkatz6 Expanding on the issue I noted, take this repro into the Sandbox project:

MainWindow:

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
        x:Class="Sandbox.MainWindow"
        Background="#202020">

  <Border Width="300" Height="300"
          Background="{DynamicResource UniqueColorBrush}" />
  
</Window>

MainWindow.cs

public ThemeVariant CustomTheme = new ThemeVariant("Custom")
{
   InheritVariant = ThemeVariant.Light
};

public MainWindow()
{
    this.InitializeComponent();
    this.AttachDevTools();

    // A method to switch themes
    KeyDown += (s, e) =>
    {
        switch (e.Key)
        {
            case Avalonia.Input.Key.L:
                Application.Current.RequestedThemeVariant = ThemeVariant.Light;
                break;

            case Avalonia.Input.Key.D:
                Application.Current.RequestedThemeVariant = ThemeVariant.Dark;
                break;

            case Avalonia.Input.Key.Space:
                Application.Current.RequestedThemeVariant = CustomTheme;
                break;
        }
    };
}

Now, take this ResourceDictionary and put it in the Window's Resources:

<ResourceDictionary>
  <ResourceDictionary.ThemeDictionaries>
    <ResourceDictionary x:Key="Default">
      <Color x:Key="UniqueColor">Red</Color>

      <SolidColorBrush x:Key="UniqueColorBrush" Color="{StaticResource UniqueColor}" />
    </ResourceDictionary>
    <ResourceDictionary x:Key="Dark">
      <Color x:Key="UniqueColor">Blue</Color>

      <SolidColorBrush x:Key="UniqueColorBrush" Color="{StaticResource UniqueColor}" />
    </ResourceDictionary>
    <ResourceDictionary x:Key="Custom">
      <Color x:Key="UniqueColor">Purple</Color>

      <SolidColorBrush x:Key="UniqueColorBrush" Color="{StaticResource UniqueColor}" />
    </ResourceDictionary>
  </ResourceDictionary.ThemeDictionaries>
</ResourceDictionary>

So far this should all work as expected.

Now make the following changes:

• Remove the resources from the light/dark theme dictionaries, but leave the Color and Brush in the Custom dictionary
• Change the binding on the brush color to a DynamicResource.

Switch to the Custom theme, then to either light or dark, then back to custom and the Border will be transparent. If you return the light/dark resources (keeping the custom theme as a DynamicResource) it will work. Interestingly, only the colors need to be kept in all dictionaries for the lookup to succeed, removing just the brushes from light/dark have no effect.

Now, remove the ResourceDictionary from the window. Copy the original (from above) to the Application resources.

Change the binding on custom theme's brush to DynamicResource (but leave the others active). The lookup will fail and fall back to Light theme (Red) when you switch to the Custom theme (this worked in the Window's resources, but fails at the App level). Thus, not having the resources present in light/dark would lead to the lookup failing, and this is basically the same issue/result with the HighContrast theme I already noted.

It seems to be an issue with DynamicResource

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0029177-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

[maxkatz6]

@amwx I investigated this issue and found surprising bug with our bindings support. See https://github.com/AvaloniaUI/Avalonia/issues/10110

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0029334-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]