Compile Skia with Direct3D on Windows platform #2823
Conversation
|
These should be exposed by SkiaSharp: https://github.com/Wodsoft/upf/blob/master/src/UniversalPresentationFramework.Platforms.Win32/Win32RendererD3D12Context.cs#L291 |
|
@Gillibald How to update |
|
I think I may have broken the merge with my merge of the Metal PR. Sorry. But, this PR is shaping up very well! |
I think you found the generator in the utils directory. You should be able to just run this one on a Windows machine with VS installed: https://github.com/mono/SkiaSharp/blob/main/utils/generate.ps1 |
|
@mattleibow Yeah, I found it. Many documents are out of date or missing. I spent a lot of date to look up how to modify codes. Pipelines failed with |
|
ah, we have a mirror of all packages for reasons to prevent supply chain attacks. Let me mirror those packs. |
binding/SkiaSharp/GRContext.cs
Outdated
| @@ -63,6 +63,18 @@ public static GRContext CreateVulkan (GRVkBackendContext backendContext, GRConte | |||
| } | |||
| } | |||
|
|
|||
| public static GRContext CreateDirect3D (GRD3dBackendcontext backendContext, GRContextOptions options) | |||
There was a problem hiding this comment.
I hate myself so much... But this may need to be 3d with a lowercase d for consistency. So yuck to look at and I have no idea what I was thinking at the time. Sorry universe.
There was a problem hiding this comment.
I saw that every Direct3D library use uppercase D, so I'm doing same with that.
There was a problem hiding this comment.
Yeah, but sadly if you look at my OpenGl members I must have lost my mind. Everywhere it is uppercase-G-lowercase-l 😭
|
increase 1,773,537 bytes compare to 3.0.0-preview2.1 |
There was a problem hiding this comment.
Very close, I added more comments on the native PR that is more about API shape and maybe an issue with native interop: mono/skia#121 (review)
Also, it would be great to get some sort of test to make sure that at least using these new types are not crashing because of some interop issue.
binding/SkiaSharp/GRContext.cs
Outdated
| @@ -63,6 +63,18 @@ public static GRContext CreateVulkan (GRVkBackendContext backendContext, GRConte | |||
| } | |||
| } | |||
|
|
|||
| public static GRContext CreateDirect3D (GRD3dBackendcontext backendContext, GRContextOptions options) | |||
There was a problem hiding this comment.
Yeah, but sadly if you look at my OpenGl members I must have lost my mind. Everywhere it is uppercase-G-lowercase-l 😭
| @@ -12638,6 +12700,156 @@ internal unsafe partial struct GRContextOptionsNative : IEquatable<GRContextOpti | |||
|
|
|||
| } | |||
|
|
|||
| // gr_d3d_backendcontext_t | |||
| [StructLayout (LayoutKind.Sequential)] | |||
| public unsafe partial struct GRD3dBackendcontext : IEquatable<GRD3dBackendcontext> { | |||
There was a problem hiding this comment.
Note about the names of the 2 new types - the info and context need to start with an uppercase letter. I added a comment on how to do this in the json mapper file here: mono/skia#121 (review)
| { | ||
| return new GRD3dTextureinfo | ||
| { | ||
| Resource = textureInfo.Resource.NativePointer, |
There was a problem hiding this comment.
Is it possible to add some test for this? I see in the C++ API it is a sk_sp<> type, but the interop is a raw pointer that is direct cast from the C pointer to the C++ sp. I have a feeling there may be memory corruption in some cases as there may be a reference count at some point.
I have added some Vulkan tests that may be helpful in what I mean: https://github.com/mono/SkiaSharp/blob/main/tests/VulkanTests/SKSurfaceTest.cs#L27
There was a problem hiding this comment.
I have been test on https://github.com/Wodsoft/upf
You can run UniversalPresentationFramework.Test.Win32 for D3D renderer default.
Testing in SkiaSharp should be after Skia merged?
There was a problem hiding this comment.
Are you able to add a unit test for this repo so we can ensure that no future changes break anything? I am not familiar with the UPF framework, but it looks like this file could be extracted/reworked a bit: https://github.com/Wodsoft/upf/blob/master/src/UniversalPresentationFramework.Platforms.Win32/Win32RendererD3D12Context.cs
I did something like this with Vulkan: https://github.com/mono/SkiaSharp/tree/main/tests/VulkanTests
|
@mattleibow Why android and macos test run failed? |
I think there is a problem with the magic step that is supposed to use the correct build of skia. Just commit the submodule in this PR. Then the checkout of SkiaSharp will also pull the correct skia. |
|
For the tests, I think this will work - still installing tooling as I have a new laptop: 6cf79ea Hopefully it just works, I copied the context setup from the skia test files... seems simple enough, no idea. |
It's fail randomly I think. Sometime android failed and sometime macos failed or android and macos both failed. |
|
ah, yeah sorry. Those are a bit flakey right now. Sometimes the Android emulator does not boot, or the macOS test runner crashes. |
|
Maybe you can click |
|
@Kation are you able to write some tests? |
|
@mattleibow Busy these days. I'm copying Vulkan test to Direct3D. |
|
Hate to be that guy but have any other Direct3D C# binding libraries been considered. This |
|
I'd love to see this one finished off. I'd take it on myself but I'm swamped with other stuff. I don't mind too much about what bindings are used, it's relatively simple to chop and change them. That said, I find that CsWin32 doesn't do too bad a job of DirectX bindings these days. |
|
@mattleibow When we could merge or something need to add? |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
Description of Change
Compile Skia with Direct3D on Windows platform
Bugs Fixed
#2817
API Changes
None.
Behavioral Changes
None.
Required skia PR
mono/skia#121
PR Checklist