shader-slang / slang

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

`tbuffer` or `TextureBuffer` use causes silent failure due to layout rules #4428

Closed chaoticbob closed 6 days ago

chaoticbob commented 1 week ago

The shader below results in NULL pointer access since the rules for texture buffer objects are not implemented:

// slang-type-layout.cpp:
LayoutRulesImpl* GLSLLayoutRulesFamilyImpl::getTextureBufferRules()
{
    return nullptr;
}

Call chain starts in slang-type-layout.cpp:createParameterGroupTypeLayout().

This is causing a silent failure, should return an error message at the very least if not implemented.

Shader

tbuffer MyTBuffer {
    float4 tb_val;
}

float4 main() : SV_Target {
    return tb_val;
}
ArielG-NV commented 1 week ago

I suppose there are 2 problems to address here:

  1. An error should be more clear here (some ASSERT)?
  2. Add layout rules to tbuffer
chaoticbob commented 1 week ago

I agree. Apologies, I was going for the least amount of work needed to address the current behavior. But you're right, would be good to have the functionality.

chaoticbob commented 1 week ago

This also applies to TextureBuffer (note: it's much less used based on memory from previous experience).

jkwak-work commented 1 week ago

I suggest to use SLANG_UNIMPLEMENTED for now and close. And create a new issue for the actual implementation.

ArielG-NV commented 1 week ago

A seperate issue was made to implement spirv and metal for TextureBuffer (significant and simmilar changes required): #4435

hlsl and glsl should both work once the linked PR is merged.