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 wgpuComputePassEncoderSetPushConstants #437

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

zackgomez
Copy link
Contributor

It was straightforward to implement. I don't expect my changes to the example to be merged, I am looking for guidance on whether they should be reverted, moved to another example, or something else entirely.

Test Plan:
Modified the compute shader to use push constants. Verified the output.

@rajveermalviya
Copy link
Collaborator

Let's keep the compute example as-is, but a separate example for push_constants would be great to have. Also seems like cargo fmt CI is not happy.

@almarklein
Copy link
Collaborator

cc @fyellin since you implemented render push constants in wgpu-py.

@zackgomez
Copy link
Contributor Author

rebased and added examples/push_constants

Also noting that I used my fork of wgpu with these PRs in a project and they all worked as expected

Copy link
Collaborator

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

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

Looks great so far! Added couple of comments.

We could also consider porting over this wgpu test so that we have a reference to track any future wgpu-core API changes and easily update the examples if needed.

examples/push_constants/main.c Outdated Show resolved Hide resolved
examples/push_constants/main.c Show resolved Hide resolved
@rajveermalviya
Copy link
Collaborator

Apart from the example, the API changes LGTM (and tests correctly with current example code). If you need we could merge the API changes in this PR, and do the example in a separate PR.

@fyellin
Copy link
Contributor

fyellin commented Oct 13, 2024

When will this code be available to use in wgpu-py?

@almarklein
Copy link
Collaborator

When will this code be available to use in wgpu-py?

@fyellin when a new version of wgpu-native is released, and wgpu-py updates to that release.

@zackgomez
Copy link
Contributor Author

pls merge

Copy link
Collaborator

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

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

Thanks, Looks good!

(I also added a commit to run the example in CI)

@almarklein almarklein merged commit 52bb912 into gfx-rs:trunk Oct 17, 2024
16 checks passed
@almarklein
Copy link
Collaborator

Thanks @zackgomez !

@zackgomez zackgomez deleted the compute_push_constants branch October 17, 2024 22:10
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.

4 participants