Skip to content

Commit

Permalink
Basic support for minBufferBindingSize
Browse files Browse the repository at this point in the history
  • Loading branch information
kvark committed Jun 16, 2020
1 parent d1deae5 commit a1730ab
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 66 deletions.
10 changes: 7 additions & 3 deletions player/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use wgc::device::trace;
use std::{
ffi::CString,
fmt::Debug,
fs::File,
fs,
marker::PhantomData,
path::{Path, PathBuf},
ptr,
Expand Down Expand Up @@ -307,7 +307,11 @@ impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
self.bind_group_destroy::<B>(id);
}
A::CreateShaderModule { id, data } => {
let spv = wgt::read_spirv(File::open(dir.join(data)).unwrap()).unwrap();
let byte_vec = fs::read(dir.join(data)).unwrap();
let spv = byte_vec
.chunks(4)
.map(|c| u32::from_le_bytes([c[0], c[1], c[2], c[3]]))
.collect::<Vec<_>>();
self.device_create_shader_module::<B>(
device,
&wgc::pipeline::ShaderModuleDescriptor {
Expand Down Expand Up @@ -470,7 +474,7 @@ fn main() {
};

log::info!("Loading trace '{:?}'", dir);
let file = File::open(dir.join(trace::FILE_NAME)).unwrap();
let file = fs::File::open(dir.join(trace::FILE_NAME)).unwrap();
let mut actions: Vec<trace::Action> = ron::de::from_reader(file).unwrap();
actions.reverse(); // allows us to pop from the top
log::info!("Found {} actions", actions.len());
Expand Down
13 changes: 10 additions & 3 deletions wgpu-core/src/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,28 @@ pub fn map_binding_type(binding: &wgt::BindGroupLayoutEntry) -> hal::pso::Descri
use hal::pso;
use wgt::BindingType as Bt;
match binding.ty {
Bt::UniformBuffer { dynamic } => pso::DescriptorType::Buffer {
Bt::UniformBuffer {
dynamic,
min_binding_size: _,
} => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Uniform,
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: dynamic,
},
},
Bt::StorageBuffer { readonly, dynamic } => pso::DescriptorType::Buffer {
Bt::StorageBuffer {
readonly,
dynamic,
min_binding_size: _,
} => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Storage {
read_only: readonly,
},
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: dynamic,
},
},
Bt::Sampler { .. } => pso::DescriptorType::Sampler,
Bt::Sampler { comparison: _ } => pso::DescriptorType::Sampler,
Bt::SampledTexture { .. } => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Sampled {
with_sampler: false,
Expand Down
85 changes: 76 additions & 9 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ pub enum BindingError {
WrongUsage(naga::GlobalUse),
/// The type on the shader side does not match the pipeline binding.
WrongType,
/// The size of a buffer structure, added to one element of an unbound array,
/// if it's the last field, ended up greater than the given `min_binding_size`.
WrongBufferSize(wgt::BufferAddress),
/// The view dimension doesn't match the shader.
WrongTextureViewDimension { dim: spirv::Dim, is_array: bool },
/// The component type of a sampled texture doesn't match the shader.
Expand Down Expand Up @@ -52,6 +55,45 @@ pub enum StageError {
},
}

fn get_aligned_type_size(
module: &naga::Module,
inner: &naga::TypeInner,
allow_unbound: bool,
) -> wgt::BufferAddress {
use naga::TypeInner as Ti;
//TODO: take alignment into account!
match *inner {
Ti::Scalar { kind: _, width } => width as wgt::BufferAddress / 8,
Ti::Vector {
size,
kind: _,
width,
} => size as wgt::BufferAddress * width as wgt::BufferAddress / 8,
Ti::Matrix {
rows,
columns,
kind: _,
width,
} => {
rows as wgt::BufferAddress * columns as wgt::BufferAddress * width as wgt::BufferAddress
/ 8
}
Ti::Pointer { .. } => 4,
Ti::Array {
base,
size: naga::ArraySize::Static(size),
} => {
size as wgt::BufferAddress
* get_aligned_type_size(module, &module.types[base].inner, false)
}
Ti::Array {
base,
size: naga::ArraySize::Dynamic,
} if allow_unbound => get_aligned_type_size(module, &module.types[base].inner, false),
_ => panic!("Unexpected struct field"),
}
}

fn check_binding(
module: &naga::Module,
var: &naga::GlobalVariable,
Expand All @@ -64,17 +106,42 @@ fn check_binding(
ty_inner = &module.types[base].inner;
}
let allowed_usage = match *ty_inner {
naga::TypeInner::Struct { .. } => match entry.ty {
BindingType::UniformBuffer { .. } => naga::GlobalUse::LOAD,
BindingType::StorageBuffer { readonly, .. } => {
if readonly {
naga::GlobalUse::LOAD
} else {
naga::GlobalUse::all()
naga::TypeInner::Struct { ref members } => {
let (allowed_usage, min_size) = match entry.ty {
BindingType::UniformBuffer {
dynamic: _,
min_binding_size,
} => (naga::GlobalUse::LOAD, min_binding_size),
BindingType::StorageBuffer {
dynamic: _,
min_binding_size,
readonly,
} => {
let global_use = if readonly {
naga::GlobalUse::LOAD
} else {
naga::GlobalUse::all()
};
(global_use, min_binding_size)
}
_ => return Err(BindingError::WrongType),
};
let mut actual_size = 0;
for (i, member) in members.iter().enumerate() {
actual_size += get_aligned_type_size(
module,
&module.types[member.ty].inner,
i + 1 == members.len(),
);
}
_ => return Err(BindingError::WrongType),
},
match min_size {
Some(non_zero) if non_zero.get() < actual_size => {
return Err(BindingError::WrongBufferSize(actual_size))
}
_ => (),
}
allowed_usage
}
naga::TypeInner::Sampler => match entry.ty {
BindingType::Sampler { .. } => naga::GlobalUse::empty(),
_ => return Err(BindingError::WrongType),
Expand Down
67 changes: 16 additions & 51 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use peek_poke::PeekPoke;
use serde::Deserialize;
#[cfg(feature = "trace")]
use serde::Serialize;
use std::{io, slice};

pub type BufferAddress = u64;
pub type NonZeroBufferAddress = std::num::NonZeroU64;

/// Buffer-Texture copies on command encoders have to have the `bytes_per_row`
/// aligned to this number.
Expand Down Expand Up @@ -320,46 +320,6 @@ pub struct DeviceDescriptor {
pub shader_validation: bool,
}

// TODO: This is copy/pasted from gfx-hal, so we need to find a new place to put
// this function
pub fn read_spirv<R: io::Read + io::Seek>(mut x: R) -> io::Result<Vec<u32>> {
let size = x.seek(io::SeekFrom::End(0))?;
if size % 4 != 0 {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"input length not divisible by 4",
));
}
if size > usize::max_value() as u64 {
return Err(io::Error::new(io::ErrorKind::InvalidData, "input too long"));
}
let words = (size / 4) as usize;
let mut result = Vec::<u32>::with_capacity(words);
x.seek(io::SeekFrom::Start(0))?;
unsafe {
// Writing all bytes through a pointer with less strict alignment when our type has no
// invalid bitpatterns is safe.
x.read_exact(slice::from_raw_parts_mut(
result.as_mut_ptr() as *mut u8,
words * 4,
))?;
result.set_len(words);
}
const MAGIC_NUMBER: u32 = 0x0723_0203;
if !result.is_empty() && result[0] == MAGIC_NUMBER.swap_bytes() {
for word in &mut result {
*word = word.swap_bytes();
}
}
if result.is_empty() || result[0] != MAGIC_NUMBER {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"input missing SPIR-V magic number",
));
}
Ok(result)
}

bitflags::bitflags! {
#[repr(transparent)]
#[cfg_attr(feature = "trace", derive(Serialize))]
Expand Down Expand Up @@ -1288,6 +1248,11 @@ pub enum BindingType {
/// Indicates that the binding has a dynamic offset.
/// One offset must be passed to [RenderPass::set_bind_group] for each dynamic binding in increasing order of binding number.
dynamic: bool,
/// Minimum size of the corresponding `BufferBinding` required to match this entry.
/// When pipeline is created, the size has to cover at least the corresponding structure in the shader
/// plus one element of the unbound array, which can only be last in the structure.
/// If `None`, the check is performed at draw call time instead of pipeline and bind group creation.
min_binding_size: Option<NonZeroBufferAddress>,
},
/// A storage buffer.
///
Expand All @@ -1301,6 +1266,11 @@ pub enum BindingType {
/// Indicates that the binding has a dynamic offset.
/// One offset must be passed to [RenderPass::set_bind_group] for each dynamic binding in increasing order of binding number.
dynamic: bool,
/// Minimum size of the corresponding `BufferBinding` required to match this entry.
/// When pipeline is created, the size has to cover at least the corresponding structure in the shader
/// plus one element of the unbound array, which can only be last in the structure.
/// If `None`, the check is performed at draw call time instead of pipeline and bind group creation.
min_binding_size: Option<NonZeroBufferAddress>,
/// The buffer can only be read in the shader and it must be annotated with `readonly`.
///
/// Example GLSL syntax:
Expand Down Expand Up @@ -1350,9 +1320,6 @@ pub enum BindingType {
StorageTexture {
/// Dimension of the texture view that is going to be sampled.
dimension: TextureViewDimension,
/// Component type of the texture.
/// This must be compatible with the format of the texture.
component_type: TextureComponentType,
/// Format of the texture.
format: TextureFormat,
/// The texture can only be read in the shader and it must be annotated with `readonly`.
Expand Down Expand Up @@ -1384,19 +1351,17 @@ pub struct BindGroupLayoutEntry {
pub _non_exhaustive: NonExhaustive,
}

impl Default for BindGroupLayoutEntry {
fn default() -> Self {
impl BindGroupLayoutEntry {
pub fn new(binding: u32, visibility: ShaderStage, ty: BindingType) -> Self {
Self {
binding: 0,
visibility: ShaderStage::NONE,
ty: BindingType::UniformBuffer { dynamic: false },
binding,
visibility,
ty,
count: None,
_non_exhaustive: unsafe { NonExhaustive::new() },
}
}
}

impl BindGroupLayoutEntry {
pub fn has_dynamic_offset(&self) -> bool {
match self.ty {
BindingType::UniformBuffer { dynamic, .. }
Expand Down

0 comments on commit a1730ab

Please sign in to comment.