Give Expression::ImageLoad separate sample and level operands.

This commit is contained in:
Jim Blandy
2022-01-13 09:51:22 -08:00
committed by Dzmitry Malyshau
parent 8fd8e7d575
commit 07f9cf670c
14 changed files with 141 additions and 115 deletions

View File

@@ -274,15 +274,19 @@ fn write_fun(
image,
coordinate,
array_index,
index,
sample,
level,
} => {
edges.insert("image", image);
edges.insert("coordinate", coordinate);
if let Some(expr) = array_index {
edges.insert("array_index", expr);
}
if let Some(expr) = index {
edges.insert("index", expr);
if let Some(sample) = sample {
edges.insert("sample", sample);
}
if let Some(level) = level {
edges.insert("level", level);
}
("ImageLoad".into(), 5)
}

View File

@@ -2249,7 +2249,8 @@ impl<'a, W: Write> Writer<'a, W> {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
// This will only panic if the module is invalid
let (dim, class) = match *ctx.info[image].ty.inner_with(&self.module.types) {
@@ -2275,9 +2276,9 @@ impl<'a, W: Write> Writer<'a, W> {
write!(self.out, ", ")?;
self.write_texture_coordinates(coordinate, array_index, dim, ctx)?;
if let Some(index_expr) = index {
if let Some(sample_or_level) = sample.or(level) {
write!(self.out, ", ")?;
self.write_expr(index_expr, ctx)?;
self.write_expr(sample_or_level, ctx)?;
}
write!(self.out, ")")?;
}

View File

@@ -1694,19 +1694,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
// https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-to-load
let (sample, mip_level) = match *func_ctx.info[image].ty.inner_with(&module.types) {
TypeInner::Image { class, .. } => match class {
crate::ImageClass::Sampled { multi: true, .. }
| crate::ImageClass::Depth { multi: true } => (index, None),
crate::ImageClass::Storage { .. } => (None, None),
_ => (None, index),
},
_ => (None, None),
};
self.write_expr(module, image, func_ctx)?;
write!(self.out, ".Load(")?;
@@ -1714,7 +1705,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
"int",
coordinate,
array_index,
mip_level,
level,
module,
func_ctx,
)?;

View File

@@ -1037,7 +1037,8 @@ impl<W: Write> Writer<W> {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
self.put_expression(image, context, false)?;
write!(self.out, ".read(")?;
@@ -1046,7 +1047,11 @@ impl<W: Write> Writer<W> {
write!(self.out, ", ")?;
self.put_expression(expr, context, true)?;
}
if let Some(index) = index {
if let Some(sample) = sample {
write!(self.out, ", ")?;
self.put_expression(sample, context, true)?
}
if let Some(level) = level {
// Metal requires that the `level` argument to
// `texture1d::read` be a constexpr equal to zero.
if let crate::TypeInner::Image {
@@ -1057,7 +1062,7 @@ impl<W: Write> Writer<W> {
// The argument defaults to zero.
} else {
write!(self.out, ", ")?;
self.put_expression(index, context, true)?
self.put_expression(level, context, true)?
}
}
write!(self.out, ")")?;

View File

@@ -915,10 +915,17 @@ impl<'w> BlockContext<'w> {
image,
coordinate,
array_index,
index,
} => {
self.write_image_load(result_type_id, image, coordinate, array_index, index, block)?
}
sample,
level,
} => self.write_image_load(
result_type_id,
image,
coordinate,
array_index,
level,
sample,
block,
)?,
crate::Expression::ImageSample {
image,
sampler,

View File

@@ -709,13 +709,15 @@ impl<'w> BlockContext<'w> {
/// Generate code for an `ImageLoad` expression.
///
/// The arguments are the components of an `Expression::ImageLoad` variant.
#[allow(clippy::too_many_arguments)]
pub(super) fn write_image_load(
&mut self,
result_type_id: Word,
image: Handle<crate::Expression>,
coordinate: Handle<crate::Expression>,
array_index: Option<Handle<crate::Expression>>,
level_or_sample: Option<Handle<crate::Expression>>,
level: Option<Handle<crate::Expression>>,
sample: Option<Handle<crate::Expression>>,
block: &mut Block,
) -> Result<Word, Error> {
let image_id = self.get_image_id(image);
@@ -728,18 +730,8 @@ impl<'w> BlockContext<'w> {
let access = Load::from_image_expr(self, image_id, image_class, result_type_id)?;
let coordinates = self.write_image_coordinates(coordinate, array_index, block)?;
// Figure out what the `level_or_sample` operand really means.
let level_or_sample_id = level_or_sample.map(|i| self.cached[i]);
let (level_id, sample_id) = match image_class {
crate::ImageClass::Sampled { multi, .. } | crate::ImageClass::Depth { multi } => {
if multi {
(None, level_or_sample_id)
} else {
(level_or_sample_id, None)
}
}
crate::ImageClass::Storage { .. } => (None, None),
};
let level_id = level.map(|expr| self.cached[expr]);
let sample_id = sample.map(|expr| self.cached[expr]);
// Perform the access, according to the bounds check policy.
let access_id = match self.writer.bounds_check_policies.image {

View File

@@ -1335,7 +1335,8 @@ impl<W: Write> Writer<W> {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
write!(self.out, "textureLoad(")?;
self.write_expr(module, image, func_ctx)?;
@@ -1345,7 +1346,7 @@ impl<W: Write> Writer<W> {
write!(self.out, ", ")?;
self.write_expr(module, array_index, func_ctx)?;
}
if let Some(index) = index {
if let Some(index) = sample.or(level) {
write!(self.out, ", ")?;
self.write_expr(module, index, func_ctx)?;
}

View File

@@ -350,7 +350,7 @@ pub fn inject_builtin(declaration: &mut FunctionDeclaration, module: &mut Module
texture_args_generator(TextureArgsOptions::all(), f)
}
"texelFetch" => {
let f = |kind, dim, arrayed, multi, _| {
let f = |kind, dim, arrayed, multi, _shadow| {
// Cube images aren't supported
if let Dim::Cube = dim {
return;
@@ -376,7 +376,7 @@ pub fn inject_builtin(declaration: &mut FunctionDeclaration, module: &mut Module
declaration
.overloads
.push(module.add_builtin(args, MacroCall::ImageLoad))
.push(module.add_builtin(args, MacroCall::ImageLoad { multi }))
};
// Don't generate shadow images since they aren't supported
@@ -439,7 +439,7 @@ pub fn inject_builtin(declaration: &mut FunctionDeclaration, module: &mut Module
declaration
.overloads
.push(module.add_builtin(args, MacroCall::ImageLoad))
.push(module.add_builtin(args, MacroCall::ImageLoad { multi: false }))
};
// Don't generate shadow nor multisampled images since they aren't supported
@@ -1537,7 +1537,9 @@ pub enum MacroCall {
TextureSize {
arrayed: bool,
},
ImageLoad,
ImageLoad {
multi: bool,
},
ImageStore,
MathFunction(MathFunction),
BitfieldExtract,
@@ -1760,15 +1762,21 @@ impl MacroCall {
expr
}
MacroCall::ImageLoad => {
MacroCall::ImageLoad { multi } => {
let comps =
parser.coordinate_components(ctx, args[0], args[1], None, meta, body)?;
let (sample, level) = match (multi, args.get(2)) {
(_, None) => (None, None),
(true, Some(&arg)) => (Some(arg), None),
(false, Some(&arg)) => (None, Some(arg)),
};
ctx.add_expression(
Expression::ImageLoad {
image: args[0],
coordinate: comps.coordinate,
array_index: comps.array_index,
index: args.get(2).copied(),
sample,
level,
},
Span::default(),
body,

View File

@@ -332,7 +332,8 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
0
};
let mut index = None;
let mut sample = None;
let mut level = None;
while image_ops != 0 {
let bit = 1 << image_ops.trailing_zeros();
match spirv::ImageOperands::from_bits_truncate(bit) {
@@ -341,13 +342,13 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
let lod_lexp = self.lookup_expression.lookup(lod_expr)?;
let lod_handle =
self.get_expr_handle(lod_expr, lod_lexp, ctx, emitter, block, body_idx);
index = Some(lod_handle);
level = Some(lod_handle);
words_left -= 1;
}
spirv::ImageOperands::SAMPLE => {
let sample_expr = self.next()?;
let sample_handle = self.lookup_expression.lookup(sample_expr)?.handle;
index = Some(sample_handle);
sample = Some(sample_handle);
words_left -= 1;
}
other => {
@@ -393,7 +394,8 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
image: image_lexp.handle,
coordinate,
array_index,
index,
sample,
level,
};
self.lookup_expression.insert(
result_id,

View File

@@ -1953,20 +1953,25 @@ impl Parser {
} else {
None
};
let index = match class {
crate::ImageClass::Storage { .. } => None,
// it's the MSAA index for multi-sampled, and LOD for the others
crate::ImageClass::Sampled { .. } | crate::ImageClass::Depth { .. } => {
lexer.expect(Token::Separator(','))?;
Some(self.parse_general_expression(lexer, ctx.reborrow())?)
}
let level = if class.is_mipmapped() {
lexer.expect(Token::Separator(','))?;
Some(self.parse_general_expression(lexer, ctx.reborrow())?)
} else {
None
};
let sample = if class.is_multisampled() {
lexer.expect(Token::Separator(','))?;
Some(self.parse_general_expression(lexer, ctx.reborrow())?)
} else {
None
};
lexer.close_arguments()?;
crate::Expression::ImageLoad {
image,
coordinate,
array_index,
index,
sample,
level,
}
}
"textureDimensions" => {

View File

@@ -1234,37 +1234,22 @@ pub enum Expression {
/// [`Sint`]: ScalarKind::Sint
array_index: Option<Handle<Expression>>,
/// The sample within a particular texel.
/// A sample index, for multisampled [`Sampled`] and [`Depth`] images.
///
/// The meaning of this value depends on the [`class`] of `image`:
///
/// - [`Storage`] images hold exactly one sample per texel, so `index` must
/// be `None`.
///
/// - [`Depth`] and [`Sampled`] images may be multisampled or have
/// mipmaps, but not both. Which one is indicated by the variant's
/// [`multi`] field:
///
/// - If `multi` is `true`, then the image has multiple samples per
/// texel, and `index` must be `Some(sample)`, where `sample` is
/// the index of the sample to retrieve.
///
/// - If `multi` is `false`, then the image may have mipmaps. In
/// this case, `index` must be `Some(level)`, where `level`
/// identifies the level of detail. Even if the image has only the
/// full-sized version, `level` must still be present; its only
/// in-range value is zero.
///
/// When `index` is `Some` the value must be a `Sint` scalar value. If
/// it identifes a level of detail, zero represents the full resolution
/// mipmap.
///
/// [`class`]: TypeInner::Image::class
/// [`Sampled`]: ImageClass::Sampled
/// [`Storage`]: ImageClass::Storage
/// [`Depth`]: ImageClass::Depth
/// [`multi`]: ImageClass::Sampled::multi
index: Option<Handle<Expression>>,
sample: Option<Handle<Expression>>,
/// A level of detail, for mipmapped images.
///
/// This must be present when accessing non-multisampled
/// [`Sampled`] and [`Depth`] images, even if only the
/// full-resolution level is present (in which case the only
/// valid level is zero).
///
/// [`Sampled`]: ImageClass::Sampled
/// [`Depth`]: ImageClass::Depth
level: Option<Handle<Expression>>,
},
/// Query information from an image.
@@ -1476,7 +1461,8 @@ pub enum Statement {
/// The `image`, `coordinate`, and `array_index` fields have the same
/// meanings as the corresponding operands of an [`ImageLoad`] expression;
/// see that documentation for details. Storing into multisampled images or
/// images with mipmaps is not supported, so there is no `index`operand.
/// images with mipmaps is not supported, so there are no `level` or
/// `sample` operands.
///
/// This statement is a barrier for any operations on the corresponding
/// [`Expression::GlobalVariable`] for this image.

View File

@@ -441,6 +441,22 @@ impl super::SwizzleComponent {
}
}
impl super::ImageClass {
pub fn is_multisampled(self) -> bool {
match self {
crate::ImageClass::Sampled { multi, .. } | crate::ImageClass::Depth { multi } => multi,
crate::ImageClass::Storage { .. } => false,
}
}
pub fn is_mipmapped(self) -> bool {
match self {
crate::ImageClass::Sampled { multi, .. } | crate::ImageClass::Depth { multi } => !multi,
crate::ImageClass::Storage { .. } => false,
}
}
}
#[test]
fn test_matrix_size() {
let constants = crate::Arena::new();

View File

@@ -557,16 +557,19 @@ impl FunctionInfo {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
let array_nur = array_index.and_then(|h| self.add_ref(h));
let index_nur = index.and_then(|h| self.add_ref(h));
let sample_nur = sample.and_then(|h| self.add_ref(h));
let level_nur = level.and_then(|h| self.add_ref(h));
Uniformity {
non_uniform_result: self
.add_ref(image)
.or(self.add_ref(coordinate))
.or(array_nur)
.or(index_nur),
.or(sample_nur)
.or(level_nur),
requirements: UniformityRequirements::empty(),
}
}

View File

@@ -75,11 +75,11 @@ pub enum ExpressionError {
InvalidDerivative,
#[error("Image array index parameter is misplaced")]
InvalidImageArrayIndex,
#[error("Image other index parameter is misplaced")]
#[error("Inappropriate sample or level-of-detail index for texel access")]
InvalidImageOtherIndex,
#[error("Image array index type of {0:?} is not an integer scalar")]
InvalidImageArrayIndexType(Handle<crate::Expression>),
#[error("Image other index type of {0:?} is not an integer scalar")]
#[error("Image sample or level-of-detail index's type of {0:?} is not an integer scalar")]
InvalidImageOtherIndexType(Handle<crate::Expression>),
#[error("Image coordinate type of {1:?} does not match dimension {0:?}")]
InvalidImageCoordinateType(crate::ImageDimension, Handle<crate::Expression>),
@@ -537,7 +537,8 @@ impl super::Validator {
image,
coordinate,
array_index,
index,
sample,
level,
} => {
let ty = match function.expressions[image] {
crate::Expression::GlobalVariable(var_handle) => {
@@ -560,16 +561,9 @@ impl super::Validator {
))
}
};
let needs_index = match class {
crate::ImageClass::Storage { .. } => false,
_ => true,
};
if arrayed != array_index.is_some() {
return Err(ExpressionError::InvalidImageArrayIndex);
}
if needs_index != index.is_some() {
return Err(ExpressionError::InvalidImageOtherIndex);
}
if let Some(expr) = array_index {
match *resolver.resolve(expr)? {
Ti::Scalar {
@@ -579,13 +573,30 @@ impl super::Validator {
_ => return Err(ExpressionError::InvalidImageArrayIndexType(expr)),
}
}
if let Some(expr) = index {
match *resolver.resolve(expr)? {
Ti::Scalar {
kind: Sk::Sint,
width: _,
} => {}
_ => return Err(ExpressionError::InvalidImageOtherIndexType(expr)),
match (sample, class.is_multisampled()) {
(None, false) => {}
(Some(sample), true) => {
if resolver.resolve(sample)?.scalar_kind() != Some(Sk::Sint) {
return Err(ExpressionError::InvalidImageOtherIndexType(
sample,
));
}
}
_ => {
return Err(ExpressionError::InvalidImageOtherIndex);
}
}
match (level, class.is_mipmapped()) {
(None, false) => {}
(Some(level), true) => {
if resolver.resolve(level)?.scalar_kind() != Some(Sk::Sint) {
return Err(ExpressionError::InvalidImageOtherIndexType(level));
}
}
_ => {
return Err(ExpressionError::InvalidImageOtherIndex);
}
}
}
@@ -603,18 +614,12 @@ impl super::Validator {
};
match module.types[ty].inner {
Ti::Image { class, arrayed, .. } => {
let can_level = match class {
crate::ImageClass::Sampled { multi, .. } => !multi,
crate::ImageClass::Depth { multi } => !multi,
crate::ImageClass::Storage { .. } => false,
};
let good = match query {
crate::ImageQuery::NumLayers => arrayed,
crate::ImageQuery::Size { level: None } => true,
crate::ImageQuery::Size { level: Some(_) }
| crate::ImageQuery::NumLevels => can_level,
//TODO: forbid on storage images
crate::ImageQuery::NumSamples => !can_level,
| crate::ImageQuery::NumLevels => class.is_mipmapped(),
crate::ImageQuery::NumSamples => class.is_multisampled(),
};
if !good {
return Err(ExpressionError::InvalidImageClass(class));