Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ray Tracing Pipeline (KHR) #2564

Merged
merged 30 commits into from
Dec 14, 2024
Merged

Ray Tracing Pipeline (KHR) #2564

merged 30 commits into from
Dec 14, 2024

Conversation

ComfyFluffy
Copy link
Contributor

@ComfyFluffy ComfyFluffy commented Sep 10, 2024

  1. Update documentation to reflect any user-facing changes - in this repository.

  2. Make sure that the changes are covered by unit-tests.

  3. Run cargo clippy on the changes.

  4. Run cargo +nightly fmt on the changes.

  5. Please put changelog entries in the description of this Pull Request
    if knowledge of this change could be valuable to users. No need to put the
    entries to the changelog directly, they will be transferred to the changelog
    file by maintainers right after the Pull Request merge.

Changelog:

### Additions
- Support for the `VK_KHR_ray_tracing_pipeline`.

Changes

  • New struct RayTracingPipeline, which corresponds to the vkPipeline object and construction methods and structs.
  • bind_pipeline_ray_tracing & trace_rays (vkCmdTraceRaysKHR) in the old and new sync.
  • Device::get_ray_tracing_shader_group_handles. Used in building the shader binding table.
  • ShaderGroupHandlesData. Helper for the data from get_ray_tracing_shader_group_handles.
  • ShaderBindingTable. Struct that holds the addresses and underlying buffer of shader binding table, which is used in trace_rays. It contains a constructor that automatically build the SBT from pipeline groups.
  • Examples: triangle-raytracing-auto for the old AutoCommandBuffer, and triangle-raytracing for the new taskgraph sync.

Not implemented (yet) in this PR

  • DeferredOperationKHR. Not planned.
  • RayTracingCaptureReplay. Not planned.
  • RayTracingPipelineStackSize (in dynamic stats). Maybe in this PR?

@ComfyFluffy
Copy link
Contributor Author

Finally a working example with vulkano-taskgraph. The next step is to test with AutoCommandBuffer.

@ComfyFluffy ComfyFluffy marked this pull request as ready for review November 27, 2024 16:01
@ComfyFluffy
Copy link
Contributor Author

I have done the major changes and the PR should be ready for review. I will add some description in PR page and maybe some unit tests soon.

vulkano/src/device/mod.rs Outdated Show resolved Hide resolved
vulkano/src/device/mod.rs Outdated Show resolved Hide resolved
@marc0246
Copy link
Contributor

Please name the examples just "ray-tracing." Most of the examples render a triangle but they do it with more advanced features than just a hello triangle.

@Rua
Copy link
Contributor

Rua commented Dec 10, 2024

This looks good so far, and runs correctly on my machine. I left some comments on individual places that need attention.

In addition, I think there should be some documentation for the ray tracing pipeline module itself. It should explain some of the basics of how ray tracing pipelines work, and a summary of the different shader types and their roles.

@marc0246
Copy link
Contributor

As a sidenote @ComfyFluffy, to help you with development in the future: I saw that you were debugging by adding a debug messenger to some examples. There's no need to do that; you should instead use vkconfig. It can inject validation layers into any existing Vulkan application (including our examples).

@marc0246 marc0246 linked an issue Dec 10, 2024 that may be closed by this pull request
@mirage2032
Copy link

Hi, any idea how long until it gets merged?

@Rua
Copy link
Contributor

Rua commented Dec 12, 2024

When it's done?

@ComfyFluffy
Copy link
Contributor Author

As a sidenote @ComfyFluffy, to help you with development in the future: I saw that you were debugging by adding a debug messenger to some examples. There's no need to do that; you should instead use vkconfig. It can inject validation layers into any existing Vulkan application (including our examples).

That's helpful! Thank you.

@Rua
Copy link
Contributor

Rua commented Dec 13, 2024

Some more public items in pipeline/ray_tracing.rs are still missing documentation.

@ComfyFluffy
Copy link
Contributor Author

A question: how should we deal with ash::vk::StridedDeviceAddressRegionKHR? Should we create a corresponding Vulkano type, or maybe pub use it in src/lib.rs?

- add doc
- add check for stages/groups
- pub modification
- `pub use` StridedDeviceAddressRegionKHR in lib.rs
@Rua
Copy link
Contributor

Rua commented Dec 13, 2024

Vulkano generally defines all its structs itself. That way we can add methods and traits to them, independent of the Ash implementation.

@ComfyFluffy
Copy link
Contributor Author

Vulkano generally defines all its structs itself. That way we can add methods and traits to them, independent of the Ash implementation.

I have added a struct definition for StridedDeviceAddressRegion in lib.rs.

@Rua Rua merged commit 130f9e5 into vulkano-rs:master Dec 14, 2024
6 checks passed
@Rua
Copy link
Contributor

Rua commented Dec 14, 2024

Thank you so much for your PR and your patience in dealing with my nitpicks!

Rua added a commit that referenced this pull request Dec 14, 2024
@marc0246
Copy link
Contributor

Thank you @ComfyFluffy for finally bringing us this long-awaited feature! We all love you for it ❤️

marc0246 added a commit that referenced this pull request Dec 15, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for VK_KHR_ray_tracing_pipeline
5 participants