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

WebGPU bindings #1114

Merged
merged 83 commits into from
Nov 9, 2022
Merged

WebGPU bindings #1114

merged 83 commits into from
Nov 9, 2022

Conversation

Beyley
Copy link
Contributor

@Beyley Beyley commented Oct 29, 2022

Summary of the PR

Adds WebGPU bindings, generated from webgpu-headers

Related issues, Discord discussions, or proposals

Closes #1113

@Beyley Beyley requested a review from a team as a code owner October 29, 2022 04:56
@Beyley Beyley marked this pull request as draft October 29, 2022 04:57
@Perksey
Copy link
Member

Perksey commented Oct 29, 2022

Nice! However I think when we discussed this last we decided that work should be done in both BuildTools and SilkTouch to use WebIDL and allow our WebGPU (and eventually WebGL) bindings to work in both Blazor mode (i.e. using Blazor's interop APIs) and native mode (using classical SilkTouch).

We came to the conclusion that it wasn't this simple because the "web" in WebGPU becomes a misnomer if we don't have an explicit code path for it.

@Beyley
Copy link
Contributor Author

Beyley commented Oct 29, 2022

Nice! However I think when we discussed this last we decided that work should be done in both BuildTools and SilkTouch to use WebIDL and allow our WebGPU (and eventually WebGL) bindings to work in both Blazor mode (i.e. using Blazor's interop APIs) and native mode (using classical SilkTouch).

We came to the conclusion that it wasn't this simple because the "web" in WebGPU becomes a misnomer if we don't have an explicit code path for it.

I was thinking more emscripten than blazor webidl, since emscripten has code to convert these C imports, i just need to specify a dllimport with the right name

@Perksey
Copy link
Member

Perksey commented Oct 29, 2022

I have been against relying on emscripten magic, but looking at other .NET projects this seems to frustratingly be the standard route so that sounds fine with me.

You'll need to use SilkTouch's P/Invoke override functionality as well as add support for Blazor (OperatingSystem.IsBrowser) in the SearchPathContainer logic.

@Beyley
Copy link
Contributor Author

Beyley commented Oct 29, 2022

I have been against relying on emscripten magic, but looking at other .NET projects this seems to frustratingly be the standard route so that sounds fine with me.

You'll need to use SilkTouch's P/Invoke override functionality as well as add support for Blazor (OperatingSystem.IsBrowser) in the SearchPathContainer logic.

Yup the P/Invoke and SearchPathContainer will come in another PR later on, i already have a lot of that done, for the initial merge ittl be a 'wgpu-native' thing only, then i'll make all the bindings i can web-compatible in the main WASM PR
and yeah while relying on emscripten fairy dust isn't the best, it let's users keep a lot of their code identical, and not have to directly write for WebGL/WebGPU(but the slightly different web api)

@Beyley
Copy link
Contributor Author

Beyley commented Oct 29, 2022

If CI passes then i think this PR is ready for a first round of review, i went through and made sure all the struct/function names make sense

generator.json Outdated Show resolved Hide resolved
@Beyley Beyley marked this pull request as ready for review October 29, 2022 19:54
@Beyley
Copy link
Contributor Author

Beyley commented Nov 2, 2022

Since initial round of review, some generation problems have cropped up relating to enums being misinterpreted as ints in some structs, namely problems like WriteMask in ColorTargetState, which is incorrectly generated as a uint, rather than a ColorWriteMask, id like to get these fixed before this is taken for another review

@Beyley Beyley requested a review from Perksey November 2, 2022 23:38
@Beyley
Copy link
Contributor Author

Beyley commented Nov 2, 2022

Should be ready for another review (but id like to pause on actually merging until the bindings regeneration is in)

examples/CSharp/WebGPU/WebGPUTexturedQuad/Program.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Bind/ClassWriter.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Bind/ProjectWriter.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Common/Project.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Common/TypeMapper.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Generator.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Generator.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Generator.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Bind/ClassWriter.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Bind/ProjectWriter.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Generator.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Cpp/Clang.cs Outdated Show resolved Hide resolved
src/Core/Silk.NET.BuildTools/Cpp/Clang.cs Outdated Show resolved Hide resolved
@Beyley
Copy link
Contributor Author

Beyley commented Nov 9, 2022

Found problem with BufferMapAsync, its MapMode isnt being registered as an enum, rather a uint, so dont merge until thats fixed

@Beyley
Copy link
Contributor Author

Beyley commented Nov 9, 2022

Found problem with BufferMapAsync, its MapMode isnt being registered as an enum, rather a uint, so dont merge until thats fixed

This has now been fixed!

@Perksey Perksey merged commit 007d160 into main Nov 9, 2022
@Perksey Perksey deleted the webgpu branch November 9, 2022 19:06
@unicomp21
Copy link

Is webgl still on the roadmap?

@Beyley
Copy link
Contributor Author

Beyley commented Feb 28, 2023

Is webgl still on the roadmap?

#740

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.

WebGPU bindings
4 participants