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

Unify all buffer types #2127

Merged
merged 29 commits into from
Jan 12, 2023
Merged

Unify all buffer types #2127

merged 29 commits into from
Jan 12, 2023

Conversation

marc0246
Copy link
Contributor

@marc0246 marc0246 commented Jan 7, 2023

This PR aims to unify the types CpuAccessibleBuffer, DeviceLocalBuffer, BufferSlice and CpuSubbuffer (formerly two types, CpuBufferPoolSubbuffer and CpuBufferPoolChunk) into one: Subbuffer.

What this solves is the need for boxed trait objects, which can be detrimental to performance and might make the API harder to understand for newcommers. Another thing is that with the functionality spread like it is, some might not even be aware of what they can do. For example BufferSlice, was until now not that visible. With everything being in once place, people can discover things more easily. Another issue was that when slicing/indexing a buffer, since all the APIs expect an Arc<dyn BufferAccess>, you have to box every BufferSlice, which includes slices of slices, and would lead to a chain of Arcs as long as the number of slicings. That's a bummer because ideally all that needs to change is an offset and size in the existing one.

Another benefit of having only one type for buffers is type complexity: the BufferAccess, TypedBufferAccess and BufferAccessObject traits are gone and so are all the generics and trait bounds associated with them. Also, BufferView no longer has a generic type parameter and the BufferViewAbstract trait is also gone. I think this makes the API more approachable.

This PR also addresses the long-standing issue of the (then) CpuBufferPool only working for download. The (now) SubbufferAllocator can allocate subbuffers for any purpose.

Fixes #867, fixes #962, fixes #1281, fixes #1733, fixes #2126.

Changelog:

  # Breaking changes
- Changes to CPU buffer allocation:
- - Replaced `CpuBufferPool` with `CpuBufferAllocator`, which is now marked `!Sync` and no longer has a `T` type parameter. The type parameter was moved to the methods, to allow one allocator to allocate as many types of buffers as needed.
+ Changes to buffer allocation:
+ - Replaced `CpuBufferPool` with `SubbufferAllocator`, which is now marked `!Sync` and no longer has a `T` type parameter. The type parameter was moved to the methods, to allow one allocator to allocate as many types of buffers as needed.

- - Merged `CpuBufferPoolChunk` and `CpuBufferPoolSubbuffer` into `CpuSubbuffer`.'
+ Changes to buffers:
+ - Merged `CpuAccessibleBuffer`, `DeviceLocalBuffer`, `BufferSlice`, `CpuBufferPoolChunk`, `CpuBufferPoolSubbuffer` into `Subbuffer`.
+ - Removed `BufferAccess`, `TypedBufferAccess`, `BufferAccessObject`, `BufferInner` and `BufferViewAbstract`.

  # Additions
  - Added `BufferAllocateInfo`.

@marc0246 marc0246 marked this pull request as draft January 7, 2023 21:22
vulkano/src/buffer/mod.rs Outdated Show resolved Hide resolved
@marc0246 marc0246 marked this pull request as ready for review January 11, 2023 21:55
@marc0246
Copy link
Contributor Author

It's ready. I wish the Discord bot would pick up on this.

@Rua Rua merged commit abfde84 into vulkano-rs:master Jan 12, 2023
Rua added a commit that referenced this pull request Jan 12, 2023
@marc0246 marc0246 deleted the buffer-unification branch January 12, 2023 13:08
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
* `CpuBufferPool` revamp

* Fix oopsie

* Fix docs

* Add `Subbuffer`

* Move `Buffer` to `buffer` module

* Replace all buffer types with `Subbuffer`

* Add constraints to `Index` and `QueryResultElement` traits

* Fix oopsies

* Fix examples

* Fix doc tests

* Rename `_unsized` to `_slice`

* Rename `CpuBufferAllocator`

* Implement `DeviceOwned` for `SubbufferAllocator`

* Use better generic type params

* Fix requirement message

* Fix docs

* Fix missing buffer alignment for texel buffer usage

* Add more docs to `Subbuffer`

* Add more utility functions to `Subbuffer`

* Remove superfluous `dst_offset`s

* Murder `FillBufferInfo`

* Fix example

* Add `Subbuffer::range`

* Fix wrong atom size alignment check

* Specify all fields in create infos derived from `BufferAllocateInfo`
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants