shader-slang / slang

Making it easier to work with shaders
http://shader-slang.com
MIT License
2.17k stars 187 forks source link

ByteAddressBuffer Matrix Load<T> order consistency #2317

Closed kevyuu closed 2 years ago

kevyuu commented 2 years ago

Hi, when I load a matrix from ByteAddressBuffer in SPIR-V backend, It will assume the row major ordering regardless of the defaultMatrixLayoutMode.

static const char* RESOURCE_SLANG = R"SLANG(
[[vk::binding(0, 0)]] ByteAddressBuffer global_buffers[];
[[vk::binding(0, 1)]] SamplerState global_samplers[];
[[vk::binding(0, 2)]] Texture2D global_2d_textures[];
[[vk::binding(0, 2)]] TextureCube global_cube_textures[];

__glsl_extension(GL_EXT_nonuniform_qualifier)
T get_buffer<T>(uint descriptor_id) {
    return global_buffers[descriptor_id].Load<T>(0);
}

SamplerState get_sampler(uint descriptor_id) {
    return global_samplers[descriptor_id];
}

Texture2D get_texture_2d(uint descriptor_id) {
    return global_2d_textures[descriptor_id];
}
)SLANG";

This is not true when I read matrix data from push constant, which follow the defaultMatrixLayoutMode. Personally, I use row major ordering., so I am okay with this. But I wonder whether this will be consistent with dx12 backend? There is a ticket on dxc compiler for this issue: 3370. They used row ordering as well when targeting SPIR-V, but column ordering when targeting dx12. Not sure whether the same is happening here. They just fix it a few days ago.

I notice that when I load a struct this way, It is translated to copy in the GLSL code since it assumes that the data have c/cpp alignment instead of (std430/std140). I am planning to do bindless buffer this way. Should I be concerned on the performance of copying these struct?

csyonghe commented 2 years ago

There shouldn't be any performance concerns regarding data layouts on most GPU architectures that are relevant today. In most cases Slang will ensure the same buffer layout across HLSL and SPIRV, so you can expect the data layout be consistent.