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

Port some glsl shaders over to wgsl #233

Merged

Conversation

josephreisinger
Copy link
Contributor

@josephreisinger josephreisinger commented Nov 2, 2024

This PR ports over a number of glsl shaders over to wgsl for usage in webgpu projects.

As wgsl does not support function definition overloading with different parameter type signatures, the naming convention roughly follows the practice of including a number after each function name to indicate the major parameter's dimension (e.g. func1 for f32 vars and func3 for vec3f). Note that this may be controversial, e.g. in cases like exposure3.

Formatting is using wgsl-analyzer.

A few important notes:

  1. The lack of preprocessor #defines in wgsl means that we can no longer use define guards to ensure against circular dependencies or re-importing the same header. Combined with wgsl's lack of overloading (noted above) I ended up having to patch webpack-glsl-loader (see fork here: https://github.com/josephreisinger/webpack-glsl-loader). This works for my particular case, but obviously is not helpful for folks who don't use webpack. Not sure what the best approach would be for this more generally / flagging for @patriciogonzalezvivo
  2. All these functions are have been tested in situ using the eyeball metric, but I haven't made any attempts to verify precision or otherwise test numerically.
  3. Generally I have avoided making changes to existing files, except in a few small cases where necessary.

Please let me know if you have any feedback or if there is anything I can do to make this easier to review. Happy to split it up functionally or otherwise if that is better for you?

@patriciogonzalezvivo patriciogonzalezvivo merged commit 098a861 into patriciogonzalezvivo:main Nov 4, 2024
@patriciogonzalezvivo
Copy link
Owner

Thank you @josephreisinger! great contribution! Wellcome permanently to the https://lygia.xyz/license as a contributor : )
I was wonder if you don't mind creating a README_WGSL.md with your translation notes + @sdedovic notes. He also did some translations and arrive to similar solutions. There is also this issue thread about WGSL constrains you will find interesting. Would be great if they can also hear you voice and wishes

@josephreisinger
Copy link
Contributor Author

Awesome! Yep will do. Great to see some motion on the wesl standard also!

@josephreisinger
Copy link
Contributor Author

Also I'm curious have you looked at Naga as a way to potentially automatically convert glsl->wgsl?

@sdedovic
Copy link
Contributor

sdedovic commented Nov 9, 2024

Hey I'm a little late to the party. This looks awesome.

@josephreisinger In my initial attempts with Naga, I found it made the code quite unreadable and decided against it. I figure for something like this library, the ability to read and understand the code was important. I tried to keep variable names consistent between WGSL and the reference implementations in GLSL / HLSL. AFAIK, Naga renames variables. So I opted to do it mostly manually.

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.

3 participants