From e8b61fbf30b1f983eae70e033161d76b7799c155 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 17 Jul 2020 13:52:27 +0100 Subject: [PATCH] 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 | 5 ++ renderdoc/driver/gl/gl_driver.h | 2 +- .../driver/gl/wrappers/gl_shader_funcs.cpp | 77 ++++++++++++++++--- util/test/demos/gl/gl_resource_lifetimes.cpp | 31 +++++++- util/test/tests/GL/GL_Resource_Lifetimes.py | 20 +++++ 5 files changed, 122 insertions(+), 13 deletions(-) diff --git a/renderdoc/driver/gl/gl_common.cpp b/renderdoc/driver/gl/gl_common.cpp index 7a6a9ea65..04cd2362a 100644 --- a/renderdoc/driver/gl/gl_common.cpp +++ b/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; } diff --git a/renderdoc/driver/gl/gl_driver.h b/renderdoc/driver/gl/gl_driver.h index f637d718b..6beaa568a 100644 --- a/renderdoc/driver/gl/gl_driver.h +++ b/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); }; diff --git a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp index ac0fac64b..fa9c1a785 100644 --- a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp +++ b/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()); } } diff --git a/util/test/demos/gl/gl_resource_lifetimes.cpp b/util/test/demos/gl/gl_resource_lifetimes.cpp index 60588b27f..e0f814f98 100644 --- a/util/test/demos/gl/gl_resource_lifetimes.cpp +++ b/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); diff --git a/util/test/tests/GL/GL_Resource_Lifetimes.py b/util/test/tests/GL/GL_Resource_Lifetimes.py index ebd4c353f..bb9ac0fbf 100644 --- a/util/test/tests/GL/GL_Resource_Lifetimes.py +++ b/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)