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

Support DXR in wgpu-hal & naga. #6777

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

Conversation

Vecvec
Copy link
Contributor

@Vecvec Vecvec commented Dec 18, 2024

Connections
Might provide alternatives for #6727?

Description
This adds support for DXR in wgpu-hal. Also removes an unnecessary feature requirement from ray_cube_compute as this was preventing it from running on DX12. Also adds support for HLSL raytracing because naga didn't have that either.

Testing
this only extends feature support to dx12 so all previous rt tests work.

Checklist

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

@Vecvec Vecvec mentioned this pull request Dec 18, 2024
22 tasks
@Elabajaba
Copy link
Contributor

Elabajaba commented Dec 18, 2024

I ran the tests for this and it doesn't appear to be working properly? (6800xt/RDNA2 has RT support, I ran $env:WGPU_BACKEND="dx12" then cargo xtask test -- ray_tracing)

        PASS [   0.076s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_build::unbuilt_blas
        PASS [   0.075s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::scene::acceleration_structure_build_no_index
        PASS [   0.075s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_build::out_of_order_as_build_use
        PASS [   0.076s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_build::out_of_order_as_build
        PASS [   0.089s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_use_after_free::acceleration_structure_use_after_free
        PASS [   0.089s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_build::out_of_order_as_build
        PASS [   0.088s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::scene::acceleration_structure_build_with_index
        PASS [   0.088s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::scene::acceleration_structure_build_no_index
        PASS [   0.088s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_build::unbuilt_blas
        PASS [   0.088s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_build::out_of_order_as_build_use
        PASS [   0.100s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_build::empty_build
        PASS [   0.095s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_build::empty_build
        PASS [   0.100s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_create::blas_invalid_vertex_format
        PASS [   0.095s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/0] wgpu_test::ray_tracing::as_create::blas_mismatched_index
        PASS [   0.101s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::scene::acceleration_structure_build_with_index
        PASS [   0.103s] wgpu-test::wgpu-test [Skipped Failure: ADAPTER] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_use_after_free::acceleration_structure_use_after_free
        PASS [   0.106s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_create::blas_mismatched_index
        PASS [   0.110s] wgpu-test::wgpu-test [Unsupported: Features] [Dx12/AMD Radeon RX 6800 XT/1] wgpu_test::ray_tracing::as_create::blas_invalid_vertex_format

edit: This is probably #6728, I'll try reverting that and see if it works.

edit2: Nope, reverted that and it's now saying [Unsupported: Features] for all of the tests.

edit3: Looks like you need to set the WGPU_DX12_COMPILER environment variable to either dxc or static-dxc for it to work, as it otherwise defaults to FXC. All the ray_tracing tests passed once I set it to either dxc (with the .dlls in my wgpu root dir) or static-dxc.

@cwfitzgerald
Copy link
Member

Looks like you need to set the WGPU_DX12_COMPILER environment variable to either dxc or static-dxc for it to work, as it otherwise defaults to FXC. All the ray_tracing tests passed once I set it to either dxc (with the .dlls in my wgpu root dir) or static-dxc.

This should now default to static-dxc after #6730

@Vecvec
Copy link
Contributor Author

Vecvec commented Dec 18, 2024

All the ray_tracing tests passed once I set it to either dxc (with the .dlls in my wgpu root dir) or static-dxc.

Since that's on a AMD GPU, that means that this probably does provide an alternative (and may also point to the issue being a wgpu bug vs a AMD bug)

Edit: I'm not sure what GPU generations were being affected though.

@cwfitzgerald
Copy link
Member

Same, my 7800xtx passes all tests

@cwfitzgerald
Copy link
Member

Both testing issues fixed in #6781 and #6782

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Super cool to see! Lots of comments, but nothing show stopping!

wgpu-hal/examples/ray-traced-triangle/main.rs Outdated Show resolved Hide resolved
wgpu-hal/src/dx12/adapter.rs Outdated Show resolved Hide resolved
wgpu-hal/src/dx12/command.rs Outdated Show resolved Hide resolved
Comment on lines 1308 to 1340
Triangles: Direct3D12::D3D12_RAYTRACING_GEOMETRY_TRIANGLES_DESC {
Transform3x4: triangle.transform.as_ref().map_or(
0,
|transform| unsafe {
transform.buffer.resource.GetGPUVirtualAddress()
+ transform.offset as u64
},
),
IndexFormat: triangle
.indices
.as_ref()
.map_or(Dxgi::Common::DXGI_FORMAT_UNKNOWN, |indices| {
conv::map_index_format(indices.format)
}),
VertexFormat: conv::map_acceleration_structure_vertex_format(
triangle.vertex_format,
),
IndexCount: triangle
.indices
.as_ref()
.map_or(0, |indices| indices.count),
VertexCount: triangle.vertex_count,
IndexBuffer: triangle.indices.as_ref().map_or(
0,
|indices| unsafe {
indices
.buffer
.expect("needs buffer to build")
.resource
.GetGPUVirtualAddress()
+ indices.offset as u64
},
),
Copy link
Member

Choose a reason for hiding this comment

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

Can we pull all of these complex assignments out of the struct init and make them local variables in the for loop - we can then have a place for computation and a place for initializing the struct. Should help reduce rightward creep too.

Copy link
Member

Choose a reason for hiding this comment

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

This applies broadly for other places in the PR too - give a pass over and see if right shift can be reduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I've addressed this here (and also in the similar version in get_acceleration_structure_build_sizes), but you may have a different opinion. I'll keep this unresolved.

wgpu-hal/src/dx12/command.rs Show resolved Hide resolved
wgpu-hal/src/dx12/conv.rs Outdated Show resolved Hide resolved
wgpu-hal/src/dx12/conv.rs Outdated Show resolved Hide resolved
wgpu-hal/src/dx12/device.rs Outdated Show resolved Hide resolved
wgpu-hal/src/dx12/suballocation.rs Show resolved Hide resolved
wgpu-types/src/counters.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
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.

5 participants