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

Ray tracing compaction #6609

Closed
wants to merge 458 commits into from
Closed

Conversation

Vecvec
Copy link
Contributor

@Vecvec Vecvec commented Nov 25, 2024

Connections
Follow up to #6291

This is entirely able to be reviewed.

Description
Adds the function compact_blas as specified in #1040. This needed two more functions in wgpu-hal. These are based on DX12 and the function call EmitRaytracingAccelerationStructurePostbuildInfo(). On Vulkan this call is emulated with query sets. Compaction does not need any additional featues. Depending on the GPU and BLAS contents compaction may not be possible. In that case the BLAS will be duplicated (logic done by GPU driver).

This change is shown by ray_cube_compute, which now compacts the BLAS before using it. This shows the requirements of the function.

Testing
Adds two more tests, testing valid and invalid compaction operations.

Checklist

  • Run cargo fmt.
  • Run taplo format. - done by CI
  • 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 and others added 30 commits April 14, 2024 10:28
 Merge commit 'fb3b33d09233140533a9e431256a8690bf4c5d42' from wgpu/trunk into ray-tracing
…rds do not support (and it now works!) currently missing validation
…cing-updated

# Conflicts:
#	Cargo.lock
#	wgpu-core/src/command/mod.rs
#	wgpu-core/src/device/resource.rs
#	wgpu-core/src/track/mod.rs
#	wgpu-core/src/validation.rs
#	wgpu/src/backend/webgpu.rs
#	wgpu/src/context.rs
…cing-updated

# Conflicts:
#	wgpu-core/src/command/compute.rs
#	wgpu-core/src/track/mod.rs
# Conflicts:
#	.gitattributes
#	examples/src/ray_cube_compute/mod.rs
#	examples/src/ray_cube_fragment/mod.rs
#	examples/src/ray_scene/mod.rs
#	wgpu-core/src/command/compute.rs
#	wgpu-core/src/command/ray_tracing.rs
#	wgpu-core/src/device/life.rs
#	wgpu-core/src/device/queue.rs
#	wgpu-core/src/device/ray_tracing.rs
#	wgpu-core/src/device/resource.rs
#	wgpu-core/src/hub.rs
#	wgpu-core/src/track/mod.rs
#	wgpu/src/backend/webgpu.rs
#	wgpu/src/backend/wgpu_core.rs
@Vecvec

This comment was marked as outdated.

@Vecvec Vecvec marked this pull request as ready for review January 16, 2025 11:46
@cwfitzgerald cwfitzgerald self-assigned this Jan 16, 2025
@JMS55
Copy link
Collaborator

JMS55 commented Jan 25, 2025

Mesa has released the patch in v24.3.4 https://docs.mesa3d.org/relnotes/24.3.4.html

@Vecvec

This comment was marked as resolved.

Vecvec and others added 11 commits January 25, 2025 10:03
# Conflicts:
#	CHANGELOG.md
#	tests/tests/ray_tracing/as_build.rs
#	wgpu-core/src/command/ray_tracing.rs
#	wgpu-core/src/device/ray_tracing.rs
#	wgpu-hal/src/dx12/command.rs
#	wgpu-hal/src/dx12/conv.rs
#	wgpu-hal/src/lib.rs
#	wgpu/src/api/command_encoder.rs
#	wgpu/src/backend/webgpu.rs
#	wgpu/src/backend/wgpu_core.rs
#	wgpu/src/dispatch.rs
# Conflicts:
#	CHANGELOG.md
#	tests/tests/ray_tracing/as_build.rs
#	wgpu-core/src/command/ray_tracing.rs
#	wgpu-core/src/device/ray_tracing.rs
#	wgpu-hal/src/dx12/command.rs
#	wgpu-hal/src/dx12/conv.rs
#	wgpu-hal/src/lib.rs
#	wgpu/src/api/command_encoder.rs
#	wgpu/src/backend/webgpu.rs
#	wgpu/src/backend/wgpu_core.rs
#	wgpu/src/dispatch.rs
…-tracing-compaction

# Conflicts:
#	wgpu-core/src/device/ray_tracing.rs
@JMS55
Copy link
Collaborator

JMS55 commented Feb 10, 2025

Some API questions:

  • I don't love that it blocks until compaction. Ideally I could spawn a bunch of compactions cheaply, and then query for when the compaction size is ready several frames later.
  • With the current API, what resources does it lock? Can I spawn it on a thread/task of some sort and proceed with the rest of my rendering?

The ideal use in my renderer is uploading meshes to BLAS's, using those, and then spawning compaction tasks that will run in the background, and swap the compacted BLAS in and delete the original BLAS once ready.

@Vecvec
Copy link
Contributor Author

Vecvec commented Feb 10, 2025

I don't love that it blocks until compaction.

To be honest I don't either but the one way around it would be a significant change from the API spec (something I don't want to do without feedback): We could go the route of map_async and have them register a callback for when the BLAS is ready (we might even be able to use the internals of it too). I haven't thought this out very well (I only actually had this idea while writing a different version of this response) so it might not work but if you like this idea I could try it.

@JMS55
Copy link
Collaborator

JMS55 commented Feb 10, 2025

I'm not familiar with which part of compaction is expensive. Is it waiting for the readback? The compaction itself (performed by the driver, but in user code?)? Both?

Whatever it is, ideally we have some way to put it on a background thread so we don't block rendering.

For the readback at least, something like map_async probably makes sense. Maybe using actual future's in rust, although maybe wgpu has a good reason they don't do that and use callbacks instead, idk.

Generally I just caution against a blocking, simple API though. I'd rather a more complex API if it comes with a higher performance ceiling.

@Vecvec
Copy link
Contributor Author

Vecvec commented Feb 10, 2025

The readback needs to be internal, which makes the entire API async. Since you seem to have no complaints I'll try and implement it.

@Vecvec
Copy link
Contributor Author

Vecvec commented Feb 10, 2025

I think the API will need to look something like this:

impl Blas {
    // prepares the BLAS for compaction asynchronously, same requirements as map_async: device.poll / 
    // surface.get_current_texture to guarantee callback being called or queue.submit being called to maybe
    // call it.
    fn prepare_for_compaction_async(&self, callback: FnOnce(Result<(), _>)) {}
}

impl CommandEncoder {
    // returns the compacted blas which will be ready once this encoder has been submitted.
    fn compact_blas(&self, blas: &Blas) -> Blas {}
}

This API may take some time to be implemented so I'll close this PR, open one with the hal changes and work on the other changes.

@Vecvec Vecvec closed this Feb 10, 2025
@Vecvec Vecvec mentioned this pull request Feb 10, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants