-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[OSX] Implemented IOSurface/MTLSharedEvent interop APIs #18791
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
base: master
Are you sure you want to change the base?
Conversation
You can test this PR using the following package version. |
//TODO12: Make private and expose IBitmapImpl-based import API for composition visuals | ||
[NotClientImplementable] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create an issue with brief API idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general idea is to have an interface similar to ICompositionGpuInterop
that's can be consumed from the render thread by CustomCompositionVisualHandler (and other API that we may add in the future). Instead of returning an asynchronously imported object the import operation would be completed immediately and will provide in some way a render-thread bitmap object suitable for consumption with ImmediateDrawingContext
|
||
public IGlExportableExternalImageTexture CreateSemaphore(string type) => throw new NotSupportedException(); | ||
|
||
public IGlExternalImageTexture ImportImage(IPlatformHandle handle, PlatformGraphicsExternalImageProperties properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we read these properties (format, width, height) from the surface directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those aren't always readable depending on the platform (it's an implementation of an xplat interface)
ImportCompleted = Compositor.InvokeServerJobAsync(() => | ||
{ | ||
using var _ = handle as IExternalObjectsWrappedGpuHandle; | ||
Import(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is not clear from the code. This handle feels like something to be disposed in the DisposeAsync method rather than here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea of handle wrappers is to have something that extends the lifetime of the object passed to the import operation.
e. g. consider the following scenario:
- a Metal API wrapper (such as MoltenVK) or a 3rd party rendering engine provides an API to access the backing resource (IOSurface/MtlTexture/etc), most notably this API does NOT increase the reference counter of said backing resource and user code has no means of manipulating such reference counter
- Avalonia API is used to import such resource asynchronously
- While operation is still pending the backing resource gets destroyed
So instead of sending the raw resource pointer/handle we instead use a handle wrapper that has internal knowledge of how to manipulate the reference counter, duplicate handle, etc. The wrapper is created immediately on the UI thread when Avalonia import API is called and then disposed on the render thread after the import operation
extern uint64_t GetRetainCountForNSObject(void* obj) | ||
{ | ||
return [(NSObject*)obj retainCount]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetRetainCountForNSObject doesn't seem to be used. And as per apple "This method is of no value in debugging memory management issues" (and in my observations, it's true, returned value often is unreliable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and in my observations, it's true, returned value often is unreliable
It was very much usable while debugging this PR (I needed to observe when the value in fact changes), but we can delete it if you insist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This doesn't build, the |
Added interop APIs for IOSurface/MTLSharedEvent.
MTLSharedEvent behaves differently to regular binary semaphores we've used previously, so new API for dealing with timeline semaphores was added (MoltenVK maps MTLSharedEvent to timeline semaphores and we are using Vulkan terminology anyway).
Public API additions:
Will address complaints about API "breaking changes" later today