Dynamic buffers #2111
Dynamic buffers #2111
Conversation
There was a problem hiding this comment.
So, to clarify: the length of this extra iterator should be equal to the number of dynamic buffers in the given descriptor sets, right?
In this case, I think the approach makes sense, it's roughly similar to the way we provide clear values and immutable samplers, so it's consistent.
src/hal/src/command/compute.rs
Outdated
| @@ -4,6 +4,7 @@ use std::borrow::Borrow; | |||
|
|
|||
| use {Backend, WorkGroupCount}; | |||
| use buffer::Offset; | |||
| use command::raw::DescriptorSetOffset; | |||
| use queue::capability::{Compute, Supports}; | |||
| use super::{CommandBuffer, RawCommandBuffer, Shot, Level}; | |||
There was a problem hiding this comment.
should probably include the DescriptorSetOffset here
src/hal/src/command/graphics.rs
Outdated
| @@ -5,6 +5,7 @@ use std::ops::Range; | |||
| use Backend; | |||
| use {image, pso}; | |||
| use buffer::IndexBufferView; | |||
| use command::raw::DescriptorSetOffset; | |||
There was a problem hiding this comment.
similarly, let's move that down into use super::{...}
y, that's correct. |
| I: IntoIterator, | ||
| I::Item: Borrow<n::DescriptorSet>, | ||
| J: IntoIterator, | ||
| J::Item: Borrow<com::DescriptorSetOffset>, |
There was a problem hiding this comment.
Actually, I don't think there is much value in going with Borrow<> here, since the offsets are easily copyable. Mind changing it to just IntoIterator<Item = com::DescriptorSetOffset>?
There was a problem hiding this comment.
sure, beside that I'm done with the PR from my side as far as I can see.
There was a problem hiding this comment.
great! i'm also ready with Metal side, just blocked on that IntoIterator change
There was a problem hiding this comment.
hold up, actually not that much a fan of the intoIterator change, makes using slices difficult/impossible as far as I can see.
msiglreith
force-pushed
the
dynamic_offset
branch
from
9b6d27f to
191ce46
Compare
| &mut self, | ||
| layout: &n::PipelineLayout, | ||
| first_set: usize, | ||
| sets: T, | ||
| sets: I, | ||
| offsets: J, |
There was a problem hiding this comment.
Where is offsets used? Or should it be _offsets?
|
Bors r+
Dx12/11 impls to follow separately
… On Jun 5, 2018, at 14:12, msiglreith ***@***.***> wrote:
@msiglreith commented on this pull request.
In src/backend/dx12/src/command.rs:
> ) -> [*mut d3d12::ID3D12DescriptorHeap; 2]
where
- T: IntoIterator,
- T::Item: Borrow<n::DescriptorSet>,
+ I: IntoIterator,
+ I::Item: Borrow<n::DescriptorSet>,
+ J: IntoIterator,
+ J::Item: Borrow<com::DescriptorSetOffset>,
hold up, actually not that much a fan of the intoIterator change, makes using slices difficult/impossible as far as I can see.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
2111: Dynamic buffers r=kvark a=msiglreith Start defining the basic API for dynamic buffer descriptors. Atm based upon vulkan, not optimal but provides the best performance. Other possbiel API design could use `(set, Option<Offset>)` for descriptor set binding, but involves more work on the portability layer and might be more typing for users. Feedback appreciated. More backends to follow .. Addresses #1949 PR checklist: - [x] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [ ] tested examples with the following backends: Co-authored-by: msiglreith <[email protected]> Co-authored-by: Dzmitry Malyshau <[email protected]>
Start defining the basic API for dynamic buffer descriptors. Atm based upon vulkan, not optimal but provides the best performance. Other possbiel API design could use
(set, Option<Offset>)for descriptor set binding, but involves more work on the portability layer and might be more typing for users.Feedback appreciated.
More backends to follow ..
Addresses #1949
PR checklist:
makesucceeds (on *nix)make reftestssucceeds