From what I can tell, the recent work on SampleBias/SampleLevel did all of the work for us, we just need to take the same framework and include the case for the `sample_l` instruction.
Tested with some shaders from the native Linux version of Little Racers STREET.
-- v5: vkd3d-shader/tpf: For sample_l/sample_b, set lod swizzle to SCALAR.
From: Ethan Lee flibitijibibo@gmail.com
Currently marked TODO, since the SM4 emitter does not support sample_l yet.
Signed-off-by: Ethan Lee flibitijibibo@gmail.com --- tests/sampler.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/sampler.shader_test b/tests/sampler.shader_test index 2cb70a0b..51d63f16 100644 --- a/tests/sampler.shader_test +++ b/tests/sampler.shader_test @@ -33,6 +33,19 @@ float4 main() : sv_target draw quad probe all rgba (0.25, 0, 0.25, 0)
+[pixel shader todo] +SamplerState s; +Texture2D t; + +float4 main() : sv_target +{ + return t.SampleLevel(s, float2(0.5, 0.5), 0.0); +} + +[test] +todo draw quad +todo probe all rgba (0.25, 0, 0.25, 0) + [pixel shader] SamplerState s; Texture2D t;
From: Ethan Lee flibitijibibo@gmail.com
This gets the SampleLevel() HLSL test to pass.
Signed-off-by: Ethan Lee flibitijibibo@gmail.com --- libs/vkd3d-shader/tpf.c | 12 +++++++----- tests/sampler.shader_test | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 3debe952..5d194709 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3717,6 +3717,10 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer instr.opcode = VKD3D_SM4_OP_SAMPLE; break;
+ case HLSL_RESOURCE_SAMPLE_LOD: + instr.opcode = VKD3D_SM4_OP_SAMPLE_LOD; + break; + case HLSL_RESOURCE_SAMPLE_LOD_BIAS: instr.opcode = VKD3D_SM4_OP_SAMPLE_B; break; @@ -3743,7 +3747,8 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer sm4_src_from_deref(ctx, &instr.srcs[2], sampler, sampler->var->data_type, VKD3DSP_WRITEMASK_ALL); instr.src_count = 3;
- if (load->load_type == HLSL_RESOURCE_SAMPLE_LOD_BIAS) + if (load->load_type == HLSL_RESOURCE_SAMPLE_LOD + || load->load_type == HLSL_RESOURCE_SAMPLE_LOD_BIAS) { sm4_src_from_node(&instr.srcs[3], load->lod.node, VKD3DSP_WRITEMASK_ALL); ++instr.src_count; @@ -4515,6 +4520,7 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, break;
case HLSL_RESOURCE_SAMPLE: + case HLSL_RESOURCE_SAMPLE_LOD: case HLSL_RESOURCE_SAMPLE_LOD_BIAS: if (!load->sampler.var) { @@ -4543,10 +4549,6 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, write_sm4_gather(ctx, buffer, resource_type, &load->node, &load->resource, &load->sampler, coords, HLSL_SWIZZLE(W, W, W, W), texel_offset); break; - - case HLSL_RESOURCE_SAMPLE_LOD: - hlsl_fixme(ctx, &load->node.loc, "SM4 sample-LOD expression."); - break; } }
diff --git a/tests/sampler.shader_test b/tests/sampler.shader_test index 51d63f16..62c71b39 100644 --- a/tests/sampler.shader_test +++ b/tests/sampler.shader_test @@ -33,7 +33,7 @@ float4 main() : sv_target draw quad probe all rgba (0.25, 0, 0.25, 0)
-[pixel shader todo] +[pixel shader] SamplerState s; Texture2D t;
@@ -43,8 +43,8 @@ float4 main() : sv_target }
[test] -todo draw quad -todo probe all rgba (0.25, 0, 0.25, 0) +draw quad +probe all rgba (0.25, 0, 0.25, 0)
[pixel shader] SamplerState s;
From: Ethan Lee flibitijibibo@gmail.com
Per Francisco Casas fcasas@codeweavers.com:
I just realized one more thing. I think we are not properly following the specification for both sample_l and sample_b.
In the Restrictions section of the documentation for sample_b it is mentioned that the last src register must use a single component selector if it is not a scalar immediate. This "select_component" is also ilustrated in the sample_l format:
sample_l[_aoffimmi(u,v,w)] dest[.mask], srcAddress[.swizzle], srcResource[.swizzle], srcSampler, srcLOD.select_component
Currently, we are not respecting that since sm4_src_from_node() sets
src->swizzle_type = VKD3D_SM4_SWIZZLE_VEC4;
by default.
It should be VKD3D_SM4_SWIZZLE_SCALAR in this case... The disassembly should show a single component in the swizzle of the last src register.
Signed-off-by: Ethan Lee flibitijibibo@gmail.com --- libs/vkd3d-shader/tpf.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 5d194709..604593eb 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3751,6 +3751,7 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer || load->load_type == HLSL_RESOURCE_SAMPLE_LOD_BIAS) { sm4_src_from_node(&instr.srcs[3], load->lod.node, VKD3DSP_WRITEMASK_ALL); + instr.srcs[3].swizzle_type = VKD3D_SM4_SWIZZLE_SCALAR; ++instr.src_count; }
On Fri Apr 28 23:27:41 2023 +0000, Francisco Casas wrote:
I just realized one more thing. I think we are not properly following the specification for both sample_l and sample_b. In the **Restrictions** section of the documentation for sample_b it is mentioned that the last src register must use a single component selector if it is not a scalar immediate. This "select_component" is also ilustrated in the sample_l format:
sample_l[_aoffimmi(u,v,w)] dest[.mask], srcAddress[.swizzle], srcResource[.swizzle], srcSampler, srcLOD.select_componentCurrently, we are not respecting that since sm4_src_from_node() sets
src->swizzle_type = VKD3D_SM4_SWIZZLE_VEC4;by default. It should be VKD3D_SM4_SWIZZLE_SCALAR in this case. If I am not mistaken, a simple:
instr.srcs[3].swizzle_type = VKD3D_SM4_SWIZZLE_SCALAR;after calling sm4_src_from_node() should do. The disassembly should show a single component in the swizzle of the last src register.
Added this as a new commit on top - the disassembly before this commit shows a single component already, which made me think it might have been getting set elsewhere, but _after_ the change the binary output was different, so it might be a coincidence.
On Fri Apr 28 23:27:41 2023 +0000, Ethan Lee wrote:
Added this as a new commit on top - the disassembly before this commit shows a single component already, which made me think it might have been getting set elsewhere, but _after_ the change the binary output was different, so it might be a coincidence.
For the record, before the patch, with the native dissasembler `fxc -dumpbin`, I was getting: ``` sample_l r0.xyzw, r0.xyxx, t0.xyzw, s0, r0.zzzz ``` and I see that our dissasembler shows the last bit as `r0.z`. So there is a discrepancy there.
Anyways, after the patch, I get ``` sample_l r0.xyzw, r0.xyxx, t0.xyzw, s0, r0.z ``` with the native dissasembler. So it seems good now. :)
This merge request was approved by Francisco Casas.
For the record, before the patch, with the native dissasembler `fxc -dumpbin`, I was getting:
sample_l r0.xyzw, r0.xyxx, t0.xyzw, s0, r0.zzzzand I see that our dissasembler shows the last bit as `r0.z`. So there is a discrepancy there.
That's mostly a consequence of sm1/d3dbc not having a distinction between VKD3D_SM4_SWIZZLE_SCALAR/VKD3D_SM4_SWIZZLE_VEC4; "r0.z" and "r0.zzzz" are equivalent there. See also shader_sm4_read_src_param() and shader_dump_src_param(). We should probably fix that, although there would be consequences for e.g. the SPIR-V backend as well.
-
Ethan Lee -
Ethan Lee (＠flibitijibibo)
-
Francisco Casas (＠fcasas)
-
Henri Verbeet (＠hverbeet)