Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[spv-in] sampling from depth textures returns a scalar and is splatted #6384

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216).

#### Naga

- SPIR-V frontend splats depth texture sample and load results. Fixes [issue #4551](https://github.com/gfx-rs/wgpu/issues/4551). By @schell in [#6384](https://github.com/gfx-rs/wgpu/pull/6384).
- Accept only `vec3` (not `vecN`) for the `cross` built-in. By @ErichDonGubler in [#6171](https://github.com/gfx-rs/wgpu/pull/6171).
- Configure `SourceLanguage` when enabling debug info in SPV-out. By @kvark in [#6256](https://github.com/gfx-rs/wgpu/pull/6256).
- Per-polygon and flat inputs should not be considered subgroup uniform. By @magcius in [#6276](https://github.com/gfx-rs/wgpu/pull/6276).
Expand Down
79 changes: 61 additions & 18 deletions naga/src/front/spv/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,36 +377,60 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
let coord_handle =
self.get_expr_handle(coordinate_id, coord_lexp, ctx, emitter, block, body_idx);
let coord_type_handle = self.lookup_type.lookup(coord_lexp.type_id)?.handle;
let (coordinate, array_index) = match ctx.type_arena[image_ty].inner {
let (coordinate, array_index, is_depth) = match ctx.type_arena[image_ty].inner {
crate::TypeInner::Image {
dim,
arrayed,
class: _,
} => extract_image_coordinates(
dim,
if arrayed {
ExtraCoordinate::ArrayLayer
} else {
ExtraCoordinate::Garbage
},
coord_handle,
coord_type_handle,
ctx,
),
class,
} => {
let (coord, array_index) = extract_image_coordinates(
dim,
if arrayed {
ExtraCoordinate::ArrayLayer
} else {
ExtraCoordinate::Garbage
},
coord_handle,
coord_type_handle,
ctx,
);
(coord, array_index, class.is_depth())
}
_ => return Err(Error::InvalidImage(image_ty)),
};

let expr = crate::Expression::ImageLoad {
let image_load_expr = crate::Expression::ImageLoad {
image: image_lexp.handle,
coordinate,
array_index,
sample,
level,
};
let image_load_handle = ctx
.expressions
.append(image_load_expr, self.span_from_with_op(start));

let handle = if is_depth {
let result_ty = self.lookup_type.lookup(result_type_id)?;
match ctx.type_arena[result_ty.handle].inner {
schell marked this conversation as resolved.
Show resolved Hide resolved
crate::TypeInner::Vector { size, .. } => {
let splat_expr = crate::Expression::Splat {
size,
value: image_load_handle,
};
ctx.expressions
.append(splat_expr, self.span_from_with_op(start))
}
_ => image_load_handle,
}
} else {
image_load_handle
};

self.lookup_expression.insert(
result_id,
LookupExpression {
handle: ctx.expressions.append(expr, self.span_from_with_op(start)),
handle,
type_id: result_type_id,
block_id,
},
Expand Down Expand Up @@ -593,11 +617,12 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
ref other => return Err(Error::InvalidGlobalVar(other.clone())),
}

let ((coordinate, array_index), depth_ref) = match ctx.type_arena[image_ty].inner {
let ((coordinate, array_index), depth_ref, is_depth) = match ctx.type_arena[image_ty].inner
{
crate::TypeInner::Image {
dim,
arrayed,
class: _,
class,
} => (
extract_image_coordinates(
dim,
Expand Down Expand Up @@ -642,6 +667,7 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
None => None,
}
},
class.is_depth(),
),
_ => return Err(Error::InvalidImage(image_ty)),
};
Expand All @@ -656,10 +682,27 @@ impl<I: Iterator<Item = u32>> super::Frontend<I> {
level,
depth_ref,
};
let image_sample_handle = ctx.expressions.append(expr, self.span_from_with_op(start));
let handle = if is_depth && depth_ref.is_none() {
let result_ty = self.lookup_type.lookup(result_type_id)?;
match ctx.type_arena[result_ty.handle].inner {
crate::TypeInner::Vector { size, .. } => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to check this here too? I thought all sampling functions return vec4s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure, so I decided to do the safe thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it doesn't hurt but might be misleading. I checked yesterday that all of SPIR-V's sampling functions that we handle all return vec4s, could you revert this?

let splat_expr = crate::Expression::Splat {
size,
value: image_sample_handle,
};
ctx.expressions
.append(splat_expr, self.span_from_with_op(start))
}
_ => image_sample_handle,
}
} else {
image_sample_handle
};
self.lookup_expression.insert(
result_id,
LookupExpression {
handle: ctx.expressions.append(expr, self.span_from_with_op(start)),
handle,
type_id: result_type_id,
block_id,
},
Expand Down
4 changes: 4 additions & 0 deletions naga/src/proc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,10 @@ impl super::ImageClass {
crate::ImageClass::Storage { .. } => false,
}
}

pub const fn is_depth(self) -> bool {
matches!(self, crate::ImageClass::Depth { .. })
}
}

impl crate::Module {
Expand Down
Binary file added naga/tests/in/spv/fetch_depth.spv
Binary file not shown.
52 changes: 52 additions & 0 deletions naga/tests/in/spv/fetch_depth.spvasm
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
; SPIR-V
; Version: 1.5
; Generator: Google rspirv; 0
; Bound: 56
; Schema: 0
OpCapability Shader
OpCapability VulkanMemoryModel
OpMemoryModel Logical Vulkan
OpEntryPoint GLCompute %1 "cull::fetch_depth" %2 %3 %4
OpExecutionMode %1 LocalSize 32 1 1
OpDecorate %_struct_10 Block
OpMemberDecorate %_struct_10 0 Offset 0
OpDecorate %_struct_11 Block
OpMemberDecorate %_struct_11 0 Offset 0
OpDecorate %2 Binding 0
OpDecorate %2 DescriptorSet 0
OpDecorate %3 NonWritable
OpDecorate %3 Binding 1
OpDecorate %3 DescriptorSet 0
OpDecorate %4 Binding 2
OpDecorate %4 DescriptorSet 0
%uint = OpTypeInt 32 0
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%void = OpTypeVoid
%16 = OpTypeFunction %void
%uint_0 = OpConstant %uint 0
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%_struct_10 = OpTypeStruct %float
%_ptr_StorageBuffer__struct_10 = OpTypePointer StorageBuffer %_struct_10
%v2uint = OpTypeVector %uint 2
%_struct_11 = OpTypeStruct %v2uint
%_ptr_StorageBuffer__struct_11 = OpTypePointer StorageBuffer %_struct_11
%24 = OpTypeImage %float 2D 1 0 0 1 Unknown
%_ptr_UniformConstant_24 = OpTypePointer UniformConstant %24
%_ptr_StorageBuffer_float = OpTypePointer StorageBuffer %float
%2 = OpVariable %_ptr_StorageBuffer__struct_10 StorageBuffer
%_ptr_StorageBuffer_v2uint = OpTypePointer StorageBuffer %v2uint
%3 = OpVariable %_ptr_StorageBuffer__struct_11 StorageBuffer
%4 = OpVariable %_ptr_UniformConstant_24 UniformConstant
%1 = OpFunction %void None %16
%32 = OpLabel
%33 = OpInBoundsAccessChain %_ptr_StorageBuffer_float %2 %uint_0
%34 = OpInBoundsAccessChain %_ptr_StorageBuffer_v2uint %3 %uint_0
%35 = OpLoad %v2uint %34
%54 = OpLoad %24 %4
%55 = OpImageFetch %v4float %54 %35 Lod %int_0
%38 = OpCompositeExtract %float %55 0
OpStore %33 %38
OpReturn
OpFunctionEnd
25 changes: 25 additions & 0 deletions naga/tests/out/hlsl/fetch_depth.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
struct type_2 {
float member;
};

struct type_4 {
uint2 member;
};

RWByteAddressBuffer global : register(u0);
ByteAddressBuffer global_1 : register(t1);
Texture2D<float> global_2 : register(t2);

void function()
{
uint2 _e6 = asuint(global_1.Load2(0));
float _e7 = global_2.Load(int3(_e6, 0)).x;
global.Store(0, asuint((_e7).xxxx.x));
return;
}

[numthreads(32, 1, 1)]
void cullfetch_depth()
{
function();
}
12 changes: 12 additions & 0 deletions naga/tests/out/hlsl/fetch_depth.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
(
vertex:[
],
fragment:[
],
compute:[
(
entry_point:"cullfetch_depth",
target_profile:"cs_5_1",
),
],
)
Loading