Skip to content

Commit

Permalink
Merge #711
Browse files Browse the repository at this point in the history
711: Implement Descriptor Array Extension r=kvark a=cwfitzgerald

## Connections

Blocked on gfx-rs/gfx#3269 and does some funky git overrides to get CI to return meaningful results what will be removed once that PR lands and is published.

## Description

This PR implements the `TEXTURE_BINDING_ARRAY` native extension. This allows users to specify a uniform sized array of textures for use in shaders.

As a corollary, this PR rustifies the Bind Group and Bind Group Layout interface. Two main actions were taken when doing this:
 - Types that were able to be shared among wgt, wgc, and wgpu-rs were moved into wgt.
 - Types that had references to other wgpu-rs specific structures were duplicated into wgc with wgpu-rs structures replaced with the appropriate ID. Notes were added to the wgc types that they were duplicated directly from wgpu-rs.

From what I can tell, this resulted in a significant reduction in code complexity when dealing with bind groups, favoring strong types over runtime assertions.

Naga validation of arrays of textures was not implemented.

Documentation was added to extensions to help users understand what underlying tech was being relied on as well as the platforms it should be expected to work on. I think this pattern should be implemented across the board for extensions as it makes them much more user friendly to use.

## Testing

There is an example included in the wgpu-rs PR which was used for testing this feature. It worked on DX12, Vulkan, and Metal (MSL > 2.0), as was expected.

Additionally the other examples were run and are still verified to run.


Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
  • Loading branch information
bors[bot] and cwfitzgerald committed Jun 11, 2020
2 parents 847d974 + 57b3b72 commit 64ae590
Show file tree
Hide file tree
Showing 10 changed files with 616 additions and 326 deletions.
327 changes: 195 additions & 132 deletions Cargo.lock

Large diffs are not rendered by default.

