Skip to content

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

Merged
merged 5 commits into from
May 18, 2025
Merged

Conversation

RubyNova
Copy link
Member

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)

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.

@RubyNova RubyNova requested a review from FiniteReality as a code owner May 18, 2025 16:30
Copy link

github-actions bot commented May 18, 2025

Test Results

   10 files  ±0     10 suites  ±0   5m 58s ⏱️ +27s
  661 tests ±0    661 ✅ ±0  0 💤 ±0  0 ❌ ±0 
6 610 runs  ±0  6 610 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a1152e4. ± Comparison against base commit d807122.

♻️ This comment has been updated with latest results.

@RubyNova RubyNova added feature A feature ticket for the NovelRT. engine core Tickets pertaining to the engine core codebase. engine api Tickets pertaining to NovelRT's end-user API. labels May 18, 2025
Copy link
Member

@capnkenny capnkenny left a 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;
};
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: EOL

Copy link
Member

@capnkenny capnkenny left a 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 :)

@RubyNova RubyNova merged commit 339db9b into main May 18, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine api Tickets pertaining to NovelRT's end-user API. engine core Tickets pertaining to the engine core codebase. feature A feature ticket for the NovelRT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants