Skip to content

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

Merged
Wumpf merged 6 commits intogfx-rs:trunkfrom
Wumpf:webgpu-dual-source-blending
Mar 8, 2025
Merged

[wgsl-in, wgsl-out, glsl-in] WebGPU compliant dual source blending feature#7146
Wumpf merged 6 commits intogfx-rs:trunkfrom
Wumpf:webgpu-dual-source-blending

Conversation

@Wumpf
Copy link
Copy Markdown
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 Translation to Naga IR 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

This comment was marked as resolved.

@cwfitzgerald

This comment was marked as resolved.

@teoxoy

This comment was marked as resolved.

@cwfitzgerald

This comment was marked as resolved.

@ErichDonGubler
Copy link
Copy Markdown
Member

I've done some significant reviewing today, and I intend to finish on my Monday. Looking mostly good so far, with most of my nits on diagnostics. 🙂

@ErichDonGubler
Copy link
Copy Markdown
Member

Sigh, I haven't been able to finish this review because of some resource conflicts in other parts of my day job and personal life. I do intend to finish this soon, though.

@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler force-pushed the webgpu-dual-source-blending branch from 66bcdf3 to ec47c29 Compare March 6, 2025 02:25
Copy link
Copy Markdown
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, conditional on outstanding conversations being resolved. Since most conversations should be resolvable without code changes here directly, I want to unblock merging.


I used Conventional Comments in this review! I hope they help with clarity and tone. 🙂

@ErichDonGubler ErichDonGubler force-pushed the webgpu-dual-source-blending branch from 6ad7bd8 to 9a0c1e1 Compare March 6, 2025 13:50
@Wumpf Wumpf merged commit fedc80e into gfx-rs:trunk Mar 8, 2025
36 checks passed
@Wumpf Wumpf deleted the webgpu-dual-source-blending branch March 8, 2025 20:07
sharmajai pushed a commit to sharmajai/wgpu that referenced this pull request Oct 12, 2025
…ature (gfx-rs#7146)

Makes the dual source implementation in wgpu WebGPU spec compliant.
Furthermore, makes the dual source blending extension available when targeting WebGPU.
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 Translation to Naga IR 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