Skip to content

Commit

Permalink
[wgsl] Storage buffer/texture access (gfx-rs#1142)
Browse files Browse the repository at this point in the history 
* Resurrect texture_storage_* tests
* Test parsing of `var<storage,write>`
* Default storage textures to READ
* Restore default features
* Fix glsl/hlsl/msl/spv front and back ends
* Add missing test outputs
* All-around fixes for the storage access

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
  • Loading branch information
@525c1e21-bd67-4735-ac99-b4b0e5262290 @kvark @JCapucho
2 people authored and JCapucho committed Jul 30, 2021
1 parent 8185ccc commit de0647a
Show file tree
Hide file tree
Showing 43 changed files with 652 additions and 435 deletions.
4 changes: 2 additions & 2 deletions src/back/glsl/features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl<'a, W> Writer<'a, W> {
self.features.request(Features::MULTISAMPLED_TEXTURE_ARRAYS);
}
}
ImageClass::Storage(format) => match format {
ImageClass::Storage { format, .. } => match format {
StorageFormat::R8Unorm
| StorageFormat::R8Snorm
| StorageFormat::R8Uint
Expand Down Expand Up @@ -336,7 +336,7 @@ impl<'a, W> Writer<'a, W> {
}
match global.class {
StorageClass::WorkGroup => self.features.request(Features::COMPUTE_SHADER),
StorageClass::Storage => self.features.request(Features::BUFFER_STORAGE),
StorageClass::Storage { .. } => self.features.request(Features::BUFFER_STORAGE),
StorageClass::PushConstant => return Err(Error::PushConstantNotSupported),
_ => {}
}
Expand Down
67 changes: 34 additions & 33 deletions src/back/glsl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,11 +497,11 @@ impl<'a, W: Write> Writer<'a, W> {
class,
} => {
// Gather the storage format if needed
let layout_storage_format = match self.module.types[global.ty].inner {
let storage_format_access = match self.module.types[global.ty].inner {
TypeInner::Image {
class: crate::ImageClass::Storage(format),
class: crate::ImageClass::Storage { format, access },
..
} => Some(glsl_storage_format(format)),
} => Some((format, access)),
_ => None,
};
// Gether the location if needed
Expand All @@ -513,23 +513,24 @@ impl<'a, W: Write> Writer<'a, W> {
};

// Write all the layout qualifiers
if layout_binding.is_some() || layout_storage_format.is_some() {
if layout_binding.is_some() || storage_format_access.is_some() {
write!(self.out, "layout(")?;
if let Some(binding) = layout_binding {
write!(self.out, "binding = {}", binding)?;
}
if let Some(format) = layout_storage_format {
if let Some((format, _)) = storage_format_access {
let format_str = glsl_storage_format(format);
let separator = match layout_binding {
Some(_) => ",",
None => "",
};
write!(self.out, "{}{}", separator, format)?;
write!(self.out, "{}{}", separator, format_str)?;
}
write!(self.out, ") ")?;
}

if let Some(storage_access) = glsl_storage_access(global.storage_access) {
write!(self.out, "{} ", storage_access)?;
if let Some((_, access)) = storage_format_access {
self.write_storage_access(access)?;
}

// All images in glsl are `uniform`
Expand Down Expand Up @@ -760,7 +761,7 @@ impl<'a, W: Write> Writer<'a, W> {
Ic::Sampled { kind, multi: false } => ("sampler", kind, "", ""),
Ic::Depth { multi: true } => ("sampler", crate::ScalarKind::Float, "MS", ""),
Ic::Depth { multi: false } => ("sampler", crate::ScalarKind::Float, "", "Shadow"),
Ic::Storage(format) => ("image", format.into(), "", ""),
Ic::Storage { format, .. } => ("image", format.into(), "", ""),
};

write!(
Expand Down Expand Up @@ -798,8 +799,8 @@ impl<'a, W: Write> Writer<'a, W> {
}
}

if let Some(storage_access) = glsl_storage_access(global.storage_access) {
write!(self.out, "{} ", storage_access)?;
if let crate::StorageClass::Storage { access } = global.class {
self.write_storage_access(access)?;
}

// Write the storage class
Expand Down Expand Up @@ -1013,7 +1014,7 @@ impl<'a, W: Write> Writer<'a, W> {
} => {
// Write the storage format if needed
if let TypeInner::Image {
class: crate::ImageClass::Storage(format),
class: crate::ImageClass::Storage { format, .. },
..
} = this.module.types[arg.ty].inner
{
Expand Down Expand Up @@ -1939,7 +1940,7 @@ impl<'a, W: Write> Writer<'a, W> {

let fun_name = match class {
crate::ImageClass::Sampled { .. } => "texelFetch",
crate::ImageClass::Storage(_) => "imageLoad",
crate::ImageClass::Storage { .. } => "imageLoad",
// TODO: Is there even a function for this?
crate::ImageClass::Depth { multi: _ } => {
return Err(Error::Custom("TODO: depth sample loads".to_string()))
Expand Down Expand Up @@ -1992,7 +1993,7 @@ impl<'a, W: Write> Writer<'a, W> {
write!(self.out, "0",)?;
}
}
ImageClass::Storage(_) => {
ImageClass::Storage { .. } => {
write!(self.out, "imageSize(")?;
self.write_expr(image, ctx)?;
}
Expand All @@ -2007,7 +2008,7 @@ impl<'a, W: Write> Writer<'a, W> {
crate::ImageQuery::NumLayers => {
let fun_name = match class {
ImageClass::Sampled { .. } | ImageClass::Depth { .. } => "textureSize",
ImageClass::Storage(_) => "imageSize",
ImageClass::Storage { .. } => "imageSize",
};
write!(self.out, "{}(", fun_name)?;
self.write_expr(image, ctx)?;
Expand All @@ -2019,7 +2020,7 @@ impl<'a, W: Write> Writer<'a, W> {
ImageClass::Sampled { .. } | ImageClass::Depth { .. } => {
"textureSamples"
}
ImageClass::Storage(_) => "imageSamples",
ImageClass::Storage { .. } => "imageSamples",
};
write!(self.out, "{}(", fun_name)?;
self.write_expr(image, ctx)?;
Expand Down Expand Up @@ -2417,6 +2418,21 @@ impl<'a, W: Write> Writer<'a, W> {
Ok(())
}

/// Helper function that return the glsl storage access string of [`StorageAccess`](crate::StorageAccess)
///
/// glsl allows adding both `readonly` and `writeonly` but this means that
/// they can only be used to query information about the resource which isn't what
/// we want here so when storage access is both `LOAD` and `STORE` add no modifiers
fn write_storage_access(&mut self, storage_access: crate::StorageAccess) -> BackendResult {
if !storage_access.contains(crate::StorageAccess::STORE) {
write!(self.out, "readonly ")?;
}
if !storage_access.contains(crate::StorageAccess::LOAD) {
write!(self.out, "writeonly ")?;
}
Ok(())
}

/// Helper method used to produce the reflection info that's returned to the user
///
/// It takes an iterator of [`Function`](crate::Function) references instead of
Expand Down Expand Up @@ -2454,7 +2470,7 @@ impl<'a, W: Write> Writer<'a, W> {
}
match self.module.types[var.ty].inner {
crate::TypeInner::Struct { .. } => match var.class {
crate::StorageClass::Uniform | crate::StorageClass::Storage => {
crate::StorageClass::Uniform | crate::StorageClass::Storage { .. } => {
let name = self.reflection_names[&var.ty].clone();
uniforms.insert(handle, name);
}
Expand Down Expand Up @@ -2568,7 +2584,7 @@ fn glsl_storage_class(class: crate::StorageClass) -> Option<&'static str> {
match class {
Sc::Function => None,
Sc::Private => None,
Sc::Storage => Some("buffer"),
Sc::Storage { .. } => Some("buffer"),
Sc::Uniform => Some("uniform"),
Sc::Handle => Some("uniform"),
Sc::WorkGroup => Some("shared"),
Expand Down Expand Up @@ -2650,21 +2666,6 @@ fn glsl_storage_format(format: crate::StorageFormat) -> &'static str {
}
}

/// Helper function that return the glsl storage access string of [`StorageAccess`](crate::StorageAccess)
///
/// glsl allows adding both `readonly` and `writeonly` but this means that
/// they can only be used to query information about the resource which isn't what
/// we want here so when storage access is both `LOAD` and `STORE` add no modifiers
fn glsl_storage_access(storage_access: crate::StorageAccess) -> Option<&'static str> {
if storage_access == crate::StorageAccess::LOAD {
Some("readonly")
} else if storage_access == crate::StorageAccess::STORE {
Some("writeonly")
} else {
None
}
}

fn is_value_init_supported(module: &crate::Module, ty: Handle<crate::Type>) -> bool {
match module.types[ty].inner {
TypeInner::Scalar { .. } | TypeInner::Vector { .. } | TypeInner::Matrix { .. } => true,
Expand Down
63 changes: 39 additions & 24 deletions src/back/hlsl/help.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,13 @@ impl<'a, W: Write> super::Writer<'a, W> {
arrayed: bool,
class: crate::ImageClass,
) -> BackendResult {
let access_str = match class {
crate::ImageClass::Storage { .. } => "RW",
_ => "",
};
let dim_str = dim.to_hlsl_str();
let arrayed_str = if arrayed { "Array" } else { "" };
write!(self.out, "Texture{}{}", dim_str, arrayed_str)?;
write!(self.out, "{}Texture{}{}", access_str, dim_str, arrayed_str)?;
match class {
crate::ImageClass::Depth { multi } => {
let multi_str = if multi { "MS" } else { "" };
Expand All @@ -118,7 +122,7 @@ impl<'a, W: Write> super::Writer<'a, W> {
let scalar_kind_str = kind.to_hlsl_str(4)?;
write!(self.out, "{}<{}4>", multi_str, scalar_kind_str)?
}
crate::ImageClass::Storage(format) => {
crate::ImageClass::Storage { format, .. } => {
let storage_format_str = format.to_hlsl_str();
write!(self.out, "<{}>", storage_format_str)?
}
Expand Down Expand Up @@ -195,9 +199,8 @@ impl<'a, W: Write> super::Writer<'a, W> {
crate::ImageClass::Sampled { multi: true, .. } => "MS",
crate::ImageClass::Depth { multi: true } => "DepthMS",
crate::ImageClass::Depth { multi: false } => "Depth",
crate::ImageClass::Sampled { multi: false, .. } | crate::ImageClass::Storage { .. } => {
""
}
crate::ImageClass::Sampled { multi: false, .. } => "",
crate::ImageClass::Storage { .. } => "RW",
};
let arrayed_str = if query.arrayed { "Array" } else { "" };
let query_str = match query.query {
Expand Down Expand Up @@ -226,7 +229,10 @@ impl<'a, W: Write> super::Writer<'a, W> {
expr_handle: Handle<crate::Expression>,
func_ctx: &FunctionCtx,
) -> BackendResult {
use crate::{back::INDENT, ImageDimension as IDim};
use crate::{
back::{COMPONENTS, INDENT},
ImageDimension as IDim,
};

const ARGUMENT_VARIABLE_NAME: &str = "texture";
const RETURN_VARIABLE_NAME: &str = "ret";
Expand All @@ -253,15 +259,24 @@ impl<'a, W: Write> super::Writer<'a, W> {
writeln!(self.out, "{{")?;

let array_coords = if wiq.arrayed { 1 } else { 0 };
// extra parameter is the mip level count or the sample count
let extra_coords = match wiq.class {
crate::ImageClass::Storage { .. } => 0,
crate::ImageClass::Sampled { .. } | crate::ImageClass::Depth { .. } => 1,
};

// GetDimensions Overloaded Methods
// https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-to-getdimensions#overloaded-methods
let (ret_swizzle, number_of_params) = match wiq.query {
ImageQuery::Size | ImageQuery::SizeLevel => match wiq.dim {
IDim::D1 => ("x", 1 + array_coords),
IDim::D2 => ("xy", 3 + array_coords),
IDim::D3 => ("xyz", 4),
IDim::Cube => ("xy", 3 + array_coords),
},
ImageQuery::Size | ImageQuery::SizeLevel => {
let ret = match wiq.dim {
IDim::D1 => "x",
IDim::D2 => "xy",
IDim::D3 => "xyz",
IDim::Cube => "xy",
};
(ret, ret.len() + array_coords + extra_coords)
}
ImageQuery::NumLevels | ImageQuery::NumSamples | ImageQuery::NumLayers => {
if wiq.arrayed || wiq.dim == IDim::D3 {
("w", 4)
Expand All @@ -284,18 +299,16 @@ impl<'a, W: Write> super::Writer<'a, W> {
}
_ => match wiq.class {
crate::ImageClass::Sampled { multi: true, .. }
| crate::ImageClass::Depth { multi: true } => {}
_ => match wiq.dim {
| crate::ImageClass::Depth { multi: true }
| crate::ImageClass::Storage { .. } => {}
_ => {
// Write zero mipmap level for supported types
IDim::D2 | IDim::D3 | IDim::Cube => {
write!(self.out, "0, ")?;
}
IDim::D1 => {}
},
write!(self.out, "0, ")?;
}
},
}

for component in crate::back::COMPONENTS[..number_of_params - 1].iter() {
for component in COMPONENTS[..number_of_params - 1].iter() {
write!(self.out, "{}.{}, ", RETURN_VARIABLE_NAME, component)?;
}

Expand All @@ -304,7 +317,7 @@ impl<'a, W: Write> super::Writer<'a, W> {
self.out,
"{}.{}",
RETURN_VARIABLE_NAME,
crate::back::COMPONENTS[number_of_params - 1]
COMPONENTS[number_of_params - 1]
)?;

writeln!(self.out, ");")?;
Expand Down Expand Up @@ -344,10 +357,12 @@ impl<'a, W: Write> super::Writer<'a, W> {
}
ref other => unreachable!("Array length of base {:?}", other),
};
let storage_access = match global_var.class {
crate::StorageClass::Storage { access } => access,
_ => crate::StorageAccess::default(),
};
let wal = WrappedArrayLength {
writable: global_var
.storage_access
.contains(crate::StorageAccess::STORE),
writable: storage_access.contains(crate::StorageAccess::STORE),
};

if !self.wrapped_array_lengths.contains(&wal) {
Expand Down
Loading

0 comments on commit de0647a

Please sign in to comment.