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

Bring Metal APIs to a common .NET TFM #2788

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Conversation

maxkatz6
Copy link
Contributor

@maxkatz6 maxkatz6 commented Mar 7, 2024

Description of Change

Currently all Metal related APIs are hardcoded to .NET iOS and .NET macOS TFM.
It makes it a bit more difficult to use these APIs, when consuming library either doesn't use these targets or just wants to abstract these calls without an extra TFM.

In Avalonia, for macOS specifically, we don't use "net8.0-macos" target, as all necessary AppKit API we call from our native library. Thus, we don't have typical .NET IMtlDevice and friends, instead we only have pointers.

This caused a problem on our side, but there is no problem that can't be solved with some reflection:https://github.com/AvaloniaUI/Avalonia/blob/master/src/Skia/Avalonia.Skia/Gpu/Metal/SkiaMetalApi.cs

From the API perspective, I think changes in this PR do make sense:

  1. Bringing all metal APIs like CreateContext to common SkiaSharp TFM, but only exposing pointers.
  2. Keep old API for backward compatibility.
  3. Integrate old typed members with new IntPtr members, reducing chance of making a mistake (though, it's relatively low).

API Changes

Metal APIs backported to common TFM, but with IntPtr members.
Note: SkiaSharp.Views package API wasn't changed, as it would be way more involving.

Behavioral Changes

None.

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

Can be considered a backport candidate, but understandably 2.88 might have API freeze at this point.

@mattleibow
Copy link
Contributor

mattleibow commented Mar 28, 2024

Thanks for this PR.

I think both properties are OK to have. In some cases you already have the queue/device and just setting it is easy enough. A bit redundant to have both, but the way you set to null is fine.

If it gets too annoying, I am sure there is a way to get the type instance via the iOS APIs - just the way getting objects from the runtime is doing for everything else. But, I am not sure how useful it is.

@mattleibow mattleibow merged commit 63141bf into mono:main Apr 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants