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 Query support #2256

Merged
merged 12 commits into from
Mar 23, 2023
Merged

Ray Query support #2256

merged 12 commits into from
Mar 23, 2023

Conversation

kvark
Copy link
Member

@kvark kvark commented Feb 17, 2023

Path support:

  • WGSL-in
  • validation
  • SPV-out
  • MSL-out
  • HLSL-out (not planned for this PR)

Note: PR also adds basic infra for specially generated types in IR, which may be handy for #1161

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2023

Codecov Report

Merging #2256 (85383aa) into master (0b87d19) will increase coverage by 0.22%.
The diff coverage is 91.54%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #2256      +/-   ##
==========================================
+ Coverage   81.96%   82.18%   +0.22%     
==========================================
  Files          80       82       +2     
  Lines       43312    44261     +949     
==========================================
+ Hits        35502    36378     +876     
- Misses       7810     7883      +73     
Impacted Files Coverage Δ
src/back/dot/mod.rs 44.15% <0.00%> (-2.02%) ⬇️
src/back/glsl/mod.rs 85.85% <0.00%> (-0.08%) ⬇️
src/back/hlsl/writer.rs 91.06% <0.00%> (-0.06%) ⬇️
src/back/mod.rs 96.96% <0.00%> (-0.99%) ⬇️
src/back/wgsl/writer.rs 91.65% <0.00%> (-0.10%) ⬇️
src/front/glsl/constants.rs 70.22% <0.00%> (-0.11%) ⬇️
src/front/mod.rs 87.69% <ø> (ø)
src/front/wgsl/error.rs 64.50% <0.00%> (-0.69%) ⬇️
src/front/wgsl/mod.rs 73.38% <0.00%> (-0.60%) ⬇️
src/front/wgsl/parse/ast.rs 22.85% <ø> (ø)
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

This is interesting. I don't fully understand the ray tracing API, but I've tried to follow what's going on as best I could. Just a few points to address.

src/back/dot/mod.rs Outdated Show resolved Hide resolved
src/valid/function.rs Outdated Show resolved Hide resolved
src/front/wgsl/lower/mod.rs Outdated Show resolved Hide resolved
src/back/mod.rs Outdated Show resolved Hide resolved
src/valid/interface.rs Show resolved Hide resolved
src/valid/expression.rs Outdated Show resolved Hide resolved
src/front/wgsl/lower/mod.rs Outdated Show resolved Hide resolved
src/back/spv/writer.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@jimblandy
Copy link
Member

@kvark By the way, I went ahead and pushed a commit that expands on the documentation, so be sure to pull before working on the branch. If the docs aren't accurate, feel free to just take the commit out.

@kvark
Copy link
Member Author

kvark commented Mar 22, 2023

@jimblandy thanks for the review! Everything should be addressed now.
I'd also like to hear your thoughts on naming, i.e. ray_desc vs RayDesc.

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

Looks good to me. Give my RayDesc fix a look, and then feel free to merge if it seems okay.

@kvark
Copy link
Member Author

kvark commented Mar 23, 2023

Much nicer than my version, thank you!

@kvark kvark merged commit 53d62b9 into gfx-rs:master Mar 23, 2023
@kvark kvark deleted the ray-query branch March 23, 2023 00:23
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.

3 participants