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

SkXamlCanvas crashes with new prerelease packages #1645

Closed
charlesroddie opened this issue Jun 20, 2024 · 5 comments · Fixed by mono/SkiaSharp#2920
Closed

SkXamlCanvas crashes with new prerelease packages #1645

charlesroddie opened this issue Jun 20, 2024 · 5 comments · Fixed by mono/SkiaSharp#2920
Labels
AOT bug Something isn't working external

Comments

@charlesroddie
Copy link

charlesroddie commented Jun 20, 2024

Putting an SkXamlCanvas in a WinUI app crashes with the new prerelease packages.

We are trying to test NativeAOT in WinUI for our app, which is built in SkiaSharp.
We are using SkXamlCanvas because hardware accelerated support for SkiaSharp views in WinUI aren't released yet.

We get the exception:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
---> System.NotSupportedException: Managed vtable types (ie. containing any reference types) are not supported.
   at WinRT.ObjectReference`1.GetVtable(IntPtr thisPtr)
   at WinRT.ObjectReference`1..ctor(IntPtr thisPtr)
   at WinRT.ObjectReference`1.Attach(IntPtr& thisPtr, Guid iid)
   at WinRT.ObjectReference`1.TryAs(IObjectReference sourceRef, Guid iid, ObjectReference`1& objRef)
   at WinRT.IObjectReference.TryAs[T](Guid iid, ObjectReference`1& objRef)
   at WinRT.IObjectReference.As[T](Guid iid)
   at WinRT.IObjectReference.As[T]()
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Reproduction:

https://github.com/charlesroddie/WinUITemplate/tree/SkXamlCanvas

Is this a CsWinRT issue or a Skiasharp issue?

Version info:

SkiaSharp 2.88.8 and also on 3.0.0-preview.3.1
CsWinRT 2.1.0-prerelease.240602.1
Microsoft.Windows.SDK.BuildTools 10.0.22621.3233
Microsoft.WindowsAppSDK 1.6.240531000-experimental1

Two trim warnings are produced in release mode, but the crash happens in debug (non-aot) and release (nativeaot):

1>ILC : Trim analysis warning IL2075: ABI.Microsoft.UI.Xaml.Data.ManagedCustomPropertyProviderVftbl.Do_Abi_GetCustomProperty_0(IntPtr,IntPtr,IntPtr*): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String,BindingFlags)'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
1>ILC : Trim analysis warning IL2075: ABI.Microsoft.UI.Xaml.Data.ManagedCustomPropertyProviderVftbl.Do_Abi_GetIndexedProperty_1(IntPtr,IntPtr,Type,IntPtr*): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String,BindingFlags,Binder,Type,Type[],ParameterModifier[])'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

@charlesroddie charlesroddie added the bug Something isn't working label Jun 20, 2024
@manodasanW
Copy link
Member

I assume the exception call stack is after a SkiaSharp API call. I haven't tried to repro this yet, but I took a quick look at SkiaSharp. It looks like they have their own implementation of IBufferByteAccess. Based on the exception message, I do believe the exception is coming from there. That code needs to be updated to reflect our new AOT compatible generated code. The main difference is the vftbl being used as the generic for ObjectReference. It should be updated to IUnknownVftbl which is what the exception message is referring to. There are a couple other changes, but you can see CsWinRT's update to its IBufferByteAccess interface implementation for storage APIs here which had the same changes done to make it AOT compatible.

@charlesroddie
Copy link
Author

@mattleibow FYI work needs to be done in SkiaSharp to support WinUI changes.

@mattleibow
Copy link

mattleibow commented Jul 5, 2024

@charlesroddie I fixed the issue in mono/SkiaSharp#2920. You can try out the artifacts here by downloading the nuget artifact:

https://dev.azure.com/xamarin/public/_build/results?buildId=118835&view=artifacts&pathAsName=false&type=publishedArtifacts

Ignore the failure, the tests are a bit flakey...

@manodasanW my fix is to just skip all the manual interop code and do it in C++. I am not sure if you know if this code is going to cause any issues: https://github.com/mono/SkiaSharp/pull/2920/files

intptr_t BufferExtensions::GetByteBuffer(IBuffer const& buffer)
{
    byte* current_data = nullptr;
    auto bufferByteAccess = buffer.as<winrt::impl::IBufferByteAccess>();
    bufferByteAccess->Buffer(&current_data);
    return (intptr_t)current_data;
}

But, one thing that I noticed is that it uses a lot of new code which only exists in the very latest WinRT.Runtime.dll. And this as far as I can tell is a viral dependency meaning that if I use this new version, all things that depend on SkiaSharp will also need this. I tried to copy some of the new code from all over, but several of the types and members are internal and it got crazy fast.

Another thing that I try and do is make sure I use the WinRT.Runtime that the SDK uses so I don't have to force all users of SkiaSharp to update things. @manodasanW when do you think this new version of WinRT.Runtime will be the version used by the .NET SDK? When I say "used" I mean the version included by default here: https://github.com/dotnet/sdk/blob/main/eng/ManualVersions.props

As of right now it is still .31

Since that is still preview and SkiaSharp is stable, I cannot really depend on it for SkiaSharp 2.x. However, the 2.x branch does not have the convenient native library that I can use. I technically can add it to 2.x but that is a lot of work, and that old branch is very iffy to build.

So, at this point @charlesroddie, are there any technical reasons why you are unable to use the 3.0 versions besides the fact it is not stable yet?

@charlesroddie
Copy link
Author

charlesroddie commented Jul 5, 2024

So, at this point @charlesroddie, are there any technical reasons why you are unable to use the 3.0 versions besides the fact it is not stable yet?

We can test on 3.0. We'd just need to stub out Svg.Skia which is waiting for release to be 3.0-compatible.

I think going all-in on 3.0 makes sense.

mattleibow added a commit to mono/SkiaSharp that referenced this issue Jul 18, 2024
@manodasanW
Copy link
Member

We expect to update the .NET SDK with the new AOT compatible version pretty soon aligned with WinAppSDK stable. Given this has been addressed by the other project, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AOT bug Something isn't working external
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants