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

New API for XAML group transformers and AvaloniaRuntimeXamlLoader.LoadGroup #9537

Merged
merged 14 commits into from
Nov 30, 2022

Conversation

maxkatz6
Copy link
Member

What does the pull request do?

Partial reimplementation of my previous PR with bunch of new features (mostly for internal use).

What is the current behavior?

There was no way to:

  • For any feature requiring transforming of nodes with information of all XAML documents in the project, it was necessary to use Cecil IL code emit, after main XAML files were already emitted.
  • Easily handle debug information for these transformations
  • Unit test combination multiple XAML files without mocked asset loader hacks
  • As a result of above, StyleInclude/ResourceInclude transformers aren't really usable in current master

What is the updated/expected behavior with this PR?

It is possible now to:

  • Process XAML documents together after they all were parsed and transformed, but before they were emitted.
  • Debug information is mostly handled automatically, as there is no unnecessary IL rewrite after
  • AvaloniaRuntimeXamlLoader.LoadGroup allows to compile and parse multiple files and test their combination.
  • Known StyleInclude/ResourceInclude bugs are fixed

Checklist

Breaking changes

Comparing to previous master builds (but not preview), there are couple of binary breaking changes.

Also, in the public API some changes were made for the AvaloniaRuntimeXamlLoader. Some properties were moved from RuntimeXamlLoaderConfiguration to the RuntimeXamlLoaderDocument, as they were specific to a single file, while same configuration should be applicable to a group of files.
New Load method has following signature:

static object Load(RuntimeXamlLoaderDocument document, RuntimeXamlLoaderConfiguration? configuration = null);
static IReadOnlyList<object> LoadGroup(IReadOnlyCollection<RuntimeXamlLoaderDocument> documents, RuntimeXamlLoaderConfiguration? configuration = null);

Fixed issues

Fixes #9127

@maxkatz6 maxkatz6 requested review from kekekeks, grokys and a team November 25, 2022 06:20
@avaloniaui-team
Copy link
Contributor

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

Comment on lines +409 to +410
throw new InvalidProgramException("Same XAML file was loaded twice." +
"Make sure there is no x:Class duplicates no files were added to the AvaloniaResource msbuild items group twice.");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kekekeks as I understand, we don't need this hack anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it was required for compatibility with pre-.axaml themes that was achieved via including the same file as EmbeddedResource and AvaloniaResource.

@avaloniaui-team
Copy link
Contributor

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

@@ -74,7 +74,7 @@
ReportImportance="$(AvaloniaXamlReportImportance)"/>
<Exec
Condition="'$(_AvaloniaUseExternalMSBuild)' == 'true'"
Command="dotnet msbuild /nodereuse:false $(MSBuildProjectFile) /t:GenerateAvaloniaResources /p:_AvaloniaForceInternalMSBuild=true /p:Configuration=$(Configuration) /p:TargetFramework=$(TargetFramework) /p:BuildProjectReferences=false"/>
Command="dotnet msbuild /nodereuse:false $(MSBuildProjectFile) /t:GenerateAvaloniaResources /p:_AvaloniaForceInternalMSBuild=true /p:Configuration=$(Configuration) /p:TargetFramework=$(TargetFramework) /p:RuntimeIdentifier=$(RuntimeIdentifier) /p:BuildProjectReferences=false"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing compile errors with if I change xaml project target framework to "net7.0".
Main msbuild task was running with a runtime identifier set (by default in net7.0), while external msbuild task wasn't (no idea why, haven't checked too deep).
As the result, IntermediateOutputPath was different between these two runs, and resource files were missing.

@avaloniaui-team
Copy link
Contributor

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

/// <param name="xaml">The string containing the XAML.</param>
/// <param name="configuration">Xaml loader configuration.</param>
/// <returns>The loaded object.</returns>
public static object Load(string xaml, RuntimeXamlLoaderConfiguration configuration)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it was supposed to be a public API for users who want to load XAML at runtime.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea was to have configuration parameters in a separate class so we can add more of them without breaking compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I added this configuration parameter couple of weeks ago. So no breaking changes here.
But this configuration shouldn't contain any file source related configs, like RootObject, otherwise it won't be reusable in the LoadGroup methods, where there could be multiple root objects.

Copy link
Member

@kekekeks kekekeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly lgtm

node);
}

AssetLoader.RegisterResUriParsers();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should happen somewhere in MSBuild task initialization rather than in a transformer. I'm also not quite sure if that's a good idea to pollute MSBuild process with our global state

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will move. Also I don't use avalonia global state here, don't access AssetLoader APIs. Instead only URI scheme configuration is registered in .NET URI parser. Otherwise it will lower case all the links.

@avaloniaui-team
Copy link
Contributor

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

@maxkatz6 maxkatz6 merged commit fc0aa01 into master Nov 30, 2022
@maxkatz6 maxkatz6 deleted the xaml-group-transfomers branch November 30, 2022 01:52
@avaloniaui-team
Copy link
Contributor

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

