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

[wgpu-core] Inline RayQuery Support #3631

Closed
wants to merge 187 commits into from

Conversation

daniel-keitel
Copy link
Contributor

@daniel-keitel daniel-keitel commented Apr 1, 2023

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Future Work

  • Acceleration Structure Compaction
  • Ray tracing pipelines
  • Ray tracing resource management

Connections
#1040
Depends on: #3507 (vulkan hal)
Matrix Room Thread

Description
Ray tracing support for wgpu and wgpu-core.
This PR implements support for acceleration structures and ray queries.

It provides a create function each for bottom and top level acceleration structures,
and a build function for the two.
Top level acceleration structures can be used in bind groups.
Shader support is already implemented in naga.

image

TODO

  • more validation
  • as_hal/into_hal
  • tracing
  • more examples
  • more tests
  • documentation
  • specification
  • procedural geometry
  • the safe and validated build function

Testing
Tested with example application (ray-cube) and test.
Needs more tests and examples.

expenses and others added 30 commits September 3, 2022 15:28
…y implementation, run cargo clippy, fix wgpu/wgpu build
(better allocation strategy required)
@JMS55
Copy link
Contributor

JMS55 commented Feb 25, 2024

@Vecvec I talked to the wgpu devs btw. If we want this to be merged, what we need is a bunch more tests. Both for happy paths, and to ensure things can't be misused. Devs will be more willing to review and merge this if they can be confident it's working correctly via tests.

@atlv24
Copy link
Contributor

atlv24 commented Apr 14, 2024

I've fixed merge conflicts here, but the tests fail now: daniel-keitel#3

@Vecvec
Copy link
Contributor

Vecvec commented Apr 15, 2024

Tests fail now though, something about resource tracking stack overruns

I've been working on fixing use after free crashes (and merged trunk while doing so), and I've encountered a similar problem. I'm happy to know that it is not just something wrong with the changes I've made. This seems, as you suggest, not related to this PR.

@atlv24
Copy link
Contributor

atlv24 commented Apr 15, 2024

@Vecvec my fails are now just vulkan validation:

[2024-04-14T23:57:23Z ERROR wgpu_hal::vulkan::instance]
    VALIDATION [SYNC-HAZARD-WRITE-AFTER-WRITE (0x5c0ec5d6)]
    Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ]
    Object 0:
        handle = 0x1fc1e3250e0,
        type = VK_OBJECT_TYPE_QUEUE; 
    | MessageID = 0x5c0ec5d6
    | vkQueueSubmit():  Hazard WRITE_AFTER_WRITE for entry 1,
    VkCommandBuffer 0x1fc2a97b060[],
    Submitted access info (
        submitted_usage: SYNC_COPY_TRANSFER_WRITE,
        command: vkCmdCopyBuffer,
        seq_no: 3,
        reset_no: 1
    ).
    Access info (
        prior_usage: SYNC_COPY_TRANSFER_WRITE,
        write_barriers: SYNC_VERTEX_SHADER_ACCELERATION_STRUCTURE_READ
            | SYNC_FRAGMENT_SHADER_ACCELERATION_STRUCTURE_READ
            | SYNC_COMPUTE_SHADER_ACCELERATION_STRUCTURE_READ
            | SYNC_ACCELERATION_STRUCTURE_BUILD_ACCELERATION_STRUCTURE_READ
            | SYNC_ACCELERATION_STRUCTURE_BUILD_ACCELERATION_STRUCTURE_WRITE,
        queue: VkQueue 0x1fc1e3250e0[],
        submit: 0,
        batch: 0,
        batch_tag: 11,
        command: vkCmdCopyBuffer,
        command_buffer: VkCommandBuffer 0x1fc2a95e730[],
        seq_no: 5,
        reset_no: 1
    ).

[2024-04-14T23:57:23Z ERROR wgpu_hal::vulkan::instance]
    objects: (type: QUEUE, hndl: 0x1fc1e3250e0, name: ?)
    

@Vecvec
Copy link
Contributor

Vecvec commented Apr 15, 2024

That's great! What did you change? Also I think that that validation error is similar to #5373, same submitted usage though different prior usage.

Edit: actually it has different write barriers, same prior usage. I read the wrong thing. Could this actually be the same issue?

@atlv24
Copy link
Contributor

atlv24 commented Apr 15, 2024

@Vecvec all my changes are pushed up, this was my last commit: daniel-keitel@67710d7

We might be missing a barrier somewhere, not sure

@Vecvec
Copy link
Contributor

Vecvec commented Apr 21, 2024

I've found the issue, turns out we need to call transition buffers on the tlas instance too (seems like a preexisting issue in ray-tracing).

@JMS55
Copy link
Contributor

JMS55 commented Apr 24, 2024

@Vecvec glad you solved it! Perhaps you and @atlv24 want to open a new PR with your combined changes? If we add a bunch of tests and smooth out the bugs, we could try and get this merged.

@Vecvec
Copy link
Contributor

Vecvec commented Apr 29, 2024

I've opened atlv24#1 for the write after write fix.

@Vecvec
Copy link
Contributor

Vecvec commented May 26, 2024

I've been working on blas compaction, and it now works! Should I split up the PR for merging (a separate hal PR on wgpu plus an extra PR to this PR) or submit as a single PR? Result is (on my system, this probably changes on different graphics cards) 3840 bytes -> 1152 bytes for ray cube compute example.

@JMS55
Copy link
Contributor

JMS55 commented May 26, 2024

Probably best to to make separate PRs. This PR is already huge, and isn't going to be reviewed until it's cleaned up and has more tests. I don't have the skill to finish it though. The best thing you could do would be to cleanup and finish this PR and work with the wgpu devs to get it reviewed, and then do another PR later for BLAS compaction and other improvements. Entirely up to you though, no pressure for how you want to spend your time :)

@Vecvec
Copy link
Contributor

Vecvec commented May 26, 2024

On another note I've been updating my validation layer, and I noticed a new validation error saying that my graphics card does not support f32x4 as a acceleration struct format, so I've had a look at the format features, something that needs validation in wgpu-core. The formats that have a high enough percentage to allow main-stream ray-tracing (>27%) are f32x2 and f32x3 (I've removed all f16s as rust does not have fp16, and nor does wgpu). I'm not sure if f32x2 is worth it as we would still lose some people still, and I'm not sure 2d ray-tracing is very useful. What we could do is require it to be f32x3 saying that this is required to be f32x3 but there may be features in the future that allow other vertex formats. Limiting the formats we use could increase testing coverage too

@Vecvec Vecvec mentioned this pull request Jun 16, 2024
3 tasks
@Vecvec Vecvec mentioned this pull request Aug 15, 2024
3 tasks
@Vecvec Vecvec mentioned this pull request Sep 14, 2024
5 tasks
@Vecvec
Copy link
Contributor

Vecvec commented Sep 16, 2024

The repo this PR is based on has not been updated in quite some time (since January 2024, 9 months ago), would it be sensible for me or someone else to create a new PR from one of the repositories that has been merged with trunk? I have an added example and test-case that should bring us closer to being merged. Alternatively, if anyone knows when @daniel-keitel might be back I would be happy to wait for his return.

@JMS55
Copy link
Contributor

JMS55 commented Sep 16, 2024

I would make a new PR.

@Vecvec Vecvec mentioned this pull request Sep 18, 2024
6 tasks
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.

8 participants