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

[wgsl-in, wgsl-out, glsl-in] WebGPU compliant dual source blending feature #7146

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Feb 14, 2025

Connections

Description

Makes the dual source implementation in wgpu WebGPU spec compliant.
This means changing the wgsl syntax & associated validation to match what's described here and here and here.
Furthermore, makes the dual source blending extension available when targeting WebGPU.

In order to reliably test wgsl output I incorrectly assumed that the way to go would be to add a glsl input to the snapshot tests (mostly to check for the enable directive being written out correctly), didn't realize that there's wgsl->wgsl snapshot testing as well, so I ended up implementing glsl index parsing as collateral😄

Testing
Added a slew of naga tests. Did manual testing in a pet project against native & WebGPU

Draft TODO

  • [help wanted:] need to validate the the right enable directive is set but can't quite figure out how to access this in the right places
  • [help wanted] [partially orthogonal]: While trying this out on my pet project I couldn't get it to fly in WebGPU because I was using naga_oil (with a hack, see also Support wgsl diagnostic directives. bevyengine/naga_oil#113 (comment)) where I have to rely on Module->write_wgsl which right now ignores all directives 😱 . Fixing that isn't entirely straight forward since Module is language agnostic so I have to detect the need for those
  • a more vertical wgpu test checking that happy path works and that shaders needing wgsl require the device to have the correct feature set

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@Wumpf Wumpf force-pushed the webgpu-dual-source-blending branch 3 times, most recently from d101739 to ccbb722 Compare February 15, 2025 13:30
@Wumpf Wumpf changed the title WebGPU compliant dual source blending feature [wgsl-in, wgsl-out, glsl-in] WebGPU compliant dual source blending feature Feb 15, 2025
@Wumpf Wumpf marked this pull request as ready for review February 15, 2025 21:04
@Wumpf Wumpf requested a review from a team as a code owner February 15, 2025 21:04
@ErichDonGubler ErichDonGubler force-pushed the webgpu-dual-source-blending branch 2 times, most recently from b14e3e1 to a15afa0 Compare February 28, 2025 14:25
@ErichDonGubler ErichDonGubler added area: validation Issues related to validation, diagnostics, and error handling area: naga back-end Outputs of naga shader conversion naga Shader Translator area: naga front-end lang: GLSL OpenGL Shading Language area: naga processing Passes over IR in the middle labels Feb 28, 2025
@ErichDonGubler ErichDonGubler force-pushed the webgpu-dual-source-blending branch from a15afa0 to 615acda Compare February 28, 2025 14:54
@ErichDonGubler
Copy link
Member

ErichDonGubler commented Feb 28, 2025

@Wumpf: Rebased and fixed up CI blockers. CI was unhappy because:

  • clippy::unnecessary_map_or started firing for code authored before the CORE_MSRV upgrade in Upgrade CORE_MSRV to 1.82 and take advantage of some new features #7043.
  • The GLSL test added here didn't enable the DUAL_SOURCE_BLENDING capability. I've enabled god_mode on the test to enable that capability, with a note of the intent of its usage.
    • We should iterate on breaking god_mode into more granular pieces. Having something that "enables all the things" sounds undesirable for tests exercising specific capabilities, shader stages, etc. CC @gfx-rs/wgpu on that.
    • We need to move away from opting into the capability at the API level, since reporting of the capability in standard WebGPU happens with ImplementedLanguageExtensions:all and is opted into per shader.

@cwfitzgerald
Copy link
Member

We should iterate on breaking god_mode into more granular pieces. Having something that "enables all the things" sounds undesirable for tests exercising specific capabilities, shader stages

Had the same thought

@teoxoy
Copy link
Member

teoxoy commented Feb 28, 2025

Yeah, I think we should remove god_mode and just ask people to provide all required capabilities.

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Feb 28, 2025

Filed #7252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion area: naga front-end area: naga processing Passes over IR in the middle area: validation Issues related to validation, diagnostics, and error handling lang: GLSL OpenGL Shading Language naga Shader Translator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Converge dual-source blending with the WebGPU standard
4 participants