[SPIR-V][vk::SampledTexture] #10. Support `vk::SampledTexture1D` and `vk::SampledTexture1DArray` type. by luciechoi · Pull Request #8212 · microsoft/DirectXShaderCompiler

luciechoi · 2026-02-27T21:08:20Z

Part of #7979

The function definitions are equivalent to that of Texture1D counterparts, just without Sampler argument.

luciechoi · 2026-03-19T07:02:36Z

(@Keenuts @s-perron next SampledTexture PR for review)

Keenuts

Overall looks good, but many overloads are not tested. I stopped checking after a few, but overall we should have at least one test for each overload.

Keenuts · 2026-03-19T10:11:57Z

utils/hct/gen_intrin_main.txt

+void [[]] GetDimensions(in uint x, out uint_only width, out $type2 elements, out $type2 levels) : resinfo_uint;
+void [[]] GetDimensions(in uint x, out float_like width, out $type2 elements, out $type2 levels) : resinfo;


Those are not tested

Keenuts · 2026-03-19T10:12:09Z

utils/hct/gen_intrin_main.txt

+void [[]] GetDimensions(in uint x, out uint_only width, out $type2 elements, out $type2 levels) : resinfo_uint;
+void [[]] GetDimensions(in uint x, out float_like width, out $type2 elements, out $type2 levels) : resinfo;
+void [[]] GetDimensions(out uint_only width, out $type1 elements) : resinfo_uint_o;
+void [[]] GetDimensions(out float_like width, out $type1 elements) : resinfo_o;


This one is not tested.

Keenuts · 2026-03-19T10:12:54Z

utils/hct/gen_intrin_main.txt

+void [[]] GetDimensions(out uint_only width, out $type1 elements) : resinfo_uint_o;
+void [[]] GetDimensions(out float_like width, out $type1 elements) : resinfo_o;
+$classT [[ro]] Load(in int<3> x) : tex1d_t_load_array;
+$classT [[ro]] Load(in int<3> x, in int<1> o) : tex1d_t_load_array_o;


This one is not tested.

Keenuts · 2026-03-19T10:13:00Z

utils/hct/gen_intrin_main.txt

+void [[]] GetDimensions(out float_like width, out $type1 elements) : resinfo_o;
+$classT [[ro]] Load(in int<3> x) : tex1d_t_load_array;
+$classT [[ro]] Load(in int<3> x, in int<1> o) : tex1d_t_load_array_o;
+$classT [[]] Load(in int<3> x, in int<1> o, out uint_only status) : tex1d_t_load_array_o_s;


This one is not tested.

Keenuts · 2026-03-19T10:13:56Z

utils/hct/gen_intrin_main.txt

+$classT [[ro]] Load(in int<3> x, in int<1> o) : tex1d_t_load_array_o;
+$classT [[]] Load(in int<3> x, in int<1> o, out uint_only status) : tex1d_t_load_array_o_s;
+$classT [[ro]] Sample(in float<2> x) : tex1d_t_array;
+$classT [[ro]] Sample(in float<2> x, in int<1> o) : tex1d_t_array_o;


Keenuts · 2026-03-19T10:14:28Z

utils/hct/gen_intrin_main.txt

+$classT [[]] Load(in int<3> x, in int<1> o, out uint_only status) : tex1d_t_load_array_o_s;
+$classT [[ro]] Sample(in float<2> x) : tex1d_t_array;
+$classT [[ro]] Sample(in float<2> x, in int<1> o) : tex1d_t_array_o;
+$classT [[ro]] SampleBias(in float<2> x, in float bias) : tex1d_t_bias_array;


Keenuts · 2026-03-19T10:14:55Z

utils/hct/gen_intrin_main.txt

+$classT [[ro]] SampleBias(in float<2> x, in float bias) : tex1d_t_bias_array;
+$classT [[ro]] SampleBias(in float<2> x, in float bias, in int<1> o) : tex1d_t_bias_array_o;
+float_like [[ro]] SampleCmp(in float<2> x, in float compareValue) : tex1d_t_comp_array;
+float_like [[ro]] SampleCmp(in float<2> x, in float compareValue, in int<1> o) : tex1d_t_comp_array_o;


