shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 159 forks source link

Request for a generic descriptor indexing intrinsic #4351

Open dubiousconst282 opened 2 weeks ago

dubiousconst282 commented 2 weeks ago

Descriptor indexing allows shaders to access resources in a natural and flexible way, while further eliminating the complexity of managing and updating descriptor sets. This feature can already be used today in Slang, but it requires that global bindings be declared for every accessed resource type and format beforehand, which can become a bit tedious and makes it difficult for users to implement abstractions on top.

Following from https://github.com/shader-slang/slang/issues/4150, it is not possible to define bindings inside a generic type due to design constraints in the compiler. What I propose instead, is for Slang to provide an intrinsic type that allows indexing over arbitrary resource types, similarly to ResourceDescriptorHeap in HLSL. This way, only one binding variable would need to be defined per descriptor set and binding index:

// Provided by Slang
__magic_type(...)
struct __RuntimeDescriptorArray {
    __intrinsic_op(...)
    T Get<T>(uint index);
}; 

// User code
[vk::binding(0, 0)] 
__RuntimeDescriptorArray g_TextureDescriptors;  // treated as opaque / unbounded array by layout and reflection

let texture = g_TextureDescriptors.Get<Texture2D<uint2>>(...);

The SPIRV backend would then emit the appropriate OpVariable definitions as it finds the specialized Get() calls. Potentially, this could also be used to implement the HLSL global heaps in a relatively portable way, although I'm not familiar with their binding model.

If this gets approved, I would be willing to work on an implementation if that is preferred.

csyonghe commented 2 weeks ago

We should provide something similar to ResourceDescriptorHeap and SamplerDescriptorHeap in HLSL.

Since D3D12/HLSL use separate descriptor heap for resources and samplers, we likely will need to keep them separate in Slang so we have a way to map to HLSL.

We would be happy to take patches for this implementation. The interface you proposed is fine, my only comment is that we should separate ResourceDescriptorArray and SamplerDescriptorArray to make hlsl easy to support.

csyonghe commented 2 weeks ago

We need to figure out how this DescirptorArray is reported by reflection API, and how to assign bindings for it.

In our layout rules, we should make it so that a DescriptorArray occupies a whole space, and is reflected as an unsized array of DescriptorTableSlot resource type.

dubiousconst282 commented 2 weeks ago

Thanks for your comments.

I agree about separating the resource and sampler heaps, but I'm a bit confused about whether these should be implemented with two different types or with the proposed type but in different globals for the (future) HLSL target. A constrained generic type in the struct could also work, although there doesn't seem to be a common interface for buffers yet.

The layout rules and reflection types make sense, I had not thought much of them at first so that is nice info to have :)

csyonghe commented 2 weeks ago

I quickly tried prototyping this in our current type system, and it seems something like this will work: image

jkwak-work commented 2 weeks ago

I will be happy to be wrong but it may fall in to a known limitation described in #3804. The second get shows red underline and that would be hard to resolve, I think.

csyonghe commented 2 weeks ago

The red underline there means the type system is working as intended. A Sampler2D is not a Resource kind, so you shouldn't be allowed to get a Sampler2D from a array that contains only resources.

natevm commented 2 weeks ago

Can the "Resource" and "Sampler" integers there be integers evaluated at runtime?

Eg,

ResourceArray<Resource> resArray;
int loc = resArray.get<int>(0);
Texture2D t = resArray.get<Texture2D>(loc);
natevm commented 2 weeks ago

The red underline there means the type system is working as intended. A Sampler2D is not a Resource kind, so you shouldn't be allowed to get a Sampler2D from a array that contains only resources.

Isn't the motivation for such a feature exactly this though? It should be possible to dynamically have different types bound to different offsets within the same descriptor array.

This feature can already be used today in Slang, but it requires that global bindings be declared for every accessed resource type and format beforehand, which can become a bit tedious and makes it difficult for users to implement abstractions on top.

csyonghe commented 2 weeks ago

We can have different type of Resources in the ResourceArray, but you can't get a Resource handle from a Sampler array.

This is because HLSL/D3D distinguishes a Sampler descriptor heap from a resource descriptor heap, unlike in Vulkan everything can be placed in a single descriptor set allocated from a single descriptor heap.

csyonghe commented 2 weeks ago

Can the "Resource" and "Sampler" integers there be integers evaluated at runtime?

Eg,

ResourceArray<Resource> resArray;
int loc = resArray.get<int>(0);
Texture2D t = resArray.get<Texture2D>(loc);

You mean the index passsed to get method being a runtime value. Yes this is the intended use of this type.

natevm commented 2 weeks ago

For some context, this is what I'm kinda forced into doing at the moment with Slang. I force users to include some code which inlines the below descriptor arrays, which then I can sorta manage myself in the host code.

[[vk::binding(0, 1)]] SamplerState samplers[];
[[vk::binding(0, 2)]] Texture1D texture1Ds[];
[[vk::binding(0, 3)]] Texture2D texture2Ds[];
[[vk::binding(0, 4)]] Texture3D texture3Ds[];
[[vk::binding(0, 5)]] RWByteAddressBuffer buffers[];

But this is also very constraining. For example, I can't distinguish between different buffer types. At the same time, I can't just enumerate all the different types of descriptors and make bindings for each one.

natevm commented 2 weeks ago

It's also very frustrating to need to include texture1Ds and texture3Ds when more often than not, folks using my framework don't use them. I have to add in these placeholder descriptors for each new type, eg a volume of 1 voxel, texture with 1 pixel, a buffer with one integer. It would be really nice if I didn't have to do that...

natevm commented 2 weeks ago

We can have different type of Resources in the ResourceArray, but you can't get a Resource handle from a Sampler array.

This is because HLSL/D3D distinguishes a Sampler descriptor heap from a resource descriptor heap, unlike in Vulkan everything can be placed in a single descriptor set allocated from a single descriptor heap.

I see. I'm coming from a Vulkan user's perspective. I'm probably ok to just separate out the samplers into their own dedicated array. Just so long as the N other resource types could all be put together.

swoods-nv commented 2 weeks ago

@dubiousconst282 Can you let us know if this is something you'll be working on in the near term? Thank you!

dubiousconst282 commented 2 weeks ago

Yes, I am working on a draft and will be ready to submit a PR shortly (hopefully before the end of the week).

jkwak-work commented 2 weeks ago

It appears that I misunderstood what was asked. Thanks for the link at the very beginning. What Yong suggested now makes all sense to me. I will review the draft tomorrow.