Open s-perron opened 3 months ago
I'd argue that a static initialization syntax for such a domain-specific issue will only fragment the language. Why not have a common attribute that we can attach to SamplerState
bindings, that accepts common sampler settings? From what I can tell this could also be implemented without requiring a spec change. Something like this:
[StaticSampler(ADDRESS_REPEAT, FILTER_LINEAR)]
SamplerState sampler : register(s0, space0);
For DXIL, this could then export to reflection, which currently does not support static samplers (and which is a large drawback imho). Of course in D3D static samplers can be set up more granularly, however, if you need such fine control you can still fall back to authoring root signatures. As for SPIR-V, the second parameter of OpConstantSampler
dictates if the coordinates are normalized or not. I am not sure if this can be implicitly deduced (I doubt it), but I think a third boolean parameter would do the job that gets ignored when targeting DXIL. Alternatively, I could even see there being multiple overloads of such an attribute with specific settings for D3D static samplers, as well as SPIR-V variants, each with fallbacks to defaults when targeting the other backend (and maybe a warning or two).
Is your feature request related to a problem? Please describe.
Constant samplers are described in HLSL for DXIL in the root signature. That does not work for Vulkan because constant samplers are treated like constant in the shader using the OpConstantSampler instructions.
We need a way to declare a constnat sampler in Vulkan.
Describe the solution you'd like
Something that would work like https://godbolt.org/z/YKbEa4o43, except the OpConstant instruction would be placed in the correct location in the spir-v.
I'm thinking we could have a builtin function in the vk namespace that would do what that sample ext_instruction does. Then the compiler could put it in the correct place.
Describe alternatives you've considered
See https://github.com/microsoft/DirectXShaderCompiler/issues/4137.
The syntax
was suggested. I do not like this because it does not have an obvious mapping to the SPIR-V operands. Also it looks like DXC does not add the information to the AST.
Additional context
Not much to add other than that the original issue seems to have a multiple +1. This could be a useful feature for users.