-
Notifications
You must be signed in to change notification settings - Fork 44
Add support for push constants. #609
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, just questions and a nit
size_t offset = 0; | ||
size_t size = 0; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: EOL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - all questions answered :)
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This adds support for CmdPushConstants for @capnkenny since its needed by imgui.
Is there an open issue that this resolves? If so, please link it here.
No.
What is the current behavior? (You can also link to an open issue here)
You can't use push constants. 😎
What is the new behavior (if this is a feature change)?
you can use push constants. 😎
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No.
Other information:
I haven't written any unit tests for this since its Vulkan API. I've also not actually tested this works as intended myself, because that would require me writing shaders I can't be bothered with to be honest, but the wiring is simple enough I don't think it matters.