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

Implement Binding Arrays #1845

Merged

Conversation

cwfitzgerald
Copy link
Member

Closes #751

Sorry that this PR is larger than I'd like, I couldn't really land any part of it without other parts, so in an attempt to make things easier to review, I have segmented the PR into sections based on the section of code. While the commits are not standalone and this PR needs to be squashed, you can review this pr commit by commit.

This PR implements Binding Arrays and their indexing in the IR, Validator, wgsl-in, and {msl,hlsl,spv}-out.

@cwfitzgerald cwfitzgerald force-pushed the resource-binding-arrays-rebased branch from 1b95694 to d22b70a Compare April 17, 2022 02:30
fn to_global_or_argument(&self) -> Result<GlobalOrArgument, ExpressionError> {
fn to_global_or_argument(
&self,
expression_arena: &Arena<crate::Expression>,
Copy link
Member

Choose a reason for hiding this comment

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

this becomes bad with this argument. If the function input is not one expression but multiple ones, it shouldn't be a method on the Expression any more. Let's turn it to a freestanding function (accepting an arena and a handle)? Or put it on the arena?

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

I looked at the core parts, and it all seems very reasonable.
Amazing (and titanic) work on your part, excited to see this landing!

@teoxoy
Copy link
Member

teoxoy commented Apr 17, 2022

Should we also implement this for the SPIR-V frontend? (related to gfx-rs/wgpu#4463)

@cwfitzgerald
Copy link
Member Author

Ideally both spirv and glsl, but those can be future prs

@cwfitzgerald cwfitzgerald force-pushed the resource-binding-arrays-rebased branch from d22b70a to 825ca93 Compare April 19, 2022 00:16
@cwfitzgerald cwfitzgerald requested a review from kvark April 19, 2022 01:10
@cwfitzgerald
Copy link
Member Author

I'm going to merge this, and if you want a different representation of that function, I'll do a follow up PR

@cwfitzgerald cwfitzgerald merged commit ad28396 into gfx-rs:master Apr 19, 2022
@cwfitzgerald cwfitzgerald deleted the resource-binding-arrays-rebased branch April 19, 2022 18:23
jimblandy added a commit to jimblandy/naga that referenced this pull request Sep 26, 2023
The duplicate entry was introduced accidentally in gfx-rs#1845.
jimblandy added a commit to jimblandy/naga that referenced this pull request Sep 28, 2023
The duplicate entry was introduced accidentally in gfx-rs#1845.
teoxoy pushed a commit that referenced this pull request Sep 28, 2023
The duplicate entry was introduced accidentally in #1845.
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.

Support arrays of descriptors (textures, samplers, buffers).
3 participants