[Metal] Add support for Xcode15. #19379

mandel-macaque · 2023-11-01T20:25:47Z

This PR brings all the changes from the new Metal APIs. During the review pay special attention to the changes done in the Protocols in order to add tvOS support.

The main problem we have had doing this PR is that tvOS was not done on time before the NET branching, that left us with a lot of memebers that were NOT added in tvOS that are abstract on dotnet, which has left use in a pickle.

Lets use the following code as an example.

Code found before this commit:

	[Mac (11, 0), iOS (14, 0), NoTV]
	[MacCatalyst (14, 0)]
#if NET
	[Abstract]
#endif
	[Export ("accelerationStructureCommandEncoder")]
	IMTLAccelerationStructureCommandEncoder CreateAccelerationStructureCommandEncoder ();

A naive approach would be to add just the tvOS suppor as follows:

	[Mac (11, 0), iOS (14, 0), TV (16,0)]
	[MacCatalyst (14, 0)]
#if NET
	[Abstract]
#endif
	[Export ("accelerationStructureCommandEncoder")]
	IMTLAccelerationStructureCommandEncoder CreateAccelerationStructureCommandEncoder ();

The above change represents and API braking change on the donet tvOS dll because it adds a new Abstrtact members, so this is no an acceptable solution.

There is a second naive approach we can take which is as follows:

	[Mac (11, 0), iOS (14, 0), TV (16,0)]
	[MacCatalyst (14, 0)]
#if NET &!TVOS
	[Abstract]
#endif
	[Export ("accelerationStructureCommandEncoder")]
	IMTLAccelerationStructureCommandEncoder CreateAccelerationStructureCommandEncoder ();

Yet again, the naive approach has an issue with it. In this case, all the extension methods that are generated for tvOS (something the generator writes when methods are not abstract) will be decorated with availability attributes for all the other platforms, which is incorrect and will make developers life worse.

That leaves us with the following approach:

#if NET
#if !TVOS
	[Mac (11, 0), iOS (14, 0), NoTV, MacCatalyst (14, 0)]
	[Abstract]
#else
	[NoMac, NoiOS, TV (16,0), NoMacCatalyst]
#endif
#else
	[Mac (11, 0), iOS (14, 0), TV (16,0), MacCatalyst (14, 0)]
#endif
	[Export ("accelerationStructureCommandEncoder")]
	IMTLAccelerationStructureCommandEncoder CreateAccelerationStructureCommandEncoder ();

With the above change, we do not add an abstract method in tvOS and we only add the tvOS abailabity attribute to the extension methods, and use NoiOS etc for all the other platforms.

The change had to be done to ALL methods that added tvOS support. The good news are that our cecil tests and our introspection tests catch the two naive approaces :)

src/Metal/MTLEnums.cs

github-actions · 2023-11-01T20:30:25Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

haritha-mohan

also was trying to follow your PR description but I think the code examples for the 2 approaches may need to be updated, they seem to be the same 🙂

src/Metal/MTLEnums.cs

haritha-mohan · 2023-11-02T00:21:40Z

src/metal.cs

 #endif
 		[Export ("accelerationStructureCommandEncoder")]
 		IMTLAccelerationStructureCommandEncoder CreateAccelerationStructureCommandEncoder ();
+
+		[Mac (13, 0), iOS (16, 0)]


catalyst and tvOS?

haritha-mohan · 2023-11-02T00:28:59Z

src/metal.cs

@@ -2091,6 +2443,13 @@ partial interface MTLTexture : MTLResource {
 		[return: NullAllowed]
 		[return: Release]
 		IMTLTexture CreateRemoteTexture (IMTLDevice device);
+
+		[Mac (13, 0), iOS (16, 0)]


catalyst and tvOS?

haritha-mohan · 2023-11-02T00:35:02Z

src/metal.cs

@@ -2153,7 +2516,7 @@ partial interface MTLTextureDescriptor : NSCopying {
 		[Export ("allowGPUOptimizedContents")]
 		bool AllowGpuOptimizedContents { get; set; }

-		[NoMac, iOS (15, 0), NoMacCatalyst, NoTV, NoWatch]
+		[Mac (12, 5), iOS (15, 0), NoMacCatalyst, TV (17, 0), NoWatch]


catalyst? https://developer.apple.com/documentation/metal/mtltexturedescriptor/3763055-compressiontype?language=objc

haritha-mohan · 2023-11-02T00:40:26Z

src/metal.cs

+		[Export ("setVertexBuffer:offset:attributeStride:atIndex:")]
+		void SetVertexBuffer ([NullAllowed] IMTLBuffer buffer, nuint offset, nuint stride, nuint index);
+
+		[Mac (14, 0), iOS (17, 0), TV (17, 0), MacCatalyst (17, 0), NoWatch]


nit doesn't make a functional diff but I don't think the no watch attr is necessary here bc the metal framework does not provide support for watchOS

haritha-mohan · 2023-11-02T00:47:14Z

src/metal.cs

@@ -4907,6 +5904,13 @@ interface MTLIndirectCommandBuffer : MTLResource {
 		[MacCatalyst (13, 1)]
 		[Export ("indirectComputeCommandAtIndex:")]
 		IMTLIndirectComputeCommand GetIndirectComputeCommand (nuint commandIndex);
+
+		[Mac (13, 0), iOS (16, 0)]


catalyst and tv?

src/metal.cs

mandel-macaque · 2023-11-02T03:24:18Z

also was trying to follow your PR description but I think the code examples for the 2 approaches may need to be updated, they seem to be the same 🙂

Look like git commit decide that the #if and #else were git comments.. yay!

tests/monotouch-test/Metal/MTLIOCompressionContextTest.cs

rolfbjarne · 2023-11-02T12:01:26Z

src/Metal/Defs.cs

@@ -527,15 +531,13 @@ public struct MTLCoordinate2D {
 	}
 #endif

-#if !TVOS || !NET
-
+#if !TVOS


This doesn't look quite right, seems like it should be:

#if NET [SupportedOSPlatform ("maccatalyst14.0")] [SupportedOSPlatform ("macos11.0")] [SupportedOSPlatform ("ios14.0")] [SupportedOSPlatform ("tvos16.1")] #else [Introduced (PlatformName.MacCatalyst, 14, 0)] [Mac (11, 0)] [iOS (14, 0)] [TV (16, 1)] #endif [StructLayout (LayoutKind.Sequential)] public struct MTLAccelerationStructureSizes { public nuint AccelerationStructureSize; public nuint BuildScratchBufferSize; public nuint RefitScratchBufferSize; } }

src/Metal/MTLEnums.cs

rolfbjarne · 2023-11-02T12:05:07Z

src/Metal/MTLIOCompression.cs

+			=> MTLIOCompressionContextAppendData ((void*) GetCheckedHandle (), data, size);
+
+		public void AppendData (byte [] data)
+			=> AppendData (new ReadOnlySpan<byte> (data, 0, data.Length));


Null check?

src/Metal/MTLIOCompression.cs

src/metal.cs

github-actions · 2023-11-03T16:27:21Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

rolfbjarne · 2023-11-06T09:33:24Z

src/metal.cs

@@ -218,6 +222,13 @@ partial interface MTLBuffer : MTLResource {
 		[return: NullAllowed]
 		[return: Release]
 		IMTLBuffer CreateRemoteBuffer (IMTLDevice device);
+
+		[Mac (13, 0), iOS (16, 0), TV (16, 0), MacCatalyst (16, 0)]
+#if XAMCORE_r54_0


We're far away from vr54...

rolfbjarne · 2023-11-06T09:39:15Z

src/metal.cs

+#if !TVOS
+		[MacCatalyst (14, 0), Mac (11, 0), iOS (13, 0), NoTV]
 		[Abstract] // @required but we can't add abstract members in C# and keep binary compatibility
+#else
+#if XAMCORE_5_0
+		[Abstract]
+#endif
+		[NoMacCatalyst, NoMac, NoiOS, TV (16,0)]
+
+#if XAMCORE_5_0
+		[Abstract]
+#endif
+
+#endif
+
+#else
+		[MacCatalyst (14, 0), Mac (11, 0), iOS (13, 0), TV (16, 0)]


This is not right, we won't end up with the correct availability attributes in XAMCORE_5_0: the Microsoft.tvOS.dll version will say it's the only platform, while the other platforms will say they all exist except tvOS. This is what we'd end up with once we define XAMCORE_5_0:

#if !TVOS [Abstract] [MacCatalyst (14, 0), Mac (11, 0), iOS (13, 0), NoTV] #else [Abstract] [Abstract] [NoMacCatalyst, NoMac, NoiOS, TV (16,0)] #endif

(there will be two [Abstract] attributes for !TVOS as well)

This is simpler and correct I think:

#if XAMCORE_5_0 [Abstract] [MacCatalyst (14, 0), Mac (11, 0), iOS (13, 0), TV (16, 0)] #elif !TVOS [Abstract] [MacCatalyst (14, 0), Mac (11, 0), iOS (13, 0), NoTV] #else [NoMacCatalyst, NoMac, NoiOS, TV (16,0)] #endif

dalexsoto

Looks good to me, @mandel-macaque just please doublecheck on the test results to be good before merging.

vs-mobiletools-engineering-service2 · 2023-12-12T16:00:49Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-12-12T16:01:54Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: f6a59cfb08b2fcad527c83bf3b8f758d3c6c74d1 [PR build]

vs-mobiletools-engineering-service2 · 2023-12-12T16:05:47Z

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-12-12T16:09:02Z

💻 [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-12-12T16:36:06Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
~~watchOS~~ (no change detected)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: f6a59cfb08b2fcad527c83bf3b8f758d3c6c74d1 [PR build]

vs-mobiletools-engineering-service2 · 2023-12-12T18:51:10Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 235 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 41 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: f6a59cfb08b2fcad527c83bf3b8f758d3c6c74d1 [PR build]

mandel-macaque commented Nov 1, 2023

View reviewed changes

src/Metal/MTLEnums.cs Show resolved Hide resolved