Skip to content

Commit

Permalink
Merge #1047
Browse files Browse the repository at this point in the history 
1047: Update bind group layout API to match upstream r=cwfitzgerald a=kvark

**Connections**
Follows gpuweb/gpuweb#1076, gpuweb/gpuweb#1223 (gpuweb/gpuweb#1164), gpuweb/gpuweb#1255, and gpuweb/gpuweb#1256

**Description**
Aligns our API closer to the latest changes in WebGPU upstream. We technically don't have to do this, but I believe in the end it would be best if our API gets close to upstream.

Note: this is a sensitive change for the users, everybody will get their code broken. So please take a look at the API and see if something is missing or needs improvement, so that we don't have to go through the changes again afterwards.

**Testing**
Doesn't really need testing. Partially covered by the existing playtest.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
  • Loading branch information
@bors @kvark
bors[bot] and kvark committed Nov 30, 2020
2 parents 3efba77 + 5949dec commit 67e652f
Show file tree
Hide file tree
Showing 6 changed files with 315 additions and 218 deletions.
6 changes: 2 additions & 4 deletions player/tests/data/bind-group.ron
View file
Expand Up @@ -34,11 +34,9 @@
(
binding: 0,
visibility: (bits: 0x3),
ty: UniformBuffer(
dynamic: false,
min_binding_size: None,
ty: Buffer(
ty: Uniform,
),
count: None,
),
],
)),
Expand Down
18 changes: 13 additions & 5 deletions wgpu-core/src/binding_model.rs
View file
Expand Up @@ -193,22 +193,30 @@ impl BindingTypeMaxCountValidator {
pub(crate) fn add_binding(&mut self, binding: &wgt::BindGroupLayoutEntry) {
let count = binding.count.map_or(1, |count| count.get());
match binding.ty {
wgt::BindingType::UniformBuffer { dynamic, .. } => {
wgt::BindingType::Buffer {
ty: wgt::BufferBindingType::Uniform,
has_dynamic_offset,
..
} => {
self.uniform_buffers.add(binding.visibility, count);
if dynamic {
if has_dynamic_offset {
self.dynamic_uniform_buffers += count;
}
}
wgt::BindingType::StorageBuffer { dynamic, .. } => {
wgt::BindingType::Buffer {
ty: wgt::BufferBindingType::Storage { .. },
has_dynamic_offset,
..
} => {
self.storage_buffers.add(binding.visibility, count);
if dynamic {
if has_dynamic_offset {
self.dynamic_storage_buffers += count;
}
}
wgt::BindingType::Sampler { .. } => {
self.samplers.add(binding.visibility, count);
}
wgt::BindingType::SampledTexture { .. } => {
wgt::BindingType::Texture { .. } => {
self.sampled_textures.add(binding.visibility, count);
}
wgt::BindingType::StorageTexture { .. } => {
Expand Down
35 changes: 16 additions & 19 deletions wgpu-core/src/conv.rs
View file
Expand Up @@ -84,36 +84,33 @@ 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,
Bt::Buffer {
ty,
has_dynamic_offset,
min_binding_size: _,
} => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Uniform,
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: dynamic,
},
},
Bt::StorageBuffer {
readonly,
dynamic,
min_binding_size: _,
} => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Storage {
read_only: readonly,
ty: match ty {
wgt::BufferBindingType::Uniform => pso::BufferDescriptorType::Uniform,
wgt::BufferBindingType::Storage { read_only } => {
pso::BufferDescriptorType::Storage { read_only }
}
},
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: dynamic,
dynamic_offset: has_dynamic_offset,
},
},
Bt::Sampler { comparison: _ } => pso::DescriptorType::Sampler,
Bt::SampledTexture { .. } => pso::DescriptorType::Image {
Bt::Sampler { .. } => pso::DescriptorType::Sampler,
Bt::Texture { .. } => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Sampled {
with_sampler: false,
},
},
Bt::StorageTexture { readonly, .. } => pso::DescriptorType::Image {
Bt::StorageTexture { access, .. } => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Storage {
read_only: readonly,
read_only: match access {
wgt::StorageTextureAccess::ReadOnly => true,
_ => false,
},
},
},
}
Expand Down
67 changes: 34 additions & 33 deletions wgpu-core/src/device/mod.rs
View file
Expand Up @@ -989,7 +989,7 @@ impl<B: GfxBackend> Device<B> {
for binding in entry_map.values() {
if binding.count.is_some() {
match binding.ty {
wgt::BindingType::SampledTexture { .. } => {
wgt::BindingType::Texture { .. } => {
if !self
.features
.contains(wgt::Features::SAMPLED_TEXTURE_BINDING_ARRAY)
Expand Down Expand Up @@ -1100,30 +1100,12 @@ impl<B: GfxBackend> Device<B> {
.ok_or(Error::MissingBindingDeclaration(binding))?;
let descriptors: SmallVec<[_; 1]> = match entry.resource {
Br::Buffer(ref bb) => {
let (pub_usage, internal_use, min_size, dynamic) = match decl.ty {
wgt::BindingType::UniformBuffer {
dynamic,
let (binding_ty, dynamic, min_size) = match decl.ty {
wgt::BindingType::Buffer {
ty,
has_dynamic_offset,
min_binding_size,
} => (
wgt::BufferUsage::UNIFORM,
resource::BufferUse::UNIFORM,
min_binding_size,
dynamic,
),
wgt::BindingType::StorageBuffer {
dynamic,
min_binding_size,
readonly,
} => (
wgt::BufferUsage::STORAGE,
if readonly {
resource::BufferUse::STORAGE_LOAD
} else {
resource::BufferUse::STORAGE_STORE
},
min_binding_size,
dynamic,
),
} => (ty, has_dynamic_offset, min_binding_size),
_ => {
return Err(Error::WrongBindingType {
binding,
Expand All @@ -1132,6 +1114,19 @@ impl<B: GfxBackend> Device<B> {
})
}
};
let (pub_usage, internal_use) = match binding_ty {
wgt::BufferBindingType::Uniform => {
(wgt::BufferUsage::UNIFORM, resource::BufferUse::UNIFORM)
}
wgt::BufferBindingType::Storage { read_only } => (
wgt::BufferUsage::STORAGE,
if read_only {
resource::BufferUse::STORAGE_LOAD
} else {
resource::BufferUse::STORAGE_STORE
},
),
};

if bb.offset % wgt::BIND_BUFFER_ALIGNMENT != 0 {
return Err(Error::UnalignedBufferOffset(bb.offset));
Expand Down Expand Up @@ -1161,7 +1156,7 @@ impl<B: GfxBackend> Device<B> {
None => (buffer.size - bb.offset, buffer.size),
};

if pub_usage == wgt::BufferUsage::UNIFORM
if binding_ty == wgt::BufferBindingType::Uniform
&& (self.limits.max_uniform_buffer_binding_size as u64) < bind_size
{
return Err(Error::UniformBufferRangeTooLarge);
Expand Down Expand Up @@ -1192,7 +1187,10 @@ impl<B: GfxBackend> Device<B> {
}
Br::Sampler(id) => {
match decl.ty {
wgt::BindingType::Sampler { comparison } => {
wgt::BindingType::Sampler {
filtering: _,
comparison,
} => {
let sampler = used
.samplers
.use_extend(&*sampler_guard, id, (), ())
Expand Down Expand Up @@ -1220,15 +1218,18 @@ impl<B: GfxBackend> Device<B> {
.use_extend(&*texture_view_guard, id, (), ())
.map_err(|_| Error::InvalidTextureView(id))?;
let (pub_usage, internal_use) = match decl.ty {
wgt::BindingType::SampledTexture { .. } => {
wgt::BindingType::Texture { .. } => {
(wgt::TextureUsage::SAMPLED, resource::TextureUse::SAMPLED)
}
wgt::BindingType::StorageTexture { readonly, .. } => (
wgt::BindingType::StorageTexture { access, .. } => (
wgt::TextureUsage::STORAGE,
if readonly {
resource::TextureUse::STORAGE_LOAD
} else {
resource::TextureUse::STORAGE_STORE
match access {
wgt::StorageTextureAccess::ReadOnly => {
resource::TextureUse::STORAGE_LOAD
}
wgt::StorageTextureAccess::WriteOnly => {
resource::TextureUse::STORAGE_STORE
}
},
),
_ => return Err(Error::WrongBindingType {
Expand Down Expand Up @@ -1290,7 +1291,7 @@ impl<B: GfxBackend> Device<B> {
}

let (pub_usage, internal_use) = match decl.ty {
wgt::BindingType::SampledTexture { .. } => {
wgt::BindingType::Texture { .. } => {
(wgt::TextureUsage::SAMPLED, resource::TextureUse::SAMPLED)
}
_ => {
Expand Down

0 comments on commit 67e652f

Please sign in to comment.