@wieslawsoltes
Copy link
Collaborator

@maxkatz6 Testing this and got following error:

App.axaml(7,6): Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/DockFluentTheme.axaml" resource on the "Dock.Avalonia" assembly. Line 7, position 6.

Xaml:

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

@wieslawsoltes
Copy link
Collaborator

wieslawsoltes commented Nov 30, 2022

Are you using reference assemblies? I believe those can't work with compile-time includes since required type doesn't exist in the reference assembly and we aren't patching it yet.

Don't think so, the Dock.Avalonia project contains DockFluentTheme.axaml and is directly referenced by sample app

@kekekeks
Copy link
Member

@maxkatz6 might be some race condition related to this:

// We also copy ref assembly just for case if needed later for testing.
// But do not remove the original one, as MSBuild actually complains about it with multi-thread compiling.

@wieslawsoltes
Copy link
Collaborator

Btw is this now required for theme files based on Styles ?

AvaloniaXamlLoader.Load(this);

@wieslawsoltes
Copy link
Collaborator

wieslawsoltes commented Nov 30, 2022

Another issue (change?) I found:

When I had in xaml file in same assembly:

<ResourceInclude Source="avares://Dock.Avalonia/Controls/DocumentTabStripItem.axaml" />

and changed to

<ResourceInclude Source="/Controls/DocumentTabStripItem.axaml" />

I got error

Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/Controls/DocumentTabStripItem.axaml" resource on the "Dock.Avalonia" assembly. Line 8, position 10.

I needed to add to csproj the following to make it work:

<AvaloniaResource Include="**/*.axaml" />

Edit: but that (adding AvaloniaResource to csproj) caused another error:

Error AVLN:0002 Avalonia: Duplicate x:Class directive, Dock.Avalonia.Themes.DockFluentTheme is already used in /Themes/DockFluentTheme.axaml

@maxkatz6
Copy link
Member Author

maxkatz6 commented Nov 30, 2022

@maxkatz6 might be some race condition related to this:

Seems like so. When I build same project second time - it works just fine.

Btw is this now required for theme files based on Styles ?

@wieslawsoltes
No, if theme class doesn't have a ctor, this line will be injected. But I needed a ctor to initialize these resources, so had to add this line manually.

When I had in xaml file in same assembly: and changed to

Found the problem. Fixing it.

I needed to add to csproj the following to make it work:

I don't think it's needed, otherwise you might end up with duplicate avalonia resources. Next error is a result.

@kekekeks
Copy link
Member

Seems like so. When I build same project second time - it works just fine.

Try changing AfterTargets="AfterCompile" to AfterTargets="CoreCompile".

@kekekeks
Copy link
Member

Changing the output path just before CoreCompile and reverting it afterwards might work too.

@wieslawsoltes
Copy link
Collaborator

Another issue (change?) I found:

When I had in xaml file in same assembly:

<ResourceInclude Source="avares://Dock.Avalonia/Controls/DocumentTabStripItem.axaml" />

and changed to

<ResourceInclude Source="/Controls/DocumentTabStripItem.axaml" />

I got error

Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/Controls/DocumentTabStripItem.axaml" resource on the "Dock.Avalonia" assembly. Line 8, position 10.

I needed to add to csproj the following to make it work:

<AvaloniaResource Include="**/*.axaml" />

Edit: but that (adding AvaloniaResource to csproj) caused another error:

Error AVLN:0002 Avalonia: Duplicate x:Class directive, Dock.Avalonia.Themes.DockFluentTheme is already used in /Themes/DockFluentTheme.axaml

Tested #9571 and it's fixed.

@wieslawsoltes
Copy link
Collaborator

wieslawsoltes commented Nov 30, 2022

@maxkatz6 Testing this and got following error:

App.axaml(7,6): Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/DockFluentTheme.axaml" resource on the "Dock.Avalonia" assembly. Line 7, position 6.

Xaml:

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

@maxkatz6
Tested #9571 and I still get this error.

@wieslawsoltes
Copy link
Collaborator

@maxkatz6 Testing this and got following error:

App.axaml(7,6): Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/DockFluentTheme.axaml" resource on the "Dock.Avalonia" assembly. Line 7, position 6.

Xaml:

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

@maxkatz6 Tested #9571 and I still get this error.

Interesting, when I switch to

<DockFluentTheme />

instead of

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

it works, so still some issue with StyleInclude.

@wieslawsoltes
Copy link
Collaborator

@maxkatz6 Testing this and got following error:

App.axaml(7,6): Error AVLN:0004 Avalonia: Unable to resolve build method "Build:/Themes/DockFluentTheme.axaml" resource on the "Dock.Avalonia" assembly. Line 7, position 6.

Xaml:

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

@maxkatz6 Tested #9571 and I still get this error.

Interesting, when I switch to

<DockFluentTheme />

instead of

<StyleInclude Source="avares://Dock.Avalonia/Themes/DockFluentTheme.axaml" />

it works, so still some issue with StyleInclude.

Fore reference this is generated app code:

