-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[wgsl in and IR] naga ray tracing pipelines #8570
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
base: trunk
Are you sure you want to change the base?
Conversation
|
Very exciting!! I will take a look sometime this week for sure! |
0b9dc42 to
bee83af
Compare
1d0dfca to
ee8883c
Compare
cwfitzgerald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Some small things then lets land and iterate, the tests look nice.
3c5a680 to
fc50128
Compare
|
Just going to note that spv-out is being worked on. Right now built-ins aren't yet supported, as I wanted to first implement traceRay (which is the most different part compared to everything else, the built-ins should just be mapping really). |
|
Lgtm in terms of RT pipelines 1.0/1.1, but we should keep in mind future support for SER hit objects (DX: https://github.com/microsoft/DirectX-Specs/blob/master/d3d/Raytracing.md#hitobject. VK: https://www.khronos.org/blog/boosting-ray-tracing-performance-with-shader-execution-reordering-introducing-vk-ext-ray-tracing-invocation-reorder). The main draw of RT pipelines for me is SER, so we should try and keep the API similar/transparent if we can. |
5bf4a3d to
ec1c25b
Compare
cwfitzgerald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small additional comments, but looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds initial support for WGSL ray tracing pipelines to naga's front-end and IR representation. It introduces new shader stages (ray generation, any hit, closest hit, and miss), address spaces for ray payloads, ray tracing-specific built-ins, and the traceRay function.
Key changes:
- Adds four new shader stages and corresponding validation for ray tracing pipelines
- Introduces
RayPayloadandIncomingRayPayloadaddress spaces for data passing between ray tracing shaders - Implements 13 new built-in types specific to ray tracing (ray invocation ID, world/object space conversions, hit information, etc.)
Reviewed changes
Copilot reviewed 47 out of 100 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wgpu-types/src/binding.rs | CRITICAL BUG: Adds ray tracing shader stage flags but ANY_HIT uses duplicate bit value |
| wgpu-hal/src/metal/mod.rs | Adds unimplemented stubs for ray tracing stages in Metal backend |
| wgpu-hal/src/gles/device.rs | Adds unreachable patterns for ray tracing stages in GLES backend |
| wgpu-hal/src/auxil/mod.rs | Maps naga ray tracing stages to wgpu shader stages |
| wgpu-core/src/validation.rs | Adds ray tracing stages to validation unreachable patterns |
| naga/tests/out/ir/* | Updates IR snapshot tests with new ray_incoming_payload field |
| naga/tests/naga/wgsl_errors.rs | Adds comprehensive tests for ray tracing pipeline validation |
| naga/tests/in/wgsl/ray-tracing-pipeline.wgsl | Adds test shader demonstrating ray tracing pipeline usage |
| naga/src/valid/* | Implements validation for ray tracing stages, payloads, and built-ins |
| naga/src/front/wgsl/parse/* | Parses new ray tracing syntax and attributes |
| naga/src/ir/mod.rs | Defines ray tracing IR structures and enums |
| naga/src/back/* | Adds unreachable/unimplemented patterns for backends not supporting ray tracing |
| docs/api-specs/ray_tracing.md | Documents ray tracing pipeline API |
| CHANGELOG.md | Documents the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0f3800b to
78dfc47
Compare
f78aa07 to
f21a0e2
Compare
f21a0e2 to
6051f56
Compare
|
@Vecvec actually one comment: Can we get a wgsl backend writer for RT pipeline stuff as well? naga_oil requires this in order to compose shaders. |
|
I'll write it in a separate branch and if it's small enough, I'll merge that into this branch (I suspect it will probably be fairly large). It could take some time, as I haven't interacted with wgsl-out much. |
cwfitzgerald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits, but lets do this!
| .enable_extensions | ||
| .contains(ImplementedEnableExtension::WgpuRayTracingPipeline) | ||
| { | ||
| // maybe we want a multi enable extension error? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File an issue?
| let mut compute_like_span = Span::new(0, 0); | ||
| // Span in case we need to report an error for a shader stage missing something (e.g. its workgroup size). | ||
| // Doesn't need to be set in the vertex and fragment stages because they don't have errors like that. | ||
| let mut shader_stage_error_span = Span::new(0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the span be the attribute for the stage - so @compute @ray_gen or whatever?
| pub task_payload: Option<Handle<GlobalVariable>>, | ||
| /// The unique global variable used as an incoming ray payload going into any hit, closest hit and miss shaders. | ||
| /// Unlike the outgoing ray payload, an incoming ray payload must be unique | ||
| pub ray_incoming_payload: Option<Handle<GlobalVariable>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub ray_incoming_payload: Option<Handle<GlobalVariable>>, | |
| pub incoming_ray_payload: Option<Handle<GlobalVariable>>, |
Take or leave
|
|
||
| /// A pointer in the ray_payload or incoming_ray_payload address spaces | ||
| payload: Handle<Expression>, | ||
| // Do we want miss index? What about sbt offset and sbt stride (could be hard to validate)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this in a issue somewhere, could you link it?
| /// Whether any trace rays call doesn't get called with an acceleration structure | ||
| /// with vertex return. This is in case another shader uses a vertex return only | ||
| /// builtin. If inner option is `Some`, then the span is of one of the `traceRay` | ||
| /// calls with a acceleration structure without vertex return. If the inner option | ||
| /// is `None` then the shader only uses acceleration structures with vertex return | ||
| /// in its trace ray calls. If the outer option is `None`, there are no `traceRay` | ||
| /// calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very confusing. Maybe a bespoke enum would be clearer?
Connections
Initial proposal for the wgsl front-end of #6760
Description
Adds wgsl-in and (partial) validation for the simplest version of ray tracing pipelines. This matches the spirv and hlsl specs to the best of my knowledge (though with less than complete validation). Things may change in the future though. @JMS55 may wish to check the naming ect. I don't normally open a naga PR w/o a full way through, but last time I tried pipelines I did that and struggled to keep it up to date.
Naming differences from vulkan
LaunchId->ray_invocation_id(I think its clearer what it is talking about)LaunchSize->num_ray_invocationsInstanceCustomIndex->instance_custom_data(like ray queries)RayTmax->ray_t_current_max(unlikeRayTmin, it changes over the life cycle of the ray, and doesn't match the field of the ray desc)Testing
Snapshot test from wgsl to IR.
Squash or Rebase?
Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests.cargo xtask testto run tests.CHANGELOG.mdentry.