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

Add support for multi-thread and/or SIMD WebAssembly #2620

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
@@ -1,19 +1,34 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<!-- if ShouldIncludeNativeHarfBuzzSharp == False then don't include the native libHarfBuzzSharp -->
<PropertyGroup>
<ShouldIncludeNativeHarfBuzzSharp Condition=" '$(ShouldIncludeNativeHarfBuzzSharp)' == '' ">True</ShouldIncludeNativeHarfBuzzSharp>
</PropertyGroup>

<!-- If this is an UNO Platform app -->
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly'">
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly' and '$(ShouldIncludeNativeHarfBuzzSharp)' == 'True'">
<Content Include="@(HarfBuzzSharpStaticLibrary)" Visible="false" />
</ItemGroup>

<!-- If this is a ASP.NET Blazor web assembly app -->
<PropertyGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true'">
<PropertyGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(ShouldIncludeNativeHarfBuzzSharp)' == 'True'">
<WasmBuildNative Condition="'$(WasmBuildNative)' == ''">true</WasmBuildNative>
<!-- Pick the threading type: 'mt' for multi-threading or 'st' for single-threading -->
<_HarfBuzzSharpNativeBinaryType Condition="'$(WasmEnableThreads)' == 'True'">mt</_HarfBuzzSharpNativeBinaryType>
<_HarfBuzzSharpNativeBinaryType Condition="'$(WasmEnableThreads)' != 'True'">st</_HarfBuzzSharpNativeBinaryType>
<!-- Pick the SIMD support: 'simd' for supported or '' for not-supported -->
<_HarfBuzzSharpNativeBinaryType Condition="'$(WasmEnableSIMD)' == 'True'">$(_HarfBuzzSharpNativeBinaryType),simd</_HarfBuzzSharpNativeBinaryType>
<_HarfBuzzSharpNativeBinaryType Condition="'$(WasmEnableSIMD)' != 'True'">$(_HarfBuzzSharpNativeBinaryType)</_HarfBuzzSharpNativeBinaryType>
</PropertyGroup>
<ItemGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(TargetFrameworkVersion)' != ''">
<ItemGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(TargetFrameworkVersion)' != '' and '$(ShouldIncludeNativeHarfBuzzSharp)' == 'True'">
<!-- net6.0 (only has st and non-simd) -->
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\2.0.23\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '6.0'))" />
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.12\st\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '7.0'))" />
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.34\st\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '8.0'))" />
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.34\st\*.a" Condition="$([MSBuild]::VersionGreaterThan($(TargetFrameworkVersion), '8.0'))" />
<!-- net7.0 -->
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.12\$(_HarfBuzzSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '7.0'))" />
<!-- net8.0 -->
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.34\$(_HarfBuzzSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '8.0'))" />
<!-- net9.0+ -->
<NativeFileReference Include="$(HarfBuzzSharpStaticLibraryPath)\3.1.34\$(_HarfBuzzSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionGreaterThan($(TargetFrameworkVersion), '8.0'))" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<!-- if ShouldIncludeNativeSkiaSharp == False then don't include the native libSkiaSharp -->
<PropertyGroup>
<ShouldIncludeNativeSkiaSharp Condition=" '$(ShouldIncludeNativeSkiaSharp)' == '' ">True</ShouldIncludeNativeSkiaSharp>
</PropertyGroup>

<!-- If this is an UNO Platform app -->
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly'">
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly' and '$(ShouldIncludeNativeSkiaSharp)' == 'True'">
<Content Include="@(SkiaSharpStaticLibrary)" Visible="false" />
</ItemGroup>
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly'">
<ItemGroup Condition="'$(IsUnoHead)' == 'True' and '$(UnoRuntimeIdentifier)' == 'WebAssembly' and '$(ShouldIncludeNativeSkiaSharp)' == 'True'">
<!-- Include the GL symbol when running under net7 (https://github.com/dotnet/runtime/issues/76077) -->
<WasmShellEmccExportedRuntimeMethod Include="GL" />
<!-- Include the emscripten_gl* symbols in net8 -->
Expand All @@ -14,15 +19,25 @@
</ItemGroup>

<!-- If this is a ASP.NET Blazor web assembly app -->
<PropertyGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true'">
<PropertyGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(ShouldIncludeNativeSkiaSharp)' == 'True'">
<WasmBuildNative Condition="'$(WasmBuildNative)' == ''">true</WasmBuildNative>
<EmccExtraLDFlags>$(EmccExtraLDFlags) -s USE_WEBGL2=1</EmccExtraLDFlags>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to me: I see Uno has -s OFFSCREEN_FRAMEBUFFER=1 in the flags, so I am wondering if we need that too for Blazor (I was not able to test before since this feature did not exist yet). However, this probably means it is actually needed so maybe I need to check that as well.

<!-- Pick the threading type: 'mt' for multi-threading or 'st' for single-threading -->
<_SkiaSharpNativeBinaryType Condition="'$(WasmEnableThreads)' == 'True'">mt</_SkiaSharpNativeBinaryType>
<_SkiaSharpNativeBinaryType Condition="'$(WasmEnableThreads)' != 'True'">st</_SkiaSharpNativeBinaryType>
<!-- Pick the SIMD support: 'simd' for supported or '' for not-supported -->
<_SkiaSharpNativeBinaryType Condition="'$(WasmEnableSIMD)' == 'True'">$(_SkiaSharpNativeBinaryType),simd</_SkiaSharpNativeBinaryType>
<_SkiaSharpNativeBinaryType Condition="'$(WasmEnableSIMD)' != 'True'">$(_SkiaSharpNativeBinaryType)</_SkiaSharpNativeBinaryType>
</PropertyGroup>
<ItemGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(TargetFrameworkVersion)' != ''">
<ItemGroup Condition="'$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' and '$(TargetFrameworkVersion)' != '' and '$(ShouldIncludeNativeSkiaSharp)' == 'True'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Since NativeFileReference format is consistent for any Mono WASM project, would it make sense to use another property here?

Imagine something like:

<PropertyGroup>
    <ShouldIncludeNativeSkiaSharp Condition="'$(ShouldIncludeNativeSkiaSharp)' == '' AND '$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true'">true</ShouldIncludeNativeSkiaSharp>
</PropertyGroup>

This way third party, like Avalonia, can set ShouldIncludeNativeSkiaSharp to true.
And it will still support Blazor as a first party by defaulting to UsingMicrosoftNETSdkBlazorWebAssembly value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor note, Native AOT uses <NativeLibrary /> instead of <NativeFileReference/>. But that's likely wouldn't be a concern here until .NET 10 or 11.

Copy link
Contributor Author

@mattleibow mattleibow Sep 30, 2024

Choose a reason for hiding this comment

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

@jeromelaban you folks have a different way of loading native things? You just read the @(Content) and figure it out. Are there some checks that if we add will cause issues?

@maxkatz6 are you just the normal logic and no special checks? So you are suggesting that we set it up such that ShouldIncludeNativeSkiaSharp =True always includes the bits without doing fancy checks, and leave the blazor check for rather the default value of ShouldIncludeNativeSkiaSharp?

And for Uno, this means that it will not be set by default but we use the $(IsUnoHead) value to add the Uno way.

@maxkatz6 is there a Is Avalonia App property that I can check to always include? Or would you rather it be opt in? Or even opt out by setting it to false?

Copy link
Contributor

@jeromelaban jeromelaban Sep 30, 2024

Choose a reason for hiding this comment

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

@jeromelaban you folks have a different way of loading native things? You just read the @(Content) and figure it out. Are there some checks that if we add will cause issues?

Indeed, this predates the support from the runtime, but it also supports automatic selection based on the path's format. Starting from net9, we'll transitively include the NativeFileReference support.

And for Uno, this means that it will not be set by default but we use the $(IsUnoHead) value to add the Uno way.

We could also add support that property, but in general, the fact that '$(OutputType)' == 'exe' may be enough to conditionally include assets. We had that $(IsUnoHead) property in the past because some targets were requiring the head to be a dll, not an exe.

As for the condition suggested by @maxkatz6, I'd adjust it as:

<PropertyGroup>
  <ShouldIncludeNativeSkiaSharp Condition="'$(ShouldIncludeNativeSkiaSharp)' == '' AND ( '$(UsingMicrosoftNETSdkBlazorWebAssembly)' == 'true' OR '$(UsingMicrosoftNETSdkWebAssembly)' == 'true' )">true</ShouldIncludeNativeSkiaSharp>
</PropertyGroup>

to support the non-blazor Wasm SDK.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maxkatz6 is there a Is Avalonia App property that I can check to always include?

No, not really.
For our needs, there is a little of advantage of adding avalonia specific checks into SkiaSharp, since we completely reuse runtime NativeFileReference support. I.e. without any special handling.

The only exception of special handling was NativeAOT, but even then, it's the same build-in runtime feature (just a different runtime), not something Avalonia specific.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mattleibow these changes look good to me.

The only thing, native files inclusion can't be forced, unless user uses correct SDK. As WASM projects can be used with a standard SDK just fine. But this change improves defaults anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you say forced, are you saying I am forcing something here? Or are you saying it as a precaution?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, no.
I mean, some developer might want to enable ShouldIncludeNativeSkiaSharp, even if they don't use WASM SDK. Since it's possible to run Browser apps without WASM SDK. Most of older Avalonia Browser apps don't use WASM SDK for example.

It's not possible to force this property right now. But is possible to manually include these files, so that's not a blocker.

Copy link
Contributor

Choose a reason for hiding this comment

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

By WASM SDK I mean <Project SDK="Microsoft.NET.Sdk.WebAssembly"> specifically. This PR includes a check for this (or Blazor) SDK.

Copy link
Contributor Author

@mattleibow mattleibow Oct 27, 2024

Choose a reason for hiding this comment

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

Oh, I see. The opposite of what I was thinking. You want to be able to force the native. Did you use the same item group name "NativeFileReference"?

<!-- net6.0 (only has st and non-simd) -->
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\2.0.23\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '6.0'))" />
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.12\st\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '7.0'))" />
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.34\st\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '8.0'))" />
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.34\st\*.a" Condition="$([MSBuild]::VersionGreaterThan($(TargetFrameworkVersion), '8.0'))" />
<!-- net7.0 -->
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.12\$(_SkiaSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '7.0'))" />
<!-- net8.0 -->
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.34\$(_SkiaSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionEquals($(TargetFrameworkVersion), '8.0'))" />
<!-- net9.0+ -->
<NativeFileReference Include="$(SkiaSharpStaticLibraryPath)\3.1.34\$(_SkiaSharpNativeBinaryType)\*.a" Condition="$([MSBuild]::VersionGreaterThan($(TargetFrameworkVersion), '8.0'))" />
</ItemGroup>

</Project>
6 changes: 3 additions & 3 deletions scripts/azure-templates-stages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ stages:
- 3.1.12:
displayName: '3.1.12_SIMD'
version: 3.1.12
features: simd
features: st,simd
- 3.1.12:
displayName: '3.1.12_Threading'
version: 3.1.12
Expand All @@ -440,11 +440,11 @@ stages:
- 3.1.34:
displayName: '3.1.34_SIMD'
version: 3.1.34
features: _wasmeh,simd,st
features: _wasmeh,st,simd
- 3.1.34:
displayName: '3.1.34_SIMD_Threading'
version: 3.1.34
features: _wasmeh,simd,mt
features: _wasmeh,mt,simd

- ${{ if ne(parameters.buildPipelineType, 'tests') }}:
- stage: native
Expand Down