Handle multiple PhysicalStorageBuffer in same struct

[spencer-lunarg]

closes #230
closes #231

@muraj and @danginsburg if you want to make sure this works for both your use cases that would be helpful

[spencer-lunarg]

@chaoticbob I'm going to just make a PR to run clang-format with more than 80 lines as this starts to just become unnecessarily hard to read

[spencer-lunarg]

Looked, these were duplicates (these tests do recursion looping) so this is actually good it was removed

[muraj]

Sure, I'll give it a try tonight, thank you very much for such a fast response!

[muraj]

Yup, this change is working for me, thanks again!

[danginsburg]

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?

[danginsburg]

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?

[spencer-lunarg]

@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)

[danginsburg]

Is this ready for me to test again or are you still working on the PR?

[spencer-lunarg]

ready to be tested, now convinced I fixed the issues, added another test as well

[danginsburg]

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.

[chaoticbob]

Just one nit