Serialise name in GL program block binding calls, to remap index

* The block index may change between capture and replay, so serialising the
  index alone is unstable similar to locations. Program initial states already
  serialise by name, but if a capture contains a block binding change mid-frame
  this could serialise wrongly.

renderdoc/driver/gl/gl_common.cpp

@@ -1189,6 +1189,11 @@ bool GLInitParams::IsSupportedVersion(uint64_t ver)
   if(ver == 0x20)
     return true;
 
+  // 0x21 -> 0x22 - serialise glUniformBlockBinding and glShaderStorageBlockBinding with the name
+  //                instead of the block index, in case the index changes between capture and replay
+  if(ver == 0x21)
+    return true;
+
   return false;
 }
 

renderdoc/driver/gl/gl_driver.h

@@ -59,7 +59,7 @@ struct GLInitParams
   rdcstr renderer, version;
 
   // check if a frame capture section version is supported
-  static const uint64_t CurrentVersion = 0x21;
+  static const uint64_t CurrentVersion = 0x22;
   static bool IsSupportedVersion(uint64_t ver);
 };
 

renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp

@@ -937,6 +937,34 @@ bool WrappedOpenGL::Serialise_glUniformBlockBinding(SerialiserType &ser, GLuint
   SERIALISE_ELEMENT(uniformBlockIndex);
   SERIALISE_ELEMENT(uniformBlockBinding);
 
+  if(ser.VersionAtLeast(0x22))
+  {
+    rdcstr blockName;
+
+    if(ser.IsWriting())
+    {
+      GLint length = 1;
+      GL.glGetActiveUniformBlockiv(program.name, uniformBlockIndex, eGL_UNIFORM_BLOCK_NAME_LENGTH,
+                                   &length);
+
+      blockName.resize(length + 1);
+
+      GL.glGetActiveUniformBlockName(program.name, uniformBlockIndex, length, &length,
+                                     blockName.data());
+
+      blockName.resize(strlen(blockName.c_str()));
+    }
+
+    SERIALISE_ELEMENT(blockName).Hidden();
+
+    if(IsReplayingAndReading())
+    {
+      GLuint idx = GL.glGetUniformBlockIndex(program.name, blockName.c_str());
+      if(idx != GL_INVALID_INDEX)
+        uniformBlockIndex = idx;
+    }
+  }
+
   SERIALISE_CHECK_READ_ERRORS();
 
   if(IsReplayingAndReading())
@@ -976,6 +1004,37 @@ bool WrappedOpenGL::Serialise_glShaderStorageBlockBinding(SerialiserType &ser, G
   SERIALISE_ELEMENT(storageBlockIndex);
   SERIALISE_ELEMENT(storageBlockBinding);
 
+  if(ser.VersionAtLeast(0x22))
+  {
+    rdcstr blockName;
+
+    if(ser.IsWriting())
+    {
+      GLenum prop = eGL_NAME_LENGTH;
+
+      GLint length = 1;
+      GL.glGetProgramResourceiv(program.name, eGL_SHADER_STORAGE_BLOCK, storageBlockIndex, 1, &prop,
+                                1, NULL, &length);
+
+      blockName.resize(length + 1);
+
+      GL.glGetProgramResourceName(program.name, eGL_SHADER_STORAGE_BLOCK, storageBlockIndex, length,
+                                  &length, blockName.data());
+
+      blockName.resize(strlen(blockName.c_str()));
+    }
+
+    SERIALISE_ELEMENT(blockName).Hidden();
+
+    if(IsReplayingAndReading())
+    {
+      GLuint idx =
+          GL.glGetProgramResourceIndex(program.name, eGL_SHADER_STORAGE_BLOCK, blockName.c_str());
+      if(idx != GL_INVALID_INDEX)
+        storageBlockIndex = idx;
+    }
+  }
+
   SERIALISE_CHECK_READ_ERRORS();
 
   if(IsReplayingAndReading())
@@ -993,19 +1052,15 @@ void WrappedOpenGL::glShaderStorageBlockBinding(GLuint program, GLuint storageBl
 {
   SERIALISE_TIME_CALL(GL.glShaderStorageBlockBinding(program, storageBlockIndex, storageBlockBinding));
 
-  if(IsCaptureMode(m_State))
+  // we should only capture this while active, since the initial states will grab everything at the
+  // start of the frame and we only want to pick up dynamic changes after that.
+  if(IsActiveCapturing(m_State))
   {
-    GLResourceRecord *record = GetResourceManager()->GetResourceRecord(ProgramRes(GetCtx(), program));
-    RDCASSERTMSG("Couldn't identify object passed to function. Mismatched or bad GLuint?", record,
-                 program);
-    if(record)
-    {
-      USE_SCRATCH_SERIALISER();
-      SCOPED_SERIALISE_CHUNK(gl_CurChunk);
-      Serialise_glShaderStorageBlockBinding(ser, program, storageBlockIndex, storageBlockBinding);
+    USE_SCRATCH_SERIALISER();
+    SCOPED_SERIALISE_CHUNK(gl_CurChunk);
+    Serialise_glShaderStorageBlockBinding(ser, program, storageBlockIndex, storageBlockBinding);
 
-      record->AddChunk(scope.Get());
-    }
+    GetContextRecord()->AddChunk(scope.Get());
   }
 }
 

util/test/demos/gl/gl_resource_lifetimes.cpp

@@ -33,7 +33,7 @@ RD_TEST(GL_Resource_Lifetimes, OpenGLGraphicsTest)
 
   std::string common = R"EOSHADER(
 
-#version 420 core
+#version 450 core
 
 #define v2f v2f_block \
 {                     \
@@ -61,10 +61,17 @@ layout(std140) uniform constsbuf
   vec4 flags;
 };
 
+layout(binding = 0, std430) buffer storebuffer
+{
+  vec4 data;
+} sbuf;
+
 uniform vec4 flags2;
 
 void main()
 {
+  sbuf.data = vec4(1,2,3,4);
+
 	vertOut.pos = vec4(Position.xyz, 1);
 	gl_Position = vertOut.pos;
 	vertOut.col = Color;
@@ -97,10 +104,17 @@ layout(std140) uniform constsbuf
   vec4 flags;
 };
 
+layout(binding = 0, std430) buffer storebuffer
+{
+  vec4 data;
+} sbuf;
+
 uniform vec4 flags2;
 
 void main()
 {
+  sbuf.data = vec4(1,2,3,4);
+
   if(flags.x != 1.0f || flags.y != 2.0f || flags.z != 4.0f || flags.w != 8.0f)
   {
     Color = vec4(1.0f, 0.0f, 1.0f, 1.0f);
@@ -215,6 +229,8 @@ void main()
 
       glUniformBlockBinding(prog, glGetUniformBlockIndex(prog, "constsbuf"), 5);
 
+      glShaderStorageBlockBinding(prog, 0, 3);
+
       const Vec4f flags = {1.0f, 2.0f, 4.0f, 8.0f};
 
       glProgramUniform4fv(prog, glGetUniformLocation(prog, "flags2"), 1, &flags.x);
@@ -240,6 +256,8 @@ void main()
 
       glUniformBlockBinding(prog, glGetUniformBlockIndex(prog, "constsbuf"), 5);
 
+      glShaderStorageBlockBinding(prog, 0, 3);
+
       const Vec4f flags = {1.0f, 2.0f, 4.0f, 8.0f};
 
       glProgramUniform4fv(prog, glGetUniformLocation(prog, "flags2"), 1, &flags.x);
@@ -250,6 +268,8 @@ void main()
     auto TrashProgram = [](GLuint prog) {
       glUniformBlockBinding(prog, glGetUniformBlockIndex(prog, "constsbuf"), 4);
 
+      glShaderStorageBlockBinding(prog, 0, 2);
+
       const Vec4f empty = {};
       glProgramUniform4fv(prog, glGetUniformLocation(prog, "flags2"), 1, &empty.x);
 
@@ -362,6 +382,15 @@ void main()
       glDeleteBuffers(1, &buf);
     };
 
+    {
+      const Vec4f empty = {};
+      GLuint buf = MakeBuffer();
+      glBindBuffer(GL_SHADER_STORAGE_BUFFER, buf);
+      glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(empty), &empty, GL_STATIC_DRAW);
+
+      glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 3, buf);
+    }
+
     glUseProgram(0);
     glViewport(0, 0, 128, 128);
 

util/test/tests/GL/GL_Resource_Lifetimes.py

@@ -7,6 +7,26 @@ class GL_Resource_Lifetimes(rdtest.TestCase):
     demos_frame_cap = 200
 
     def check_capture(self):
+        draw: rd.DrawcallDescription = self.find_draw("glDraw")
+
+        self.controller.SetFrameEvent(draw.eventId, True)
+
+        mapping: rd.ShaderBindpointMapping = self.controller.GetPipelineState().GetBindpointMapping(rd.ShaderStage.Vertex)
+        self.check(mapping.readWriteResources[0].bind == 3)
+
+        mapping: rd.ShaderBindpointMapping = self.controller.GetPipelineState().GetBindpointMapping(rd.ShaderStage.Pixel)
+        self.check(mapping.readWriteResources[0].bind == 3)
+
+        draw: rd.DrawcallDescription = self.find_draw("glDraw", draw.eventId+1)
+
+        self.controller.SetFrameEvent(draw.eventId, True)
+
+        mapping: rd.ShaderBindpointMapping = self.controller.GetPipelineState().GetBindpointMapping(rd.ShaderStage.Vertex)
+        self.check(mapping.readWriteResources[0].bind == 3)
+
+        mapping: rd.ShaderBindpointMapping = self.controller.GetPipelineState().GetBindpointMapping(rd.ShaderStage.Pixel)
+        self.check(mapping.readWriteResources[0].bind == 3)
+
         last_draw: rd.DrawcallDescription = self.get_last_draw()
 
         self.controller.SetFrameEvent(last_draw.eventId, True)