luciechoi · 2026-03-20T00:49:42Z

Overall looks good, but many overloads are not tested. I stopped checking after a few, but overall we should have at least one test for each overload.

Hmmm do you mean every function variant should have a test..? The overloads are tested with SampledTexture2D type, and adding a 1D type doesn't require code changes.

I took a look at adding these, but noticed that the existing test coverage for the other texture types (like Texture2D, Texture2DMS, and their array counterparts) also doesn't exhaustively test every single overload.

To keep the scope of this PR manageable and maintain parity with regular Texture types, I've mirrored the existing level of coverage for the 1D types. Let me know if there's a specific overload variant you are concerned about and I'd be happy to add a test just for that one.

Keenuts · 2026-03-20T08:49:14Z

Hmmm do you mean every function variant should have a test..? The overloads are tested with SampledTexture2D type, and adding a 1D type doesn't require code changes.

If something is not tested, something can be removed accidentally without us noticing.
That's why some projects implement mutation testing or similar.
The test does not have to be complex, but we should make sure each overload is at least present through a test.

s-perron

I agree that we need to haves testing for all for all of the new functions, even if the code generating them is the same. I think you are working on that.

We can also improve some string checking, which can be slow.

s-perron · 2026-03-24T13:52:49Z

tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

+    const spv::Dim dimension =
+        name.count("1D") > 0 ? spv::Dim::Dim1D : spv::Dim::Dim2D;
+    const bool isArray = name.count("Array") > 0;
+    const bool isMS = name.count("MS") > 0;


count iterates over the whole string. You can use other member functions that might be more efficient.

I don't know all of the future cases we might need to handle.

Could we do:

// Drop the "SampledTexture" prefix. 1 StringRef suffix = name.drop_front(14); 2 const spv::Dim dimension = 3 suffix.startswith("1D") ? spv::Dim::Dim1D : spv::Dim::Dim2D; 4 const bool isArray = suffix.endswith("Array"); 5 const bool isMS = suffix.find("MS") != StringRef::npos;

s-perron · 2026-03-24T13:55:07Z

tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

+  if (name == "SampledTexture1D" || name == "SampledTexture1DArray" ||
+      name == "SampledTexture2D" || name == "SampledTexture2DArray" ||
      name == "SampledTexture2DMS" || name == "SampledTexture2DMSArray") {


This is turning into a lot of string compares. Could we simplly do:

name.startswith("SampledTexture");

github-project-automation bot added this to HLSL Roadmap Feb 27, 2026

github-project-automation bot moved this to New in HLSL Roadmap Feb 27, 2026

luciechoi requested review from Keenuts and s-perron February 27, 2026 21:10

luciechoi force-pushed the texture1d branch 2 times, most recently from d198142 to 7d4ff86 Compare February 27, 2026 22:12

luciechoi added 4 commits March 4, 2026 00:49

Implement vk::SampledTexture2DArray

16294e5

Address copilot comments

92e04d1

SampledTexture2DMS, SampledTexture2DMSArray

d2a7278

SampledTexture1D, SampledTexture1DArray

26674de

luciechoi force-pushed the texture1d branch from 7d4ff86 to 26674de Compare March 4, 2026 19:46

Merge branch 'main' into texture1d

9a4f2ba

Keenuts requested changes Mar 19, 2026

View reviewed changes

github-project-automation bot moved this from New to In progress in HLSL Roadmap Mar 19, 2026

luciechoi requested a review from Keenuts March 20, 2026 00:50

s-perron reviewed Mar 24, 2026

View reviewed changes

		void [[]] GetDimensions(in uint x, out uint_only width, out $type2 elements, out $type2 levels) : resinfo_uint;
		void [[]] GetDimensions(in uint x, out float_like width, out $type2 elements, out $type2 levels) : resinfo;

Conversation

luciechoi commented Feb 27, 2026

Uh oh!

luciechoi commented Mar 19, 2026

Uh oh!

Keenuts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luciechoi commented Mar 20, 2026

Uh oh!

Keenuts commented Mar 20, 2026

Uh oh!

s-perron left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

s-perron left a comment •

edited

Loading