Skip to content

Commit

Permalink
Merge gfx-rs#980
Browse files Browse the repository at this point in the history
980: Group binding writes by wgt::BindingType r=cwfitzgerald a=kvark

**Connections**
Follows up gfx-rs#970
Fixes gfx-rs#979

**Description**
The problem was that Vulkan restriction applies to its own descriptor types, as specified in the layout. I originally interpret this as types specified in the descriptor set. So we erroneously considered `StorageTexture` and `SampledTexture` to be in the same descriptor write.

This PR makes it use the `wgt::BindingType` instead. It's a bit richer than Vulkan side, but still correct. We are just more conservative than we have to be.

I think gfx-hal API could be better here. Filed gfx-rs/gfx#3408 to look more.

**Testing**
Tested on gfx-rs#979 test case (thanks!)

Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
  • Loading branch information
bors[bot] and kvark committed Oct 13, 2020
2 parents c933b97 + 98c3de7 commit 7ac706f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 22 deletions.
29 changes: 8 additions & 21 deletions wgpu-core/src/device/mod.rs
Expand Up @@ -701,13 +701,13 @@ impl<B: GfxBackend> Device<B> {
.map_or(1, |v| v.get() as hal::pso::DescriptorArrayIndex), //TODO: consolidate
stage_flags: conv::map_shader_stage_flags(entry.visibility),
immutable_samplers: false, // TODO
})
.collect::<Vec<_>>(); //TODO: avoid heap allocation
});
let desc_counts = raw_bindings.clone().collect();

let raw = unsafe {
let mut raw_layout = self
.raw
.create_descriptor_set_layout(&raw_bindings, &[])
.create_descriptor_set_layout(raw_bindings, &[])
.or(Err(DeviceError::OutOfMemory))?;
if let Some(label) = label {
self.raw
Expand All @@ -733,7 +733,7 @@ impl<B: GfxBackend> Device<B> {
ref_count: self.life_guard.add_ref(),
},
multi_ref_count: MultiRefCount::new(),
desc_counts: raw_bindings.iter().cloned().collect(),
desc_counts,
dynamic_count: entry_map
.values()
.filter(|b| b.ty.has_dynamic_offset())
Expand Down Expand Up @@ -2356,34 +2356,21 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

if !write_map.is_empty() {
#[derive(PartialEq)]
enum DescriptorType {
Buffer,
Sampler,
TextureView,
}
let mut writes = Vec::<hal::pso::DescriptorSetWrite<_, SmallVec<[_; 1]>>>::new();
let mut prev_stages = wgt::ShaderStage::empty();
let mut prev_ty = DescriptorType::Buffer;
let mut prev_ty = wgt::BindingType::Sampler { comparison: false }; // doesn't matter
for (binding, list) in write_map {
let layout = &bind_group_layout.entries[&binding];
let ty = match layout.ty {
wgt::BindingType::UniformBuffer { .. }
| wgt::BindingType::StorageBuffer { .. } => DescriptorType::Buffer,
wgt::BindingType::Sampler { .. } => DescriptorType::Sampler,
wgt::BindingType::SampledTexture { .. }
| wgt::BindingType::StorageTexture { .. } => DescriptorType::TextureView,
};
if layout.visibility == prev_stages && ty == prev_ty {
if layout.visibility == prev_stages && layout.ty == prev_ty {
writes.last_mut().unwrap().descriptors.extend(list);
} else {
prev_stages = layout.visibility;
prev_ty = ty;
prev_ty = layout.ty;
writes.push(hal::pso::DescriptorSetWrite {
set: desc_set.raw(),
binding,
array_offset: 0,
descriptors: list.into_iter().collect(),
descriptors: list,
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion wgpu-types/src/lib.rs
Expand Up @@ -1720,7 +1720,7 @@ pub struct TextureDataLayout {
/// Specific type of a binding.
///
/// WebGPU spec: https://gpuweb.github.io/gpuweb/#dictdef-gpubindgrouplayoutentry
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub enum BindingType {
Expand Down

0 comments on commit 7ac706f

Please sign in to comment.