Skip to content

Commit

Permalink
Merge #1511
Browse files Browse the repository at this point in the history 
1511: Added storage texture array support. r=kvark a=ElectronicRU

**Description**
Arrays of storage images (*f*image*n*D in GLSL parlance) should now be supported.
I also took the liberty to refactor texture format checking a bit,
and tighten up sampled texture support detection for Metal as well.

**Testing**
Not completely sure what's the proper testing approach here, open to suggestions.


Co-authored-by: Alex S <alex0player@gmail.com>
  • Loading branch information
@bors @ElectronicRU
bors[bot] and ElectronicRU authored Jun 18, 2021
2 parents fae0d4b + 894489b commit 1d2efb4
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 116 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
- fix SPIR-V generation from WGSL, which was broken due to "Kernel" capability
- validate buffer storage classes

## Unreleased
- Added support for storage texture arrays for Vulkan and Metal.

## v0.8 (2021-04-29)
- Naga is used by default to translate shaders, SPIRV-Cross is optional behind `cross` feature
- Features:
Expand Down
222 changes: 112 additions & 110 deletions wgpu-core/src/device/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ impl<A: HalApi> Device<A> {
Bt::Sampler { .. } => (None, false),
Bt::Texture { .. } => (Some(wgt::Features::SAMPLED_TEXTURE_BINDING_ARRAY), false),
Bt::StorageTexture { access, .. } => (
None,
Some(wgt::Features::STORAGE_TEXTURE_BINDING_ARRAY),
match access {
wgt::StorageTextureAccess::ReadOnly => false,
wgt::StorageTextureAccess::WriteOnly => true,
Expand Down Expand Up @@ -1309,104 +1309,13 @@ impl<A: HalApi> Device<A> {
.views
.use_extend(&*texture_view_guard, id, (), ())
.map_err(|_| Error::InvalidTextureView(id))?;
let format_info = view.desc.format.describe();
let (pub_usage, internal_use) = match decl.ty {
wgt::BindingType::Texture {
sample_type,
view_dimension,
multisampled,
} => {
use wgt::TextureSampleType as Tst;
if multisampled != (view.samples != 1) {
return Err(Error::InvalidTextureMultisample {
binding,
layout_multisampled: multisampled,
view_samples: view.samples,
});
}
match (sample_type, format_info.sample_type, view.format_features.filterable ) {
(Tst::Uint, Tst::Uint, ..) |
(Tst::Sint, Tst::Sint, ..) |
(Tst::Depth, Tst::Depth, ..) |
// if we expect non-filterable, accept anything float
(Tst::Float { filterable: false }, Tst::Float { .. }, ..) |
// if we expect filterable, require it
(Tst::Float { filterable: true }, Tst::Float { filterable: true }, ..) |
// if we expect filterable, also accept Float that is defined as unfilterable if filterable feature is explicitly enabled
// (only hit if wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES is enabled)
(Tst::Float { filterable: true }, Tst::Float { .. }, true) |
// if we expect float, also accept depth
(Tst::Float { .. }, Tst::Depth, ..) => {}
_ => {
return Err(Error::InvalidTextureSampleType {
binding,
layout_sample_type: sample_type,
view_format: view.desc.format,
})
}
}
if view_dimension != view.desc.dimension {
return Err(Error::InvalidTextureDimension {
binding,
layout_dimension: view_dimension,
view_dimension: view.desc.dimension,
});
}
(wgt::TextureUsage::SAMPLED, view.sampled_internal_use)
}
wgt::BindingType::StorageTexture {
access,
format,
view_dimension,
} => {
if format != view.desc.format {
return Err(Error::InvalidStorageTextureFormat {
binding,
layout_format: format,
view_format: view.desc.format,
});
}
if view_dimension != view.desc.dimension {
return Err(Error::InvalidTextureDimension {
binding,
layout_dimension: view_dimension,
view_dimension: view.desc.dimension,
});
}
let internal_use = match access {
wgt::StorageTextureAccess::ReadOnly => {
hal::TextureUse::STORAGE_LOAD
}
wgt::StorageTextureAccess::WriteOnly => {
hal::TextureUse::STORAGE_STORE
}
wgt::StorageTextureAccess::ReadWrite => {
if !view.format_features.flags.contains(
wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE,
) {
return Err(Error::StorageReadWriteNotSupported(
view.desc.format,
));
}

hal::TextureUse::STORAGE_STORE | hal::TextureUse::STORAGE_LOAD
}
};
(wgt::TextureUsage::STORAGE, internal_use)
}
_ => return Err(Error::WrongBindingType {
binding,
actual: decl.ty,
expected:
"SampledTexture, ReadonlyStorageTexture or WriteonlyStorageTexture",
}),
};
let (pub_usage, internal_use) = Self::texture_use_parameters(
binding,
decl,
view,
"SampledTexture, ReadonlyStorageTexture or WriteonlyStorageTexture",
)?;

if hal::FormatAspect::from(view.desc.format)
.contains(hal::FormatAspect::DEPTH | hal::FormatAspect::STENCIL)
{
return Err(Error::DepthStencilAspect);
}
match view.source {
resource::TextureViewSource::Native(ref source_id) => {
// Careful here: the texture may no longer have its own ref count,
Expand Down Expand Up @@ -1454,18 +1363,10 @@ impl<A: HalApi> Device<A> {
.views
.use_extend(&*texture_view_guard, id, (), ())
.map_err(|_| Error::InvalidTextureView(id))?;
let (pub_usage, internal_use) = match decl.ty {
wgt::BindingType::Texture { .. } => {
(wgt::TextureUsage::SAMPLED, view.sampled_internal_use)
}
_ => {
return Err(Error::WrongBindingType {
binding,
actual: decl.ty,
expected: "SampledTextureArray",
})
}
};
let (pub_usage, internal_use) =
Self::texture_use_parameters(binding, decl, view,
"SampledTextureArray, ReadonlyStorageTextureArray or WriteonlyStorageTextureArray"
)?;

match view.source {
resource::TextureViewSource::Native(ref source_id) => {
Expand Down Expand Up @@ -1538,6 +1439,107 @@ impl<A: HalApi> Device<A> {
})
}

fn texture_use_parameters(
binding: u32,
decl: &wgt::BindGroupLayoutEntry,
view: &crate::resource::TextureView<A>,
expected: &'static str,
) -> Result<(wgt::TextureUsage, hal::TextureUse), binding_model::CreateBindGroupError> {
use crate::binding_model::CreateBindGroupError as Error;
if hal::FormatAspect::from(view.desc.format)
.contains(hal::FormatAspect::DEPTH | hal::FormatAspect::STENCIL)
{
return Err(Error::DepthStencilAspect);
}
let format_info = view.desc.format.describe();
match decl.ty {
wgt::BindingType::Texture {
sample_type,
view_dimension,
multisampled,
} => {
use wgt::TextureSampleType as Tst;
if multisampled != (view.samples != 1) {
return Err(Error::InvalidTextureMultisample {
binding,
layout_multisampled: multisampled,
view_samples: view.samples,
});
}
match (sample_type, format_info.sample_type, view.format_features.filterable) {
(Tst::Uint, Tst::Uint, ..) |
(Tst::Sint, Tst::Sint, ..) |
(Tst::Depth, Tst::Depth, ..) |
// if we expect non-filterable, accept anything float
(Tst::Float { filterable: false }, Tst::Float { .. }, ..) |
// if we expect filterable, require it
(Tst::Float { filterable: true }, Tst::Float { filterable: true }, ..) |
// if we expect filterable, also accept Float that is defined as unfilterable if filterable feature is explicitly enabled
// (only hit if wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES is enabled)
(Tst::Float { filterable: true }, Tst::Float { .. }, true) |
// if we expect float, also accept depth
(Tst::Float { .. }, Tst::Depth, ..) => {}
_ => {
return Err(Error::InvalidTextureSampleType {
binding,
layout_sample_type: sample_type,
view_format: view.desc.format,
})
}
}
if view_dimension != view.desc.dimension {
return Err(Error::InvalidTextureDimension {
binding,
layout_dimension: view_dimension,
view_dimension: view.desc.dimension,
});
}
Ok((wgt::TextureUsage::SAMPLED, view.sampled_internal_use))
}
wgt::BindingType::StorageTexture {
access,
format,
view_dimension,
} => {
if format != view.desc.format {
return Err(Error::InvalidStorageTextureFormat {
binding,
layout_format: format,
view_format: view.desc.format,
});
}
if view_dimension != view.desc.dimension {
return Err(Error::InvalidTextureDimension {
binding,
layout_dimension: view_dimension,
view_dimension: view.desc.dimension,
});
}
let internal_use = match access {
wgt::StorageTextureAccess::ReadOnly => hal::TextureUse::STORAGE_LOAD,
wgt::StorageTextureAccess::WriteOnly => hal::TextureUse::STORAGE_STORE,
wgt::StorageTextureAccess::ReadWrite => {
if !view
.format_features
.flags
.contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE)
{
return Err(Error::StorageReadWriteNotSupported(view.desc.format));
}

hal::TextureUse::STORAGE_STORE | hal::TextureUse::STORAGE_LOAD
}
};
Ok((wgt::TextureUsage::STORAGE, internal_use))
}
_ => Err(Error::WrongBindingType {
binding,
actual: decl.ty,
expected,
}),
}
}

fn create_pipeline_layout(
&self,
self_id: id::DeviceId,
Expand Down
33 changes: 31 additions & 2 deletions wgpu-hal/src/metal/adapter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,22 @@ impl super::PrivateCapabilities {
} else {
Self::version_at_least(major, minor, 11, 0)
},
supports_arrays_of_textures: Self::supports_any(
&device,
&[
MTLFeatureSet::iOS_GPUFamily3_v2,
MTLFeatureSet::iOS_GPUFamily4_v1,
MTLFeatureSet::iOS_GPUFamily5_v1,
MTLFeatureSet::tvOS_GPUFamily2_v1,
MTLFeatureSet::macOS_GPUFamily1_v3,
MTLFeatureSet::macOS_GPUFamily2_v1,
],
),
supports_arrays_of_textures_write: device.supports_family(MTLGPUFamily::Apple6)
|| device.supports_family(MTLGPUFamily::Mac1)
|| device.supports_family(MTLGPUFamily::Mac2)
|| device.supports_family(MTLGPUFamily::MacCatalyst1)
|| device.supports_family(MTLGPUFamily::MacCatalyst2),
}
}

Expand All @@ -830,8 +846,21 @@ impl super::PrivateCapabilities {
| F::VERTEX_WRITABLE_STORAGE;

features.set(
F::SAMPLED_TEXTURE_BINDING_ARRAY | F::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING,
self.msl_version >= MTLLanguageVersion::V2_0,
F::SAMPLED_TEXTURE_BINDING_ARRAY
| F::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING
| F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
self.msl_version >= MTLLanguageVersion::V2_0 && self.supports_arrays_of_textures,
);
//// XXX: this is technically not true, as read-only storage images can be used in arrays
//// on precisely the same conditions that sampled textures can. But texel fetch from a
//// sampled texture is a thing; should we bother introducing another feature flag?
features.set(
F::STORAGE_TEXTURE_BINDING_ARRAY
| F::STORAGE_TEXTURE_ARRAY_DYNAMIC_INDEXING
| F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
self.msl_version >= MTLLanguageVersion::V2_2
&& self.supports_arrays_of_textures
&& self.supports_arrays_of_textures_write,
);
features.set(
F::ADDRESS_MODE_CLAMP_TO_BORDER,
Expand Down
2 changes: 2 additions & 0 deletions wgpu-hal/src/metal/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ struct PrivateCapabilities {
can_set_maximum_drawables_count: bool,
can_set_display_sync: bool,
can_set_next_drawable_timeout: bool,
supports_arrays_of_textures: bool,
supports_arrays_of_textures_write: bool,
}

#[derive(Clone, Debug)]
Expand Down
25 changes: 21 additions & 4 deletions wgpu-hal/src/vulkan/adapter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ impl PhysicalDeviceFeatures {
wgt::Features::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
),
)
//.shader_storage_image_array_non_uniform_indexing(
.shader_storage_image_array_non_uniform_indexing(
requested_features.contains(
wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
),
)
//.shader_storage_buffer_array_non_uniform_indexing(
.shader_uniform_buffer_array_non_uniform_indexing(
requested_features
Expand All @@ -164,7 +168,11 @@ impl PhysicalDeviceFeatures {
wgt::Features::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
),
)
//.shader_storage_image_array_non_uniform_indexing(
.shader_storage_image_array_non_uniform_indexing(
requested_features.contains(
wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
),
)
//.shader_storage_buffer_array_non_uniform_indexing(
.shader_uniform_buffer_array_non_uniform_indexing(
requested_features
Expand Down Expand Up @@ -199,6 +207,7 @@ impl PhysicalDeviceFeatures {
| F::PUSH_CONSTANTS
| F::ADDRESS_MODE_CLAMP_TO_BORDER
| F::SAMPLED_TEXTURE_BINDING_ARRAY
| F::STORAGE_TEXTURE_BINDING_ARRAY
| F::BUFFER_BINDING_ARRAY;
let mut dl_flags = Df::all();

Expand Down Expand Up @@ -244,6 +253,10 @@ impl PhysicalDeviceFeatures {
F::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING,
self.core.shader_sampled_image_array_dynamic_indexing != 0,
);
features.set(
F::STORAGE_TEXTURE_ARRAY_DYNAMIC_INDEXING,
self.core.shader_storage_image_array_dynamic_indexing != 0,
);
features.set(
F::STORAGE_BUFFER_ARRAY_DYNAMIC_INDEXING,
self.core.shader_storage_buffer_array_dynamic_indexing != 0,
Expand All @@ -270,7 +283,9 @@ impl PhysicalDeviceFeatures {
if vulkan_1_2.shader_sampled_image_array_non_uniform_indexing != 0 {
features |= F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
}
//if vulkan_1_2.shader_storage_image_array_non_uniform_indexing != 0 {
if vulkan_1_2.shader_storage_image_array_non_uniform_indexing != 0 {
features |= F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
}
//if vulkan_1_2.shader_storage_buffer_array_non_uniform_indexing != 0 {
if vulkan_1_2.shader_uniform_buffer_array_non_uniform_indexing != 0 {
features |= F::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
Expand All @@ -289,7 +304,9 @@ impl PhysicalDeviceFeatures {
if descriptor_indexing.shader_sampled_image_array_non_uniform_indexing != 0 {
features |= F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
}
//if descriptor_indexing.shader_storage_image_array_non_uniform_indexing != 0 {
if descriptor_indexing.shader_storage_image_array_non_uniform_indexing != 0 {
features |= F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
}
//if descriptor_indexing.shader_storage_buffer_array_non_uniform_indexing != 0 {
if descriptor_indexing.shader_uniform_buffer_array_non_uniform_indexing != 0 {
features |= F::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
Expand Down
Loading

0 comments on commit 1d2efb4

Please sign in to comment.