// Decompiled with JetBrains decompiler
// Type: AvaloniaDemo.Xaml.App
// Assembly: DemoXaml, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null
// MVID: 710CC489-2D33-44F9-A635-313141D04CE2
// Assembly location: C:\Users\Administrator\Documents\GitHub\Dock\samples\DemoXaml\bin\Debug\net7.0\DemoXaml.dll
// Compiler-generated code is shown

using Avalonia;
using Avalonia.Controls;
using Avalonia.Controls.ApplicationLifetimes;
using Avalonia.Layout;
using Avalonia.Markup.Xaml.Styling;
using Avalonia.Markup.Xaml.XamlIl.Runtime;
using Avalonia.Styling;
using Avalonia.Themes.Fluent;
using CompiledAvaloniaXaml;
using System;
using System.Runtime.InteropServices;

namespace AvaloniaDemo.Xaml
{
  public class App : Application
  {
    private static Action<object> \u0021XamlIlPopulateOverride;

    public override void Initialize()
    {
      App.\u0021XamlIlPopulateTrampoline(this);
    }

    public override void OnFrameworkInitializationCompleted()
    {
      if (this.ApplicationLifetime is IClassicDesktopStyleApplicationLifetime applicationLifetime1)
        applicationLifetime1.MainWindow = (Window) new MainWindow();
      if (this.ApplicationLifetime is ISingleViewApplicationLifetime applicationLifetime2)
        applicationLifetime2.MainView = (Control) new MainView();
      base.OnFrameworkInitializationCompleted();
    }

    public App()
    {
      base.\u002Ector();
    }

    static void \u0021XamlIlPopulate([In] IServiceProvider obj0, [In] App obj1)
    {
      XamlIlContext.Context<App> context = new XamlIlContext.Context<App>(obj0, new object[1]
      {
        (object) \u0021AvaloniaResources.NamespaceInfo\u003A\u002FApp\u002Eaxaml.Singleton
      }, "avares://DemoXaml/App.axaml");
      context.RootObject = obj1;
      context.IntermediateRoot = (object) obj1;
      App app1;
      App app2 = app1 = obj1;
      context.PushParent((object) app2);
      App app3 = app2;
      app3.Name = "Dock Avalonia Demo";
      Styles styles1 = app3.Styles;
      FluentTheme fluentTheme = new FluentTheme();
      fluentTheme.Mode = FluentThemeMode.Light;
      styles1.Add((IStyle) fluentTheme);
      Styles styles2 = app3.Styles;
      StyleInclude styleInclude = new StyleInclude((IServiceProvider) context);
      styleInclude.Source = new Uri("avares://Dock.Avalonia/Themes/DockFluentTheme.axaml", UriKind.RelativeOrAbsolute);
      styles2.Add((IStyle) styleInclude);
      Styles styles3 = app3.Styles;
      Style style = new Style();
      style.Selector = ((Selector) null).OfType(typeof (TextBlock));
      Setter setter1 = new Setter();
      setter1.Property = (AvaloniaProperty) Layoutable.HorizontalAlignmentProperty;
      setter1.Value = (object) HorizontalAlignment.Center;
      style.Add((ISetter) setter1);
      Setter setter2 = new Setter();
      setter2.Property = (AvaloniaProperty) Layoutable.VerticalAlignmentProperty;
      setter2.Value = (object) VerticalAlignment.Center;
      style.Add((ISetter) setter2);
      styles3.Add((IStyle) style);
      context.PopParent();
      if (app1 is StyledElement styled)
        NameScope.SetNameScope(styled, context.AvaloniaNameScope);
      context.AvaloniaNameScope.Complete();
    }

    private static void \u0021XamlIlPopulateTrampoline([In] App obj0)
    {
      if (App.\u0021XamlIlPopulateOverride != null)
        App.\u0021XamlIlPopulateOverride((object) obj0);
      else
        App.\u0021XamlIlPopulate(XamlIlRuntimeHelpers.CreateRootServiceProviderV2(), obj0);
    }
  }
}

@maxkatz6
Copy link
Member Author

@wieslawsoltes

Fore reference this is generated app code:

Problem was with ref assembly, in which "!XamlIlPopulate" was not generated and was not copied from the non-ref assembly.
This method was used only for validation, that this class is a XAML class.
Removed this validation, and replaced with type check (i.e., type should implement IResourceDictionary or IStyle depending on Loaded property type).

@wieslawsoltes
Copy link
Collaborator

@wieslawsoltes

Fore reference this is generated app code:

Problem was with ref assembly, in which "!XamlIlPopulate" was not generated and was not copied from the non-ref assembly. This method was used only for validation, that this class is a XAML class. Removed this validation, and replaced with type check (i.e., type should implement IResourceDictionary or IStyle depending on Loaded property type).

#9571 fixed all issues I had, amazing work @maxkatz6

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

Successfully merging this pull request may close these issues.

Link trimming doesn't appear to work with Avalonia 11 (11.0.999-cibuild0026354-beta)
5 participants