23 changes: 11 additions & 12 deletions player/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,11 @@ impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
}
}
A::CreateBindGroupLayout { id, label, entries } => {
let label = Label::new(&label);
self.device_create_bind_group_layout::<B>(
device,
&wgc::binding_model::BindGroupLayoutDescriptor {
label: label.as_ptr(),
entries: entries.as_ptr(),
entries_length: entries.len(),
&wgt::BindGroupLayoutDescriptor {
label: Some(&label),
bindings: &entries,
},
id,
)
Expand Down Expand Up @@ -294,12 +292,11 @@ impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
entries,
} => {
use wgc::binding_model as bm;
let label = Label::new(&label);
let entry_vec = entries
.into_iter()
.iter()
.map(|(binding, res)| wgc::binding_model::BindGroupEntry {
binding,
resource: match res {
binding: *binding,
resource: match *res {
trace::BindingResource::Buffer { id, offset, size } => {
bm::BindingResource::Buffer(bm::BufferBinding {
buffer: id,
Expand All @@ -311,17 +308,19 @@ impl GlobalExt for wgc::hub::Global<IdentityPassThroughFactory> {
trace::BindingResource::TextureView(id) => {
bm::BindingResource::TextureView(id)
}
trace::BindingResource::TextureViewArray(ref id_array) => {
bm::BindingResource::TextureViewArray(id_array)
}
},
})
.collect::<Vec<_>>();
self.device_maintain_ids::<B>(device);
self.device_create_bind_group::<B>(
device,
&wgc::binding_model::BindGroupDescriptor {
label: label.as_ptr(),
label: Some(&label),
layout: layout_id,
entries: entry_vec.as_ptr(),
entries_length: entry_vec.len(),
bindings: &entry_vec,
},
id,
);
Expand Down
12 changes: 6 additions & 6 deletions wgpu-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ bitflags = "1.0"
copyless = "0.1"
fxhash = "0.2"
log = "0.4"
hal = { package = "gfx-hal", version = "0.5" }
hal = { package = "gfx-hal", version = "0.5.1" }
gfx-backend-empty = "0.5"
gfx-descriptor = "0.1"
gfx-memory = "0.1"
Expand All @@ -51,16 +51,16 @@ version = "0.5"
features = ["peek-poke"]

[target.'cfg(any(target_os = "ios", target_os = "macos"))'.dependencies]
gfx-backend-metal = { version = "0.5" }
gfx-backend-vulkan = { version = "0.5", optional = true }
gfx-backend-metal = { version = "0.5.3" }
gfx-backend-vulkan = { version = "0.5.7", optional = true }

[target.'cfg(all(unix, not(target_os = "ios"), not(target_os = "macos")))'.dependencies]
gfx-backend-vulkan = { version = "0.5" }
gfx-backend-vulkan = { version = "0.5.7" }

[target.'cfg(windows)'.dependencies]
gfx-backend-dx12 = { version = "0.5" }
gfx-backend-dx12 = { version = "0.5.5" }
gfx-backend-dx11 = { version = "0.5" }
gfx-backend-vulkan = { version = "0.5" }
gfx-backend-vulkan = { version = "0.5.7" }

[target.'cfg(any(target_os = "linux", target_os = "macos", target_os = "windows", target_os = "dragonfly", target_os = "freebsd"))'.dependencies]
battery = { version = "0.7", optional = true }
Expand Down
102 changes: 16 additions & 86 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,83 +17,17 @@ use serde::Deserialize;
use serde::Serialize;
use std::borrow::Borrow;

#[repr(C)]
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub enum BindingType {
UniformBuffer = 0,
StorageBuffer = 1,
ReadonlyStorageBuffer = 2,
Sampler = 3,
ComparisonSampler = 4,
SampledTexture = 5,
ReadonlyStorageTexture = 6,
WriteonlyStorageTexture = 7,
}

#[repr(C)]
#[derive(Clone, Debug, Hash, PartialEq)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct BindGroupLayoutEntry {
pub binding: u32,
pub visibility: wgt::ShaderStage,
pub ty: BindingType,
pub multisampled: bool,
pub has_dynamic_offset: bool,
pub view_dimension: wgt::TextureViewDimension,
pub texture_component_type: wgt::TextureComponentType,
pub storage_texture_format: wgt::TextureFormat,
}

#[derive(Clone, Debug)]
pub enum BindGroupLayoutEntryError {
NoVisibility,
UnexpectedHasDynamicOffset,
UnexpectedMultisampled,
}

impl BindGroupLayoutEntry {
pub(crate) fn validate(&self) -> Result<(), BindGroupLayoutEntryError> {
if self.visibility.is_empty() {
return Err(BindGroupLayoutEntryError::NoVisibility);
}
match self.ty {
BindingType::UniformBuffer | BindingType::StorageBuffer => {}
_ => {
if self.has_dynamic_offset {
return Err(BindGroupLayoutEntryError::UnexpectedHasDynamicOffset);
}
}
}
match self.ty {
BindingType::SampledTexture => {}
_ => {
if self.multisampled {
return Err(BindGroupLayoutEntryError::UnexpectedMultisampled);
}
}
}
Ok(())
}
}

#[repr(C)]
#[derive(Debug)]
pub struct BindGroupLayoutDescriptor {
pub label: *const std::os::raw::c_char,
pub entries: *const BindGroupLayoutEntry,
pub entries_length: usize,
}

#[derive(Clone, Debug)]
pub enum BindGroupLayoutError {
ConflictBinding(u32),
Entry(u32, BindGroupLayoutEntryError),
MissingExtension(wgt::Extensions),
/// Arrays of bindings can't be 0 elements long
ZeroCount,
/// Arrays of bindings unsupported for this type of binding
ArrayUnsupported,
}

pub(crate) type BindEntryMap = FastHashMap<u32, BindGroupLayoutEntry>;
pub(crate) type BindEntryMap = FastHashMap<u32, wgt::BindGroupLayoutEntry>;

#[derive(Debug)]
pub struct BindGroupLayout<B: hal::Backend> {
Expand Down Expand Up @@ -135,32 +69,28 @@ pub struct BufferBinding {
pub size: wgt::BufferSize,
}

#[repr(C)]
// Note: Duplicated in wgpu-rs as BindingResource
#[derive(Debug)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub enum BindingResource {
pub enum BindingResource<'a> {
Buffer(BufferBinding),
Sampler(SamplerId),
TextureView(TextureViewId),
TextureViewArray(&'a [TextureViewId]),
}

#[repr(C)]
// Note: Duplicated in wgpu-rs as Binding
#[derive(Debug)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct BindGroupEntry {
pub struct BindGroupEntry<'a> {
pub binding: u32,
pub resource: BindingResource,
pub resource: BindingResource<'a>,
}

#[repr(C)]
// Note: Duplicated in wgpu-rs as BindGroupDescriptor
#[derive(Debug)]
pub struct BindGroupDescriptor {
pub label: *const std::os::raw::c_char,
pub struct BindGroupDescriptor<'a> {
pub label: Option<&'a str>,
pub layout: BindGroupLayoutId,
pub entries: *const BindGroupEntry,
pub entries_length: usize,
pub bindings: &'a [BindGroupEntry<'a>],
}

#[derive(Debug)]
Expand Down
36 changes: 16 additions & 20 deletions wgpu-core/src/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use crate::{binding_model, resource, PrivateFeatures};
use crate::{resource, PrivateFeatures};

pub fn map_buffer_usage(usage: wgt::BufferUsage) -> (hal::buffer::Usage, hal::memory::Properties) {
use hal::buffer::Usage as U;
Expand Down Expand Up @@ -75,40 +75,36 @@ pub fn map_texture_usage(
value
}

pub fn map_binding_type(binding: &binding_model::BindGroupLayoutEntry) -> hal::pso::DescriptorType {
use crate::binding_model::BindingType as Bt;
pub fn map_binding_type(binding: &wgt::BindGroupLayoutEntry) -> hal::pso::DescriptorType {
use hal::pso;
use wgt::BindingType as Bt;
match binding.ty {
Bt::UniformBuffer => pso::DescriptorType::Buffer {
Bt::UniformBuffer { dynamic } => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Uniform,
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: binding.has_dynamic_offset,
dynamic_offset: dynamic,
},
},
Bt::StorageBuffer => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Storage { read_only: false },
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: binding.has_dynamic_offset,
Bt::StorageBuffer { readonly, dynamic } => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Storage {
read_only: readonly,
},
},
Bt::ReadonlyStorageBuffer => pso::DescriptorType::Buffer {
ty: pso::BufferDescriptorType::Storage { read_only: true },
format: pso::BufferDescriptorFormat::Structured {
dynamic_offset: binding.has_dynamic_offset,
dynamic_offset: dynamic,
},
},
Bt::Sampler | Bt::ComparisonSampler => pso::DescriptorType::Sampler,
Bt::SampledTexture => pso::DescriptorType::Image {
Bt::Sampler { .. } => pso::DescriptorType::Sampler,
Bt::SampledTexture { .. } => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Sampled {
with_sampler: false,
},
},
Bt::ReadonlyStorageTexture => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Storage { read_only: true },
},
Bt::WriteonlyStorageTexture => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Storage { read_only: false },
Bt::StorageTexture { readonly, .. } => pso::DescriptorType::Image {
ty: pso::ImageDescriptorType::Storage {
read_only: readonly,
},
},
_ => unreachable!(),
}
}

Expand Down
Loading

0 comments on commit 64ae590

Please sign in to comment.