nordic-dev.net/vulkano
nordic-dev.net/vulkano
2
0
Fork 0
mirror of https://github.com/vulkano-rs/vulkano.git synced 2025-02-16 09:02:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

update push constant ranges to match the spec. overlapping ranges are OK but intersecting stage flags are not (#1659)

Browse Source 
* update push constant ranges to match the spec. overlapping ranges are OK but intersecting stage flags are not

* add a check for push constant ranges
This commit is contained in:
Will Song 2021-08-09 11:19:54 -04:00 committed by GitHub
parent 7290af6c42
commit 4f4580e749
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 107 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
examples/src/bin/dynamic-buffers.rs
View File

@ -140,7 +140,7 @@ fn main() {
descriptor_set_layouts,
shader
.main_entry_point()
.push_constant_ranges()
.push_constant_range()
.iter()
.cloned(),
)

4
examples/src/bin/runtime-shader/main.rs
View File

@ -221,7 +221,7 @@ fn main() {
vs.graphics_entry_point(
CStr::from_bytes_with_nul_unchecked(b"main\0"),
[], // No descriptor sets.
[], // No push constants.
None, // No push constants.
<()>::descriptors(),
vertex_input,
vertex_output,
@ -233,7 +233,7 @@ fn main() {
fs.graphics_entry_point(
CStr::from_bytes_with_nul_unchecked(b"main\0"),
[], // No descriptor sets.
[], // No push constants.
None, // No push constants.
<()>::descriptors(),
fragment_input,
fragment_output,

12
vulkano-shaders/src/descriptor_sets.rs
View File

@ -28,7 +28,7 @@ pub(super) fn write_descriptor_set_layout_descs(
entrypoint_id: u32,
interface: &[u32],
exact_entrypoint_interface: bool,
stages: TokenStream,
stages: &TokenStream,
) -> TokenStream {
// TODO: somewhat implemented correctly
@ -84,7 +84,7 @@ pub(super) fn write_descriptor_set_layout_descs(
}
}
pub(super) fn write_push_constant_ranges(doc: &Spirv, types_meta: &TypesMeta) -> TokenStream {
pub(super) fn write_push_constant_ranges(doc: &Spirv, stage: &TokenStream, types_meta: &TypesMeta) -> TokenStream {
// TODO: somewhat implemented correctly
// Looping to find all the push constant structs.
@ -106,17 +106,17 @@ pub(super) fn write_push_constant_ranges(doc: &Spirv, types_meta: &TypesMeta) ->
if push_constants_size == 0 {
quote! {
[]
None
}
} else {
quote! {
[
Some(
PipelineLayoutPcRange {
offset: 0, // FIXME: not necessarily true
size: #push_constants_size,
stages: ShaderStages::all(), // FIXME: wrong
stages: #stage,
}
]
)
}
}
}

4
vulkano-shaders/src/entry_point.rs
View File

@ -82,8 +82,8 @@ pub(super) fn write_entry_point(
};
let descriptor_set_layout_descs =
write_descriptor_set_layout_descs(&doc, id, interface, exact_entrypoint_interface, stage);
let push_constant_ranges = write_push_constant_ranges(&doc, &types_meta);
write_descriptor_set_layout_descs(&doc, id, interface, exact_entrypoint_interface, &stage);
let push_constant_ranges = write_push_constant_ranges(&doc, &stage, &types_meta);
let spec_consts_struct = if crate::spec_consts::has_specialization_constants(doc) {
quote! { SpecializationConstants }

8
vulkano/src/pipeline/cache.rs
View File

@ -290,7 +290,7 @@ mod tests {
module.compute_entry_point(
CStr::from_ptr(NAME.as_ptr() as *const _),
[],
[],
None,
<()>::descriptors(),
)
};
@ -335,7 +335,7 @@ mod tests {
first_module.compute_entry_point(
CStr::from_ptr(NAME.as_ptr() as *const _),
[],
[],
None,
<()>::descriptors(),
)
};
@ -376,7 +376,7 @@ mod tests {
second_module.compute_entry_point(
CStr::from_ptr(NAME.as_ptr() as *const _),
[],
[],
None,
<()>::descriptors(),
)
};
@ -430,7 +430,7 @@ mod tests {
module.compute_entry_point(
CStr::from_ptr(NAME.as_ptr() as *const _),
[],
[],
None,
<()>::descriptors(),
)
};

6
vulkano/src/pipeline/compute_pipeline.rs
View File

@ -76,7 +76,7 @@ impl ComputePipeline {
let pipeline_layout = Arc::new(PipelineLayout::new(
device.clone(),
descriptor_set_layouts,
shader.push_constant_ranges().iter().cloned(),
shader.push_constant_range().iter().cloned(),
)?);
ComputePipeline::with_unchecked_pipeline_layout(
device,
@ -110,7 +110,7 @@ impl ComputePipeline {
unsafe {
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
ComputePipeline::with_unchecked_pipeline_layout(
device,
@ -460,7 +460,7 @@ mod tests {
},
readonly: true,
})])],
[],
None,
SpecConsts::descriptors(),
)
};

36
vulkano/src/pipeline/graphics_pipeline/builder.rs
View File

@ -51,6 +51,7 @@ use crate::render_pass::Subpass;
use crate::OomError;
use crate::VulkanObject;
use smallvec::SmallVec;
use std::collections::hash_map::{Entry, HashMap};
use std::mem;
use std::mem::MaybeUninit;
use std::ptr;
@ -183,9 +184,28 @@ where
dynamic_buffers.into_iter().cloned(),
);
let push_constant_ranges = stages.iter().fold(vec![], |total, shader| {
PipelineLayoutPcRange::union_multiple(&total, shader.push_constant_ranges())
});
// We want to union each push constant range into a set of ranges that do not have intersecting stage flags.
// e.g. The range [0, 16) is either made available to Vertex | Fragment or we only make [0, 16) available to
// Vertex and a subrange available to Fragment, like [0, 8)
let mut range_map = HashMap::new();
for stage in stages.iter() {
if let Some(range) = stage.push_constant_range() {
match range_map.entry((range.offset, range.size)) {
Entry::Vacant(entry) => {
entry.insert(range.stages);
},
Entry::Occupied(mut entry) => {
*entry.get_mut() = *entry.get() | range.stages;
},
}
}
}
let push_constant_ranges: Vec<_> = range_map
.iter()
.map(|((offset, size), stages)| {
PipelineLayoutPcRange { offset: *offset, size: *size, stages: *stages }
})
.collect();
(descriptor_set_layout_descs, push_constant_ranges)
};
@ -222,7 +242,7 @@ where
let shader = &self.vertex_shader.as_ref().unwrap().0;
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
}
@ -230,7 +250,7 @@ where
let shader = &geometry_shader.0;
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
}
@ -239,7 +259,7 @@ where
let shader = &tess.tessellation_control_shader.0;
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
}
@ -247,7 +267,7 @@ where
let shader = &tess.tessellation_evaluation_shader.0;
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
}
}
@ -256,7 +276,7 @@ where
let shader = &fragment_shader.0;
pipeline_layout.ensure_superset_of(
shader.descriptor_set_layout_descs(),
shader.push_constant_ranges(),
shader.push_constant_range(),
)?;
// Check that the subpass can accept the output of the fragment shader.

