Initial pass of adding .NET 6 support for sustaining #586

Perksey · 2021-08-18T20:59:25Z

~~Draft for now as I want to lookover it myself.~~

Adds support for .NET 6 on 2.X for sustaining. Should work exactly as with Xamarin, ~~but yet to be tested. Will strive to do this before we go any futher.~~

Perksey · 2021-08-21T20:50:10Z

Ready for review.

Noteworthy Changes

Introduced an UnmanagedCallersOnly shim to use the correct calling convention for DllImport overrides
- Required due to a refactor in the .NET 6 mono threading state machine
- Because of UnmanagedCallersOnly, fix is only available on .NET 5+. It is available on .NET 6's mobile platforms.
  - It worked on old mono, so should be benign
Multitargeted the SDL windowing packages and the windowing metapackage to .NET 6, with Android, iOS, and Mac Catalyst platforms
- Changed native NuGet packaging accordingly (which, might I add, is now 1000x harder)
Added the Xamarin Android support in Silk.NET.Windowing.Sdl.Android to Silk.NET.Windowing.Sdl on the .NET 6 Android TFM.
- No real changes needed, and Input required no changes either.
Stopped giving SDL control of screen orientation
Revived the SilkTouch name generator

Changelog Entries

Add .NET 6 support for Windowing and Input (Desktop and Android only at this time)
Fix SilkTouch so that on .NET 5+ it uses the correct calling convention for PInvokeOverride
Fix SDL taking control of orientation on Android & Silk.NET providing no way to control it.
Miscellaneous SilkTouch improvements and fixes.

HurricanKai · 2021-08-21T20:58:11Z

Have you verified the name gen works?

Perksey · 2021-08-21T22:33:00Z

Yup, works flawlessly.

Perksey · 2021-08-21T22:40:11Z

Also, I'll fix CI in this PR as well. Currently delayed by a unexpected VS update, so it'll be tomorrow now.

HurricanKai · 2021-08-22T13:42:00Z

.github/workflows/build.yml

+    - name: Install Workloads
+      # TODO: This is slow. Maybe we can make a docker container with this already done?
+      run: dotnet workload install android android-aot ios maccatalyst maui


Do we not want to matrix this and have separate CI for core, new Android, old Android, new IOS and old IOS?

Given we're multitargeting we need all of them in any case. We'd need to do lots of build hacking to get each buildable separately and it also would just massively complicate the workflow.

Also it turns out it's not that slow, it only takes 2 minutes. The runners clearly have very good internet connection.

HurricanKai · 2021-08-22T13:43:32Z

README.md


 There are more advanced build actions you can do too, such as FullBuild, Pack, FullPack, among others which you can view by doing `nuke --plan`.

-Some projects may not build properly without being configured to use Desktop MSBuild (i.e. the MSBuild shipped with Visual Studio). As a result, you will ideally need to be on Windows and have Visual Studio 2019 Community (or greater) installed with .NET Core and Xamarin workloads. If you don't have this (i.e. because you're on Linux or Mac), you can still develop Silk.NET in a limited capacity but you will not be able to work on mobile workloads.
+The older Xamarin projects will not build properly without being configured to use Desktop MSBuild (i.e. the MSBuild shipped with Visual Studio). As a result, you will ideally need to be on Windows and have Visual Studio 2019 Community (v16.10 or greater) installed with .NET Core and Xamarin workloads. If you don't have this (i.e. because you're on Linux or Mac), you won't be able to build the older Xamarin packages.


Is there actually a Rider configuration that allows to build one project with MSBuild v16 and another with v17? .NET 6 needs MSBuild v17

Rider can use desktop MSBuild if you configure it to. Why'd you ask?

HurricanKai · 2021-08-22T13:46:09Z

src/Core/Silk.NET.SilkTouch/INativeContextOverride.cs

 using Microsoft.CodeAnalysis.CSharp.Syntax;

 namespace Silk.NET.SilkTouch
 {
    public interface INativeContextOverride
    {
-        TypeDeclarationSyntax Type(string name, string lib, EntryPoint[] entrypoints);
+        TypeDeclarationSyntax Type(string name, string lib, EntryPoint[] entrypoints, Compilation comp);


That doesn't seem right

Changed to take a new context structure.

HurricanKai · 2021-08-22T13:48:24Z

src/Core/Silk.NET.SilkTouch/NativeContextOverrides/PInvokeNativeContextOverride.cs


 namespace Silk.NET.SilkTouch.NativeContextOverrides
 {
    public sealed class PInvokeNativeContextOverride : INativeContextOverride
    {
        /// <inheritdoc />
-        public TypeDeclarationSyntax Type(string name, string lib, EntryPoint[] entrypoints)
+        public TypeDeclarationSyntax Type(string name, string lib, EntryPoint[] entrypoints, Compilation comp)
        {


if we only pass the compilation to get this bool, why not just pass the bool?

Changed to use a new context structure which does not have a compilation within it.

HurricanKai · 2021-08-22T13:49:20Z

src/Lab/Experiments/TriangleNET6/AndroidManifest.xml

+               android:roundIcon="@mipmap/ic_launcher_round"
+               android:supportsRtl="true">
+  </application>
+  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />


Default.

Suggested change

<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />

HurricanKai · 2021-08-22T13:55:34Z

src/Windowing/Silk.NET.Windowing/Silk.NET.Windowing.csproj

+    <ItemGroup Condition="'$(TargetFramework)' != 'net6.0-android' And '$(TargetFramework)' != 'net6.0-ios' And '$(TargetFramework)' != 'net6.0-maccatalyst' And '$(TargetFramework)' != 'net6.0'">
        <ProjectReference Include="..\Silk.NET.Windowing.Glfw\Silk.NET.Windowing.Glfw.csproj" />
        <ProjectReference Include="..\Silk.NET.Windowing.Sdl\Silk.NET.Windowing.Sdl.csproj" />
    </ItemGroup>

+    <ItemGroup Condition="'$(TargetFramework)' == 'net6.0-android' Or '$(TargetFramework)' == 'net6.0-ios' Or '$(TargetFramework)' == 'net6.0-maccatalyst'">
+        <ProjectReference Include="..\Silk.NET.Windowing.Sdl\Silk.NET.Windowing.Sdl.csproj" />
+    </ItemGroup>
+
+    <ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
+        <ProjectReference Include="..\Silk.NET.Windowing.Glfw\Silk.NET.Windowing.Glfw.csproj" />
+    </ItemGroup>


I'm sorry, what? Why? Why not just include both GLFW and SDL everywhere?

Because we have no reason to. GLFW is priority one for desktop and there's zero reason to use SDL over GLFW on desktop - if for whatever reason you do want to use SDL you can go reference it yourself, but I don't think we should mandate that our users ship two extra (very large) DLLs (one for backend, and one for bindings) for a backend that they just don't need on their platform.

The SDL backend was only written to get us to platforms where GLFW doesn't work, so let's only use it on those platforms?

Perksey · 2021-08-27T14:51:47Z

This is ready for another review.

HurricanKai

Reviewed on my phone, so it's possible I missed something. Looks good.

HurricanKai · 2021-08-28T08:26:26Z

src/Core/Silk.NET.SilkTouch/Extensions.cs

+        public static FunctionPointerTypeSyntax GetFuncPtrType
+        (
+            this IEnumerable<FunctionPointerParameterSyntax> parameters,
+            CallingConvention? unmanagedCallingConvention


Should be non-null

Null is treated as managed i.e. there is no unmanaged calling convention.

HurricanKai · 2021-08-28T08:26:41Z

src/Core/Silk.NET.SilkTouch/Extensions.cs

+        public static FunctionPointerTypeSyntax GetFuncPtrType
+        (
+            this IEnumerable<ITypeSymbol> loadTypes,
+            CallingConvention? unmanagedCallingConvention


Should be non-null

Same comment as above.

* Initial pass of adding .NET 6 support for sustaining * Get packaging working properly (afiak) * lol * SilkTouch: Stop making managed conv assumptions (.NET 5+), short names * Triangle on .NET 6 on Android now works * Fix old android lab app for fullscreen * Fix bad ifdef * CI fix? * Review fixes

Initial pass of adding .NET 6 support for sustaining

9f3091c

Perksey added enhancement New feature or request area-Windowing labels Aug 18, 2021

Perksey added this to the 2.X milestone Aug 18, 2021

Perksey self-assigned this Aug 18, 2021

Perksey added this to WIP / Next Monthly Update in 2.X Sustaining via automation Aug 18, 2021

Perksey added 4 commits August 20, 2021 16:56

Get packaging working properly (afiak)

dfbd298

lol

5eb603c

SilkTouch: Stop making managed conv assumptions (.NET 5+), short names

dd507ad

Triangle on .NET 6 on Android now works

207a392

Perksey marked this pull request as ready for review August 21, 2021 20:50

Perksey requested a review from HurricanKai as a code owner August 21, 2021 20:50

Perksey removed their assignment Aug 21, 2021

Fix old android lab app for fullscreen

5672726

Perksey assigned HurricanKai Aug 21, 2021

Fix bad ifdef

f58dbf0

CI fix?

bc38c2b

HurricanKai requested changes Aug 22, 2021

View reviewed changes

Review fixes

ba78646

Perksey requested a review from HurricanKai August 23, 2021 14:04

HurricanKai approved these changes Aug 28, 2021

View reviewed changes

Perksey merged commit 0277641 into main Aug 28, 2021

2.X Sustaining automation moved this from WIP / Next Monthly Update to Confirmed / Next Monthly Update Aug 28, 2021

Perksey deleted the feature/sustaining-net6 branch August 28, 2021 12:49

HurricanKai mentioned this pull request Feb 12, 2023

Getting back on track #1288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial pass of adding .NET 6 support for sustaining #586

Initial pass of adding .NET 6 support for sustaining #586

Perksey commented Aug 18, 2021 •

edited

Loading

Perksey commented Aug 21, 2021 •

edited

Loading

HurricanKai commented Aug 21, 2021

Perksey commented Aug 21, 2021

Perksey commented Aug 21, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021 •

edited

Loading

Perksey Aug 22, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021

HurricanKai Aug 22, 2021

Perksey Aug 22, 2021

Perksey commented Aug 27, 2021

HurricanKai left a comment

HurricanKai Aug 28, 2021

Perksey Aug 28, 2021

HurricanKai Aug 28, 2021

Perksey Aug 28, 2021

Initial pass of adding .NET 6 support for sustaining #586

Initial pass of adding .NET 6 support for sustaining #586

Conversation

Perksey commented Aug 18, 2021 • edited Loading

Perksey commented Aug 21, 2021 • edited Loading

Noteworthy Changes

Changelog Entries

HurricanKai commented Aug 21, 2021

Perksey commented Aug 21, 2021

Perksey commented Aug 21, 2021

Choose a reason for hiding this comment

Perksey Aug 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Perksey commented Aug 27, 2021

HurricanKai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Perksey commented Aug 18, 2021 •

edited

Loading

Perksey commented Aug 21, 2021 •

edited

Loading

Perksey Aug 22, 2021 •

edited

Loading