Skip to content

Commit

Permalink
[wgsl-in] Ensure textureSampleLevel's level argument is an integer fo…
Browse files Browse the repository at this point in the history
…r depth textures

Until now we accepted a float, as is the case for non-depth textures.
This makes us compliant with the spec.

The validator is updated to expect an Sint or Uint when the ImageClass
is ImageClass::Depth. The SPIR-V frontend converts the LOD argument
from float to Sint (assuming that it is representable), likewise The
SPIR-V backend now converts the LOD from either Sint or Uint to
Float. HLSL and MSL backends require no changes as they implicitly do
that conversion. GLSL does not support non-compare LOD samples,
therefore no changes are required.
  • Loading branch information
jamienicol committed Nov 14, 2024
1 parent 693eaaf commit 4b56262
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 15 deletions.
38 changes: 37 additions & 1 deletion naga/src/back/spv/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,43 @@ impl<'w> BlockContext<'w> {
depth_id,
);

let lod_id = self.cached[lod_handle];
let mut lod_id = self.cached[lod_handle];
// SPIR-V expects the LOD to be a float for all image classes.
// lod_id, however, will be an integer for depth images,
// therefore we must do a conversion.
if matches!(
self.ir_module.types[image_type].inner,
crate::TypeInner::Image {
class: crate::ImageClass::Depth { .. },
..
}
) {
let lod_f32_id = self.gen_id();
let f32_type_id = self.get_type_id(LookupType::Local(LocalType::Numeric(
NumericType::Scalar(crate::Scalar::F32),
)));
let convert_op = match *self.fun_info[lod_handle]
.ty
.inner_with(&self.ir_module.types)
{
crate::TypeInner::Scalar(crate::Scalar {
kind: crate::ScalarKind::Uint,
width: 4,
}) => spirv::Op::ConvertUToF,
crate::TypeInner::Scalar(crate::Scalar {
kind: crate::ScalarKind::Sint,
width: 4,
}) => spirv::Op::ConvertSToF,
_ => unreachable!(),
};
block.body.push(Instruction::unary(
convert_op,
f32_type_id,
lod_f32_id,
lod_id,
));
lod_id = lod_f32_id;
}
mask |= spirv::ImageOperands::LOD;
inst.add_operand(mask.bits());
inst.add_operand(lod_id);
Expand Down
27 changes: 27 additions & 0 deletions naga/src/front/spv/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ impl<'function> super::BlockContext<'function> {
match self.expressions[handle] {
crate::Expression::GlobalVariable(handle) => Ok(self.global_arena[handle].ty),
crate::Expression::FunctionArgument(i) => Ok(self.arguments[i as usize].ty),
crate::Expression::Access { base, .. } => Ok(self.get_image_expr_ty(base)?),
ref other => Err(Error::InvalidImageExpression(other.clone())),
}
}
Expand Down Expand Up @@ -460,6 +461,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
} else {
None
};
let span = self.span_from_with_op(start);

let mut image_ops = if words_left != 0 {
words_left -= 1;
Expand All @@ -486,9 +488,34 @@ impl<I: Iterator<Item = u32>> super::Frontend<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);

let is_depth_image = {
let image_lexp = self.lookup_sampled_image.lookup(sampled_image_id)?;
let image_ty = ctx.get_image_expr_ty(image_lexp.image)?;
matches!(
ctx.type_arena[image_ty].inner,
crate::TypeInner::Image {
class: crate::ImageClass::Depth { .. },
..
}
)
};