119
vulkano/src/pipeline/layout/sys.rs
View File

@ -54,22 +54,13 @@ impl PipelineLayout {
let push_constant_ranges: SmallVec<[PipelineLayoutPcRange; 8]> =
push_constant_ranges.into_iter().collect();
// Check for overlapping ranges
// Check for overlapping stages
for (a_id, a) in push_constant_ranges.iter().enumerate() {
for b in push_constant_ranges.iter().skip(a_id + 1) {
if a.offset <= b.offset && a.offset + a.size > b.offset {
if a.stages.intersects(&b.stages) {
return Err(PipelineLayoutCreationError::PushConstantsConflict {
first_offset: a.offset,
first_size: a.size,
second_offset: b.offset,
});
}
if b.offset <= a.offset && b.offset + b.size > a.offset {
return Err(PipelineLayoutCreationError::PushConstantsConflict {
first_offset: b.offset,
first_size: b.size,
second_offset: a.offset,
first_range: *a,
second_range: *b,
});
}
}
@ -179,7 +170,7 @@ impl PipelineLayout {
pub fn ensure_superset_of(
&self,
descriptor_set_layout_descs: &[DescriptorSetDesc],
push_constant_ranges: &[PipelineLayoutPcRange],
push_constant_range: &Option<PipelineLayoutPcRange>,
) -> Result<(), PipelineLayoutSupersetError> {
// Ewwwwwww
let empty = DescriptorSetDesc::empty();
@ -207,6 +198,18 @@ impl PipelineLayout {
}
// FIXME: check push constants
if let Some(range) = push_constant_range {
for own_range in self.push_constant_ranges.as_ref().into_iter() {
if range.stages.intersects(&own_range.stages) && // check if it shares any stages
(range.offset < own_range.offset || // our range must start before and end after the given range
own_range.offset + own_range.size < range.offset + range.size) {
return Err(PipelineLayoutSupersetError::PushConstantRange {
first_range: *own_range,
second_range: *range,
});
}
}
}
Ok(())
}
@ -264,9 +267,8 @@ pub enum PipelineLayoutCreationError {
InvalidPushConstant,
/// Conflict between different push constants ranges.
PushConstantsConflict {
first_offset: usize,
first_size: usize,
second_offset: usize,
first_range: PipelineLayoutPcRange,
second_range: PipelineLayoutPcRange,
},
}
@ -339,6 +341,10 @@ pub enum PipelineLayoutSupersetError {
error: DescriptorSetDescSupersetError,
set_num: u32,
},
PushConstantRange {
first_range: PipelineLayoutPcRange,
second_range: PipelineLayoutPcRange,
},
}
impl error::Error for PipelineLayoutSupersetError {
@ -346,6 +352,7 @@ impl error::Error for PipelineLayoutSupersetError {
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
match *self {
PipelineLayoutSupersetError::DescriptorSet { ref error, .. } => Some(error),
ref error @ PipelineLayoutSupersetError::PushConstantRange { .. } => Some(error),
}
}
}
@ -353,21 +360,36 @@ impl error::Error for PipelineLayoutSupersetError {
impl fmt::Display for PipelineLayoutSupersetError {
#[inline]
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
write!(
fmt,
"{}",
match *self {
PipelineLayoutSupersetError::DescriptorSet { .. } => {
match *self {
PipelineLayoutSupersetError::DescriptorSet { .. } => {
write!(
fmt,
"the descriptor set was not a superset of the other"
}
}
)
)
},
PipelineLayoutSupersetError::PushConstantRange { first_range, second_range } => {
writeln!(fmt, "our range did not completely encompass the other range")?;
writeln!(fmt, " our stages: {:?}", first_range.stages)?;
writeln!(
fmt,
" our range: {} - {}",
first_range.offset,
first_range.offset + first_range.size
)?;
writeln!(fmt, " other stages: {:?}", second_range.stages)?;
write!(fmt,
" other range: {} - {}",
second_range.offset,
second_range.offset + second_range.size
)
},
}
}
}
/// Description of a range of the push constants of a pipeline layout.
// TODO: should contain the layout as well
#[derive(Debug, Copy, Clone)]
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct PipelineLayoutPcRange {
/// Offset in bytes from the start of the push constants to this range.
pub offset: usize,
@ -378,51 +400,6 @@ pub struct PipelineLayoutPcRange {
pub stages: ShaderStages,
}
impl PipelineLayoutPcRange {
pub fn union_multiple(
first: &[PipelineLayoutPcRange],
second: &[PipelineLayoutPcRange],
) -> Vec<PipelineLayoutPcRange> {
first
.iter()
.map(|&(mut new_range)| {
// Find the ranges in `second` that share the same stages as `first_range`.
// If there is a range with a similar stage in `second`, then adjust the offset
// and size to include it.
for second_range in second {
if !second_range.stages.intersects(&new_range.stages) {
continue;
}
if second_range.offset < new_range.offset {
new_range.size += new_range.offset - second_range.offset;
new_range.size = cmp::max(new_range.size, second_range.size);
new_range.offset = second_range.offset;
} else if second_range.offset > new_range.offset {
new_range.size = cmp::max(
new_range.size,
second_range.size + (second_range.offset - new_range.offset),
);
}
}
new_range
})
.chain(
// Add the ones from `second` that were filtered out previously.
second
.iter()
.filter(|second_range| {
first
.iter()
.all(|first_range| !second_range.stages.intersects(&first_range.stages))
})
.map(|range| *range),
)
.collect()
}
}
/* TODO: restore
#[cfg(test)]
mod tests {

28
vulkano/src/pipeline/shader.rs
View File

@ -129,11 +129,11 @@ impl ShaderModule {
/// - The input, output and layout must correctly describe the input, output and layout used
/// by this stage.
///
pub unsafe fn graphics_entry_point<'a, D, P>(
pub unsafe fn graphics_entry_point<'a, D>(
&'a self,
name: &'a CStr,
descriptor_set_layout_descs: D,
push_constant_ranges: P,
push_constant_range: Option<PipelineLayoutPcRange>,
spec_constants: &'static [SpecializationMapEntry],
input: ShaderInterface,
output: ShaderInterface,
@ -141,13 +141,12 @@ impl ShaderModule {
) -> GraphicsEntryPoint<'a>
where
D: IntoIterator<Item = DescriptorSetDesc>,
P: IntoIterator<Item = PipelineLayoutPcRange>,
{
GraphicsEntryPoint {
module: self,
name,
descriptor_set_layout_descs: descriptor_set_layout_descs.into_iter().collect(),
push_constant_ranges: push_constant_ranges.into_iter().collect(),
push_constant_range,
spec_constants,
input,
output,
@ -167,22 +166,21 @@ impl ShaderModule {
/// - The layout must correctly describe the layout used by this stage.
///
#[inline]
pub unsafe fn compute_entry_point<'a, D, P>(
pub unsafe fn compute_entry_point<'a, D>(
&'a self,
name: &'a CStr,
descriptor_set_layout_descs: D,
push_constant_ranges: P,
push_constant_range: Option<PipelineLayoutPcRange>,
spec_constants: &'static [SpecializationMapEntry],
) -> ComputeEntryPoint<'a>
where
D: IntoIterator<Item = DescriptorSetDesc>,
P: IntoIterator<Item = PipelineLayoutPcRange>,
{
ComputeEntryPoint {
module: self,
name,
descriptor_set_layout_descs: descriptor_set_layout_descs.into_iter().collect(),
push_constant_ranges: push_constant_ranges.into_iter().collect(),
push_constant_range,
spec_constants,
}
}
@ -219,7 +217,7 @@ pub unsafe trait EntryPointAbstract {
fn descriptor_set_layout_descs(&self) -> &[DescriptorSetDesc];
/// Returns the push constant ranges.
fn push_constant_ranges(&self) -> &[PipelineLayoutPcRange];
fn push_constant_range(&self) -> &Option<PipelineLayoutPcRange>;
/// Returns the layout of the specialization constants.
fn spec_constants(&self) -> &[SpecializationMapEntry];
@ -234,7 +232,7 @@ pub struct GraphicsEntryPoint<'a> {
name: &'a CStr,
descriptor_set_layout_descs: SmallVec<[DescriptorSetDesc; 16]>,
push_constant_ranges: SmallVec<[PipelineLayoutPcRange; 8]>,
push_constant_range: Option<PipelineLayoutPcRange>,
spec_constants: &'static [SpecializationMapEntry],
input: ShaderInterface,
output: ShaderInterface,
@ -278,8 +276,8 @@ unsafe impl<'a> EntryPointAbstract for GraphicsEntryPoint<'a> {
}
#[inline]
fn push_constant_ranges(&self) -> &[PipelineLayoutPcRange] {
&self.push_constant_ranges
fn push_constant_range(&self) -> &Option<PipelineLayoutPcRange> {
&self.push_constant_range
}
#[inline]
@ -347,7 +345,7 @@ pub struct ComputeEntryPoint<'a> {
module: &'a ShaderModule,
name: &'a CStr,
descriptor_set_layout_descs: SmallVec<[DescriptorSetDesc; 16]>,
push_constant_ranges: SmallVec<[PipelineLayoutPcRange; 8]>,
push_constant_range: Option<PipelineLayoutPcRange>,
spec_constants: &'static [SpecializationMapEntry],
}
@ -368,8 +366,8 @@ unsafe impl<'a> EntryPointAbstract for ComputeEntryPoint<'a> {
}
#[inline]
fn push_constant_ranges(&self) -> &[PipelineLayoutPcRange] {
&self.push_constant_ranges
fn push_constant_range(&self) -> &Option<PipelineLayoutPcRange> {
&self.push_constant_range
}
#[inline]