diff --git a/src/back/dot/mod.rs b/src/back/dot/mod.rs index f24d67df7b..523e48ce5f 100644 --- a/src/back/dot/mod.rs +++ b/src/back/dot/mod.rs @@ -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) } diff --git a/src/back/glsl/mod.rs b/src/back/glsl/mod.rs index 20d0035037..b765c03417 100644 --- a/src/back/glsl/mod.rs +++ b/src/back/glsl/mod.rs @@ -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, ")")?; } diff --git a/src/back/hlsl/writer.rs b/src/back/hlsl/writer.rs index e2fccb0e13..35b76f3fc4 100644 --- a/src/back/hlsl/writer.rs +++ b/src/back/hlsl/writer.rs @@ -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, )?; diff --git a/src/back/msl/writer.rs b/src/back/msl/writer.rs index d0dea3ef17..32e003c656 100644 --- a/src/back/msl/writer.rs +++ b/src/back/msl/writer.rs @@ -1037,7 +1037,8 @@ impl Writer { image, coordinate, array_index, - index, + sample, + level, } => { self.put_expression(image, context, false)?; write!(self.out, ".read(")?; @@ -1046,7 +1047,11 @@ impl Writer { 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 Writer { // The argument defaults to zero. } else { write!(self.out, ", ")?; - self.put_expression(index, context, true)? + self.put_expression(level, context, true)? } } write!(self.out, ")")?; diff --git a/src/back/spv/block.rs b/src/back/spv/block.rs index c4ad830679..0ab9c4a71f 100644 --- a/src/back/spv/block.rs +++ b/src/back/spv/block.rs @@ -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, diff --git a/src/back/spv/image.rs b/src/back/spv/image.rs index 2ce3a30b49..1eb5ab3b9c 100644 --- a/src/back/spv/image.rs +++ b/src/back/spv/image.rs @@ -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, coordinate: Handle, array_index: Option>, - level_or_sample: Option>, + level: Option>, + sample: Option>, block: &mut Block, ) -> Result { 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 { diff --git a/src/back/wgsl/writer.rs b/src/back/wgsl/writer.rs index fa32c6b6d8..d2eecd8cc8 100644 --- a/src/back/wgsl/writer.rs +++ b/src/back/wgsl/writer.rs @@ -1335,7 +1335,8 @@ impl Writer { image, coordinate, array_index, - index, + sample, + level, } => { write!(self.out, "textureLoad(")?; self.write_expr(module, image, func_ctx)?; @@ -1345,7 +1346,7 @@ impl Writer { 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)?; } diff --git a/src/front/glsl/builtins.rs b/src/front/glsl/builtins.rs index 17eef65796..71e1f3be6b 100644 --- a/src/front/glsl/builtins.rs +++ b/src/front/glsl/builtins.rs @@ -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, diff --git a/src/front/spv/image.rs b/src/front/spv/image.rs index e797a516a1..c45288ab15 100644 --- a/src/front/spv/image.rs +++ b/src/front/spv/image.rs @@ -332,7 +332,8 @@ impl> super::Parser { 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> super::Parser { 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> super::Parser { image: image_lexp.handle, coordinate, array_index, - index, + sample, + level, }; self.lookup_expression.insert( result_id, diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index 6d2f0fa46e..164d5aa708 100644 --- a/src/front/wgsl/mod.rs +++ b/src/front/wgsl/mod.rs @@ -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" => { diff --git a/src/lib.rs b/src/lib.rs index ac1118c060..9873d23305 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1234,37 +1234,22 @@ pub enum Expression { /// [`Sint`]: ScalarKind::Sint array_index: Option>, - /// 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>, + sample: Option>, + + /// 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>, }, /// 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. diff --git a/src/proc/mod.rs b/src/proc/mod.rs index c62e587764..15cc3a7fc5 100644 --- a/src/proc/mod.rs +++ b/src/proc/mod.rs @@ -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(); diff --git a/src/valid/analyzer.rs b/src/valid/analyzer.rs index b95a8a3484..28a0f8a8ab 100644 --- a/src/valid/analyzer.rs +++ b/src/valid/analyzer.rs @@ -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(), } } diff --git a/src/valid/expression.rs b/src/valid/expression.rs index 1cba9e9e4c..7fb64ab988 100644 --- a/src/valid/expression.rs +++ b/src/valid/expression.rs @@ -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), - #[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), #[error("Image coordinate type of {1:?} does not match dimension {0:?}")] InvalidImageCoordinateType(crate::ImageDimension, Handle), @@ -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));