level = if options.compare {
log::debug!("Assuming {:?} is zero", lod_handle);
crate::SampleLevel::Zero
} else if is_depth_image {
log::debug!(
"Assuming level {:?} converts losslessly to an integer",
lod_handle
);
let expr = crate::Expression::As {
expr: lod_handle,
kind: crate::ScalarKind::Sint,
convert: Some(4),
};
let s32_lod_handle = ctx.expressions.append(expr, span);
crate::SampleLevel::Exact(s32_lod_handle)
} else {
crate::SampleLevel::Exact(lod_handle)
};
Expand Down
25 changes: 19 additions & 6 deletions naga/src/valid/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub enum ExpressionError {
InvalidGatherComponent(crate::SwizzleComponent),
#[error("Gather can't be done for image dimension {0:?}")]
InvalidGatherDimension(crate::ImageDimension),
#[error("Sample level (exact) type {0:?} is not a scalar float")]
#[error("Sample level (exact) type {0:?} has an invalid type")]
InvalidSampleLevelExactType(Handle<crate::Expression>),
#[error("Sample level (bias) type {0:?} is not a scalar float")]
InvalidSampleLevelBiasType(Handle<crate::Expression>),
Expand Down Expand Up @@ -530,11 +530,24 @@ impl super::Validator {
crate::SampleLevel::Auto => ShaderStages::FRAGMENT,
crate::SampleLevel::Zero => ShaderStages::all(),
crate::SampleLevel::Exact(expr) => {
match resolver[expr] {
Ti::Scalar(Sc {
kind: Sk::Float, ..
}) => {}
_ => return Err(ExpressionError::InvalidSampleLevelExactType(expr)),
match class {
crate::ImageClass::Depth { .. } => match resolver[expr] {
Ti::Scalar(Sc {
kind: Sk::Sint | Sk::Uint,
..
}) => {}
_ => {
return Err(ExpressionError::InvalidSampleLevelExactType(expr))
}
},
_ => match resolver[expr] {
Ti::Scalar(Sc {
kind: Sk::Float, ..
}) => {}
_ => {
return Err(ExpressionError::InvalidSampleLevelExactType(expr))
}
},
}
ShaderStages::all()
}
Expand Down
4 changes: 3 additions & 1 deletion naga/tests/in/image.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ fn gather() -> @location(0) vec4<f32> {
@fragment
fn depth_no_comparison() -> @location(0) vec4<f32> {
let tc = vec2<f32>(0.5);
let level = 1;
let s2d = textureSample(image_2d_depth, sampler_reg, tc);
let s2d_gather = textureGather(image_2d_depth, sampler_reg, tc);
return s2d + s2d_gather;
let s2d_level = textureSampleLevel(image_2d_depth, sampler_reg, tc, level);
return s2d + s2d_gather + s2d_level;
}
3 changes: 2 additions & 1 deletion naga/tests/out/hlsl/image.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -369,5 +369,6 @@ float4 depth_no_comparison() : SV_Target0
float2 tc_3 = (0.5).xx;
float s2d_1 = image_2d_depth.Sample(sampler_reg, tc_3);
float4 s2d_gather = image_2d_depth.Gather(sampler_reg, tc_3);
return ((s2d_1).xxxx + s2d_gather);
float s2d_level = image_2d_depth.SampleLevel(sampler_reg, tc_3, 1);
return (((s2d_1).xxxx + s2d_gather) + (s2d_level).xxxx);
}
3 changes: 2 additions & 1 deletion naga/tests/out/msl/image.msl
Original file line number Diff line number Diff line change
Expand Up @@ -265,5 +265,6 @@ fragment depth_no_comparisonOutput depth_no_comparison(
metal::float2 tc_3 = metal::float2(0.5);
float s2d_1 = image_2d_depth.sample(sampler_reg, tc_3);
metal::float4 s2d_gather = image_2d_depth.gather(sampler_reg, tc_3);
return depth_no_comparisonOutput { metal::float4(s2d_1) + s2d_gather };
float s2d_level = image_2d_depth.sample(sampler_reg, tc_3, metal::level(1));
return depth_no_comparisonOutput { (metal::float4(s2d_1) + s2d_gather) + metal::float4(s2d_level) };
}
14 changes: 10 additions & 4 deletions naga/tests/out/spv/image.spvasm
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
; SPIR-V
; Version: 1.1
; Generator: rspirv
; Bound: 520
; Bound: 526
OpCapability Shader
OpCapability Image1D
OpCapability Sampled1D
Expand Down Expand Up @@ -687,8 +687,14 @@ OpBranch %512
%515 = OpCompositeExtract %7 %514 0
%516 = OpSampledImage %428 %511 %510
%517 = OpImageGather %23 %516 %280 %198
%518 = OpCompositeConstruct %23 %515 %515 %515 %515
%519 = OpFAdd %23 %518 %517
OpStore %508 %519
%518 = OpSampledImage %428 %511 %510
%520 = OpConvertSToF %7 %29
%519 = OpImageSampleExplicitLod %23 %518 %280 Lod %520
%521 = OpCompositeExtract %7 %519 0
%522 = OpCompositeConstruct %23 %515 %515 %515 %515
%523 = OpFAdd %23 %522 %517
%524 = OpCompositeConstruct %23 %521 %521 %521 %521
%525 = OpFAdd %23 %523 %524
OpStore %508 %525
OpReturn
OpFunctionEnd
3 changes: 2 additions & 1 deletion naga/tests/out/wgsl/image.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -235,5 +235,6 @@ fn depth_no_comparison() -> @location(0) vec4<f32> {
const tc_3 = vec2(0.5f);
let s2d_1 = textureSample(image_2d_depth, sampler_reg, tc_3);
let s2d_gather = textureGather(image_2d_depth, sampler_reg, tc_3);
return (vec4(s2d_1) + s2d_gather);
let s2d_level = textureSampleLevel(image_2d_depth, sampler_reg, tc_3, 1i);
return ((vec4(s2d_1) + s2d_gather) + vec4(s2d_level));
}

0 comments on commit 4b56262

Please sign in to comment.