dx12: Add dynamically sized CPU descriptor heaps #2110
Conversation
msiglreith
changed the title
| } | ||
| } | ||
|
|
||
| // Fixed-size (64) free-list allocator for CPU descriptors. |
There was a problem hiding this comment.
let's have this defined as a constant here
|
|
||
| pub fn alloc_handle(&mut self) -> d3d12::D3D12_CPU_DESCRIPTOR_HANDLE { | ||
| // Find first free slot | ||
| let slot = (0..64) |
There was a problem hiding this comment.
please add TODO to use https://doc.rust-lang.org/std/intrinsics/fn.ctlz.html (or actually do it behind the nightly` feature ;) )
msiglreith
force-pushed
the
cpu_descriptors
branch
from
fca3ccf to
6911688
Compare
| // | ||
| // 0 - Occupied | ||
| // 1 - free | ||
| occupancy: u64, |
There was a problem hiding this comment.
should be renamed into availability to avoid confusion
| } | ||
| } | ||
|
|
||
| pub fn full(&self) -> bool { |
There was a problem hiding this comment.
given that this is not an accessor, I don't think it's idiomatic to omit the verb here. Should be is_full() to mimic is_empty() in standard collections
| assert!(!self.full()); | ||
|
|
||
| // Find first free slot. | ||
| let slot = self.occupancy.trailing_zeros() as usize; |
There was a problem hiding this comment.
super nice! What's the result if we were running it on !0? should we perhaps move the assert down here checking for slot < HEAP_SIZE_FIXED?
There was a problem hiding this comment.
!0 returns 0.
yes, that assert check looks a bit nicer.
|
Greatness here! |
2110: dx12: Add dynamically sized CPU descriptor heaps r=kvark a=msiglreith Dynamic allocator for CPU descriptor heaps. We currently have 2 ways of allocating CPU descriptors: from a device global heap and command local linear allocators. This PR cleans up the two paths a bit and fixes the current size limitation for the first kind by dynamically creating new small fixed size CPU heaps. Freeing currently not support but that's not an regression as we didn't support it before either. Small 'regression' concerning `fill_buffer` which was implemented incorrectly as far as I can see. Probably would work only on AMD. This breaks the fill reftests. Fixes #1915 PR checklist: - [x] tested quad, relevant CTS and a few vulkan samples with the following backends: dx12 Co-authored-by: msiglreith <m.siglreith@gmail.com>
Dynamic allocator for CPU descriptor heaps. We currently have 2 ways of allocating CPU descriptors: from a device global heap and command local linear allocators. This PR cleans up the two paths a bit and fixes the current size limitation for the first kind by dynamically creating new small fixed size CPU heaps.
Freeing currently not support but that's not an regression as we didn't support it before either.
Small 'regression' concerning
fill_bufferwhich was implemented incorrectly as far as I can see. Probably would work only on AMD. This breaks the fill reftests.Fixes #1945
PR checklist: