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

Bindless Descriptor fixes and optimizations #2515

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Firestar99
Copy link
Contributor

@Firestar99 Firestar99 commented Apr 9, 2024

  • added DescriptorSet::invalidate() to make vulkano forget about resources that bound to a descriptor_set, so they can be freed. As (normal) descriptors are validated when bound, this is entirely safe to do.
  • disable validation for individual descriptors if the descriptor set is UPDATE_AFTER_BIND or PARTIALLY_BOUND, making them actually usable
    • I'm not sure these changes are entirely safe. For it to be, we'd need to validate the descriptor set is properly populated just before flushing any commands to the GPU. And I'm not sure if that's done already, or where it should be added. We could ofc also decide to just leave these two flags as unsafe, somehow.
  • some small Descriptor optimizations:
    • More SmallVec use in DescriptorPool to improve variable descriptor count allocation. It only ever allocates one descriptor set at a time, i.e. allocate_infos only ever has one entry in that code path. Annoyed me and just had to fix :D
    • Descriptor updates with no writes or copies return early instead of calling into vulkan. Particularly useful when creating a new descriptor, writing nothing, and updating it properly later.
  1. Update documentation to reflect any user-facing changes - in this repository.

  2. Make sure that the changes are covered by unit-tests.

  3. Run cargo clippy on the changes.

  4. Run cargo +nightly fmt on the changes.

  5. Please put changelog entries in the description of this Pull Request
    if knowledge of this change could be valuable to users. No need to put the
    entries to the changelog directly, they will be transferred to the changelog
    file by maintainers right after the Pull Request merge.

    Please remove any items from the template below that are not applicable.

  6. Describe in common words what is the purpose of this change, related
    Github Issues, and highlight important implementation aspects.

Changelog:

### Additions
* added `DescriptorSet::invalidate()` to make vulkano forget about resources that bound to a descriptor_set, so they can be freed

### Bugs fixed
* fixed descriptor sets with `UPDATE_AFTER_BIND` or `PARTIALLY_BOUND` being wrongly validated on bind

@Firestar99 Firestar99 changed the title more SmallVec use in DescriptorPool small Descriptor optimizations Apr 10, 2024
@Firestar99 Firestar99 marked this pull request as draft April 14, 2024 14:16
@Firestar99
Copy link
Contributor Author

Firestar99 commented Apr 14, 2024

I've turned this PR into a draft, as I've added DescriptorSet::invalidate() to it and haven't gotten around to extensively test it tested it and it works :D

@Rua Rua linked an issue Apr 14, 2024 that may be closed by this pull request
@Firestar99 Firestar99 changed the title small Descriptor optimizations Bindless Descriptor fixes and optimizations May 21, 2024
@Firestar99 Firestar99 marked this pull request as ready for review May 21, 2024 15:48
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.

Allow releasing resources from descriptor sets
1 participant