Hotfix metal for ios 15 and lower

[taublast]

Description of Change

It has been reported that on lower iOS current metal itialiazation settings suggested by Apple are resulting in a crash, this hotfix solves this for iOS 15 and lower.

Bugs Fixed

• Fixes

[BUG] SKMetalView issues on older hardware (iOS 15)

API Changes

None.

Behavioral Changes

Should act as before .commit c2a1fde for devices with iOS 15 and lower.

Required skia PR

None.

PR Checklist

• Has tests (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Merged related skia PRs
• Changes adhere to coding standard
• Updated documentation

[mattleibow]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[mattleibow]

/rebase

[mattleibow]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[mattleibow]

/Users/runner/work/1/s/source/SkiaSharp.Views.Maui/SkiaSharp.Views.Maui.Core/Platform/Android/SKTouchHandler.cs(120,37): warning CA1416: This call site is reachable on: 'Android' 21.0 and later. 'MotionEventButtonState.StylusSecondary' is only supported on: 'android' 23.0 and later. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1416) [/Users/runner/work/1/s/source/SkiaSharp.Views.Maui/SkiaSharp.Views.Maui.Core/SkiaSharp.Views.Maui.Core.csproj::TargetFramework=net8.0-android34.0]
/Users/runner/work/1/s/source/SkiaSharp.Views/SkiaSharp.Views/Platform/Apple/SKMetalView.cs(96,8): error CS0103: The name 'UIKit' does not exist in the current context [/Users/runner/work/1/s/source/SkiaSharp.Views/SkiaSharp.Views/SkiaSharp.Views.csproj::TargetFramework=net8.0-macos14.0]
98 Warning(s)
1 Error(s)

[Copilot]

Pull Request Overview

This PR applies a hotfix to the Metal initialization on Apple devices, specifically addressing crashes observed on iOS 15 and lower.

• Updated sample count logic in the SKMetalView initialization to conditionally set SampleCount based on the iOS version.
• Segregated the configuration for private and shared DepthStencil modes using a local variable.

Comments suppressed due to low confidence (1)

source/SkiaSharp.Views/SkiaSharp.Views/Platform/Apple/SKMetalView.cs:105

• For devices under iOS 16, the SampleCount now defaults to 1 for the shared mode, whereas the prior configuration set it to 2. Please verify that this intentional change does not adversely affect rendering on older devices.

					DepthStencilStorageMode = MTLStorageMode.Shared;

[mattleibow]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[taublast]

We had SampleCount 1 before fixing metal for simulator, so in context of info from Jeremy0 (#3156 (comment)) it could be safer to return to the state of SampleCount for phisical devices we had previously.
I did not notice any difference on real device (not simulator) with SampleCount 1 or 2, so 1 is fine with me personally to revert to the way it was.

[mattleibow]

@taublast are you saying to revert entirely, or detect if physican and then use 1, else use this version check?

[mattleibow]

If we got some testing to do here, I can merge the revert PR and we can iterate on this?

[taublast]

This pr "reverts for old phisical devices". The "revert pr" would make ios simulator unusable for skglview animated scenarios for people that use default skiasharp handlers.

[taublast]

I'm using this code several times a week on simulator/real mac/real iphone, if this info could help..

[jeremy-visionaid]

@taublast IIRC, the sample you provided for the performance measurements used a redundant intermediate blit (and also used a sample count of 1 on the intermediate surface). Have you fixed those yet? It would be good to rule them out. Might be helpful to get the actual render timings as opposed to the FPS too.

In my use case I did not notice any significant performance difference in the simulator with the defaults (i.e. 1), and I still think that the docs do not recommend this. If I'm reading it correctly, the docs and the sample just avoid using a sample count of 2 - they don't say not to use 1.

I'm happy to go and triple check my measurements, and I have an older MacBook I can test on too, but prob be a bit later in the day as I've got other more urgent bits I need to get to first.

[taublast]

@taublast IIRC, the sample you provided for the performance measurements used a redundant intermediate blit (and also used a sample count of 1 on the intermediate surface). Have you fixed those yet? It would be good to rule them out. Might be helpful to get the actual render timings as opposed to the FPS too.

@jeremy-visionaid
The sample i provided in the first place was the current skiasharp handler, and my custom handlers appeared after the fix was already working, nothing to do with it. To answer direct about the handler, yes it is now better "timed" with more fluid rendering than what we had.

In my use case I did not notice any significant performance difference in the simulator with the defaults (i.e. 1), and I still think that the docs do not recommend this. If I'm reading it correctly, the docs and the sample just avoid using a sample count of 2 - they don't say not to use 1.

Exact quoted examples from apple were followed both for simulator and real device. Simulator settings worked perfectly, on real devices we had an issue with iOS lower than 16 (thanks apple).
This PR sets SampleCount to 1 for real devices as it was already the case previously, default is 1 and our code is not changing this.
The "1" variable is there for an explicit set, to be visible and customizable.
And I think we already had this interesting conversation between me "simulator went from unusable to awesome" and you "did not notice any significant performance difference", will not continue on this.

I'm happy to go and triple check my measurements, and I have an older MacBook I can test on too, but prob be a bit later in the day as I've got other more urgent bits I need to get to first.

Please suite yourself, meanwhile I would not choose to revert the good to avoid the bad, instead of fixing the bad to achieve a better state.

[jeremy-visionaid]

Hey, sorry, I didn't mean for my post to seem so terse - just got a lot of stuff on. I was only pointing out that there were other factors in the sample you provided me that might have been a contributing factor to the performance issues you experienced - I wasn't aware of the timeline of your own handlers to know whether that was important or not. I can only say that original PR didn't seem to affect the performance for my particular usage on my particular hardware - and I completely accept there might be something with either of those factors that make this PR the better one.

With the sample count set to 1 for real devices, I don't see that it's a particularly big deal either way about which PR gets accepted (just technical correctness and simulator will render slightly different results). I'd rather the attention go to higher priority stuff like #3258 since that's blocking for stuff like Avalonia than foresake the great for the good.

[jeremy-visionaid]

OK, I've got some time now to look at this. I recall now that my initial investigation was based off https://github.com/taublast/Maui.Game.SpaceShooter
and
https://github.com/taublast/DrawnUi
in order to eliminate my use case being a factor.

I still had some of the modifications locally, so I've simplified them and pused those to a fork:
https://github.com/jeremy-visionaid/Maui.Game.SpaceShooter/tree/samplecount-test
https://github.com/jeremy-visionaid/DrawnUi/tree/samplecount-test
The timings are obviously very quick and dirty - I have more sophisticated timings in my own app, but I hope they would be sufficient to show if there were a significant performance issue related to the sample count in the simulator.

At the time, there was a bug in dotnet and it wasn't possible to compile a release build for the simulator without setting UseInterpreter:
dotnet/macios#22205
This has now been resolved, so I've disabled UseInterpreter again.

Here are the timings with dotnet sdk 9.0.301:

M2 Pro - iPad A16 18.5 Simulator - Debug
SampleCount = 1: ~5.5 ms
SampleCount = 4: ~5.5 ms

M2 Pro - iPad A16 18.5 Simulator - Release
SampleCount = 1: ~5.5 ms
SampleCount = 4: ~5.5 ms

I also added a 5 ms sleep as a control, which increased the render timings to over 10 ms as expected. The shooter app maintained 60 fps for all the tests.

So, the test method is pretty blunt, please let me know if you disagree with it, but it is at least something that everyone here could look at and spin up as a test pretty quickly to inform a decision here.

[jeremy-visionaid]

Urgh. I forgot to hit reload when I switched to the release build 🤦 It's too late here for me to repeat the timings now since release builds are so very slow.

In the meantime I tested my x86 machine...

MacBook Pro 2019 - iPad A16 18.5 Simulator - Debug
SampleCount = 1: ~15 ms
SampleCount = 4: ~16 ms

So it seems sample count of 1 might be measurably faster there, which is probably what you'd expect, but one off timings are obviously pretty sketchy - it would really need an average to be more certain.