Closed spencer-lunarg closed 8 months ago
Sure, I'll give it a try tonight, thank you very much for such a fast response!
Yup, this change is working for me, thanks again!
I tested this change, it does seem to not assert anymore, but I'm not getting any information about which members of a buffer_reference are statically referenced, I just see this in the output:
`Push constant blocks: 1
0:
spirv id : 1649
name : g_GlobalsBDAPushConstant_0 (_S2)
// size = 96, padded size = 96
struct _S2 {
// abs offset = 0, rel offset = 0, size = 96, padded size = 96, array stride = 16
ref struct BufferPointer_BDAGlobals_t_0_1 {
} g_GlobalsBDAPerStage_0[6];
} g_GlobalsBDAPushConstant_0;`
My understanding from @greg-lunarg is that it should be reporting which members are statically referenced.
The shader in #231 demonstrates this problem. You can now reflect the buffer_reference, but shouldn't it show that only g_GlobalsBDAPerStage_0[6]._data.g_vTest
is referenced?
Hi @spencer-lunarg - I am trying to look at this change some more. When I run your change on tests\glsl\buffer_pointer.spv I see:
D:\Users\Dan\src\SPIRV-Reflect\tests\glsl>..\..\bin\Debug\spirv-reflect.exe -v 2 buffer_pointer.spv
generator : Khronos Glslang Reference Front End
source lang : GLSL
source lang ver : 460
source file : buffer_pointer.glsl
entry point : main (stage=PS)
Output variables: 1
0:
spirv id : 26
location : 0
type : float4
semantic :
name : Color
qualifier :
Push constant blocks: 1
0:
spirv id : 14
name : push (PushData)
// size = 16, padded size = 16
struct PushData {
// abs offset = 0, rel offset = 0, size = 8, padded size = 16
ref struct Data {
} data_ptr;
} push;
In @greg-lunarg original PR (https://github.com/KhronosGroup/SPIRV-Reflect/pull/180/files) both in the spirv-reflect output and yaml it would report on the referenced variables g0/g1/g2, i.e.:
name : push (PushData)
// size = 16, padded size = 16
struct PushData {
// abs offset = 0, rel offset = 0, size = 8, padded size = 16
ref struct Data {
float g0; // abs offset = 0, rel offset = 0, size = 4, padded
size = 4 UNUSED
float g1; // abs offset = 4, rel offset = 4, size = 4, padded
size = 4
float g2; // abs offset = 8, rel offset = 8, size = 4, padded
size = 8 UNUSED
} data_ptr;
} push;
I am trying also to use the API and I am not seeing how I can reflect out which members pointed to by the buffer_ref's in the push_constant are active. So it seems this change isn't fully working?
@danginsburg looking into fixing the print-out logic now
I am not seeing how I can reflect out which members pointed to by the buffer_ref's in the push_constant are active
this is an orthogonal issue of spirv-reflect
as the accessed
flag is only in the DescriptorBinding
and not currently found at a push constant level (that will require adding logic to mark accesses at a more fine granularity)
Is this ready for me to test again or are you still working on the PR?
ready to be tested, now convinced I fixed the issues, added another test as well
From my testing so far, this PR now does seem to correctly fill in the SPV_REFLECT_VARIABLE_FLAGS_UNUSED
flag for the unused BDA fields.
closes #230 closes #231
@muraj and @danginsburg if you want to make sure this works for both your use cases that would be helpful