From 84dabc7b5432791510b6192cf4da12485f4d5f65 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 18 Jan 2021 11:57:50 +0000 Subject: [PATCH] Update buffer size when it resizes mid-capture. Closes #2149 * We need to be careful with this, as we want to update the buffer's creation chunks without invalidating any data that may be present there. --- renderdoc/core/core.cpp | 14 ++ renderdoc/core/core.h | 2 + renderdoc/driver/gl/gl_driver.cpp | 21 ++ renderdoc/driver/gl/gl_driver.h | 2 + .../driver/gl/wrappers/gl_buffer_funcs.cpp | 37 +++- util/test/demos/CMakeLists.txt | 1 + util/test/demos/demos.vcxproj | 1 + util/test/demos/demos.vcxproj.filters | 3 + util/test/demos/gl/gl_buffer_resizing.cpp | 181 ++++++++++++++++++ util/test/tests/GL/GL_Buffer_Resizing.py | 56 ++++++ 10 files changed, 310 insertions(+), 8 deletions(-) create mode 100644 util/test/demos/gl/gl_buffer_resizing.cpp create mode 100644 util/test/tests/GL/GL_Buffer_Resizing.py diff --git a/renderdoc/core/core.cpp b/renderdoc/core/core.cpp index 0ae2230e8..9bbdd1b5c 100644 --- a/renderdoc/core/core.cpp +++ b/renderdoc/core/core.cpp @@ -149,6 +149,20 @@ INSTANTIATE_SERIALISE_TYPE(ResourceId); // from image_viewer.cpp ReplayStatus IMG_CreateReplayDevice(RDCFile *rdc, IReplayDriver **driver); +template <> +rdcstr DoStringise(const CaptureState &el) +{ + BEGIN_ENUM_STRINGISE(CaptureState); + { + STRINGISE_ENUM_CLASS(LoadingReplaying); + STRINGISE_ENUM_CLASS(ActiveReplaying); + STRINGISE_ENUM_CLASS(StructuredExport); + STRINGISE_ENUM_CLASS(BackgroundCapturing); + STRINGISE_ENUM_CLASS(ActiveCapturing); + } + END_ENUM_STRINGISE(); +} + template <> rdcstr DoStringise(const RDCDriver &el) { diff --git a/renderdoc/core/core.h b/renderdoc/core/core.h index 1797b5207..7343d64b2 100644 --- a/renderdoc/core/core.h +++ b/renderdoc/core/core.h @@ -124,6 +124,8 @@ enum class CaptureState ActiveCapturing, }; +DECLARE_REFLECTION_ENUM(CaptureState); + constexpr inline bool IsReplayMode(CaptureState state) { return state == CaptureState::LoadingReplaying || state == CaptureState::ActiveReplaying; diff --git a/renderdoc/driver/gl/gl_driver.cpp b/renderdoc/driver/gl/gl_driver.cpp index 03dbb5d75..d9557d460 100644 --- a/renderdoc/driver/gl/gl_driver.cpp +++ b/renderdoc/driver/gl/gl_driver.cpp @@ -2412,6 +2412,13 @@ bool WrappedOpenGL::EndFrameCapture(void *dev, void *wnd) record->FreeShadowStorage(); } + for(const rdcpair &r : m_BufferResizes) + { + r.first->AddChunk(r.second); + r.first->SetDataPtr(r.second->GetData()); + } + m_BufferResizes.clear(); + // if we changed contexts above, pop back to where we were if(pushChildSaved) { @@ -2468,6 +2475,13 @@ bool WrappedOpenGL::EndFrameCapture(void *dev, void *wnd) record->FreeShadowStorage(); } + for(const rdcpair &r : m_BufferResizes) + { + r.first->AddChunk(r.second); + r.first->SetDataPtr(r.second->GetData()); + } + m_BufferResizes.clear(); + // if it's a capture triggered from application code, immediately // give up as it's not reasonable to expect applications to detect and retry. // otherwise we can retry in case the next frame works. @@ -2542,6 +2556,13 @@ bool WrappedOpenGL::DiscardFrameCapture(void *dev, void *wnd) record->FreeShadowStorage(); } + for(const rdcpair &r : m_BufferResizes) + { + r.first->AddChunk(r.second); + r.first->SetDataPtr(r.second->GetData()); + } + m_BufferResizes.clear(); + m_CapturedFrames.pop_back(); FreeCaptureData(); diff --git a/renderdoc/driver/gl/gl_driver.h b/renderdoc/driver/gl/gl_driver.h index c29c7c8bd..42997415b 100644 --- a/renderdoc/driver/gl/gl_driver.h +++ b/renderdoc/driver/gl/gl_driver.h @@ -187,6 +187,8 @@ private: std::set m_AcceptedCtx; + rdcarray> m_BufferResizes; + public: enum { diff --git a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp index f3dc4a9e4..07554165e 100644 --- a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp @@ -679,13 +679,15 @@ void WrappedOpenGL::glNamedBufferDataEXT(GLuint buffer, GLsizeiptr size, const v return; } + const bool isResizingOrphan = + (record->HasDataPtr() || (record->Length > 0 && size != (GLsizeiptr)record->Length)); + // if we're recreating the buffer, clear the record and add new chunks. Normally // we would just mark this record as dirty and pick it up on the capture frame as initial // data, but we don't support (if it's even possible) querying out size etc. // we need to add only the chunks required - glGenBuffers, glBindBuffer to current target, // and this buffer storage. All other chunks have no effect - if(IsBackgroundCapturing(m_State) && - (record->HasDataPtr() || (record->Length > 0 && size != (GLsizeiptr)record->Length))) + if(IsBackgroundCapturing(m_State) && isResizingOrphan) { // we need to maintain chunk ordering, so fetch the first two chunk IDs. // We should have at least two by this point - glGenBuffers and whatever gave the record @@ -753,15 +755,26 @@ void WrappedOpenGL::glNamedBufferDataEXT(GLuint buffer, GLsizeiptr size, const v GetContextRecord()->AddChunk(chunk); GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_PartialWrite); + + // if this is a resizing call, we also need to store a copy in the record so future captures + // have an accurate creation chunk. However we can't do that yet because this buffer may not + // have initial contents. If we store the chunk immediately we'd corrupt data potentially used + // earlier in the captured frame from the previous creation chunk :(. So push it into a list + // that we'll 'apply' at the end of the frame capture. + if(isResizingOrphan) + m_BufferResizes.push_back({record, chunk->Duplicate()}); } else { record->AddChunk(chunk); record->SetDataPtr(chunk->GetData()); - record->Length = (int32_t)size; - record->usage = usage; record->DataInSerialiser = true; } + + // always update length and usage even during capture. If buffers resize mid-capture we'll + // record them both into the active frame and the record, but we need an up to date length. + record->Length = (int32_t)size; + record->usage = usage; } else { @@ -830,13 +843,15 @@ void WrappedOpenGL::glBufferData(GLenum target, GLsizeiptr size, const void *dat GLuint buffer = record->Resource.name; + const bool isResizingOrphan = + (record->HasDataPtr() || (record->Length > 0 && size != (GLsizeiptr)record->Length)); + // if we're recreating the buffer, clear the record and add new chunks. Normally // we would just mark this record as dirty and pick it up on the capture frame as initial // data, but we don't support (if it's even possible) querying out size etc. // we need to add only the chunks required - glGenBuffers, glBindBuffer to current target, // and this buffer storage. All other chunks have no effect - if(IsBackgroundCapturing(m_State) && - (record->HasDataPtr() || (record->Length > 0 && size != (GLsizeiptr)record->Length))) + if(IsBackgroundCapturing(m_State) && isResizingOrphan) { // we need to maintain chunk ordering, so fetch the first two chunk IDs. // We should have at least two by this point - glGenBuffers and whatever gave the record @@ -904,13 +919,16 @@ void WrappedOpenGL::glBufferData(GLenum target, GLsizeiptr size, const void *dat GetContextRecord()->AddChunk(chunk); GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_PartialWrite); + + // if this is a resizing call, also store a copy in the record so future captures have an + // accurate creation chunk + if(isResizingOrphan) + m_BufferResizes.push_back({record, chunk->Duplicate()}); } else { record->AddChunk(chunk); record->SetDataPtr(chunk->GetData()); - record->Length = size; - record->usage = usage; record->DataInSerialiser = true; // if we're active capturing then we need to add a duplicate call in so that the data is @@ -922,6 +940,9 @@ void WrappedOpenGL::glBufferData(GLenum target, GLsizeiptr size, const void *dat eFrameRef_PartialWrite); } } + + record->Length = size; + record->usage = usage; } else { diff --git a/util/test/demos/CMakeLists.txt b/util/test/demos/CMakeLists.txt index d704750db..d062e059c 100644 --- a/util/test/demos/CMakeLists.txt +++ b/util/test/demos/CMakeLists.txt @@ -59,6 +59,7 @@ set(OPENGL_SRC 3rdparty/glad/glad_glx.c gl/gl_test.cpp gl/gl_test_linux.cpp + gl/gl_buffer_resizing.cpp gl/gl_buffer_spam.cpp gl/gl_buffer_truncation.cpp gl/gl_buffer_updates.cpp diff --git a/util/test/demos/demos.vcxproj b/util/test/demos/demos.vcxproj index c7c537369..a86ab7ee3 100644 --- a/util/test/demos/demos.vcxproj +++ b/util/test/demos/demos.vcxproj @@ -217,6 +217,7 @@ true + diff --git a/util/test/demos/demos.vcxproj.filters b/util/test/demos/demos.vcxproj.filters index 4ae0a3a3f..a2daca41b 100644 --- a/util/test/demos/demos.vcxproj.filters +++ b/util/test/demos/demos.vcxproj.filters @@ -571,6 +571,9 @@ D3D11\demos + + OpenGL\demos + diff --git a/util/test/demos/gl/gl_buffer_resizing.cpp b/util/test/demos/gl/gl_buffer_resizing.cpp new file mode 100644 index 000000000..1f19d0c1f --- /dev/null +++ b/util/test/demos/gl/gl_buffer_resizing.cpp @@ -0,0 +1,181 @@ +/****************************************************************************** + * The MIT License (MIT) + * + * Copyright (c) 2019-2021 Baldur Karlsson + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + ******************************************************************************/ + +#include "gl_test.h" + +RD_TEST(GL_Buffer_Resizing, OpenGLGraphicsTest) +{ + static constexpr const char *Description = + "Test that buffer resizing is handled correctly, both out of frame and in-frame."; + + int main() + { + // initialise, create window, create context, etc + if(!Init()) + return 3; + + GLuint vao = MakeVAO(); + glBindVertexArray(vao); + + GLuint vbs[10]; + int i = 0; + + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + + // create the buffer initially too small + glBufferData(GL_ARRAY_BUFFER, 4, NULL, GL_DYNAMIC_DRAW); + + // then resize it up while at init time, to ensure we handle this correctly + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), DefaultTri, GL_DYNAMIC_DRAW); + + // while harmless, test that we can resize *down* as well. + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri) * 10, NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), DefaultTri, GL_DYNAMIC_DRAW); + + // these will be resized in-frame + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, 4, NULL, GL_DYNAMIC_DRAW); + + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri) * 10, NULL, GL_DYNAMIC_DRAW); + + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, 4, NULL, GL_DYNAMIC_DRAW); + + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, 1000, NULL, GL_DYNAMIC_DRAW); + + vbs[i] = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, 4, NULL, GL_DYNAMIC_DRAW); + + TEST_ASSERT(i < ARRAY_COUNT(vbs), "Make vbs[] bigger"); + + GLuint program = MakeProgram(GLDefaultVertex, GLDefaultPixel); + glUseProgram(program); + + glViewport(0, 0, GLsizei(screenWidth), GLsizei(screenHeight)); + + float col[] = {0.2f, 0.2f, 0.2f, 1.0f}; + + while(Running()) + { + i = 0; + // check these VBs are OK + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + ConfigureDefaultVAO(); + glDrawArrays(GL_TRIANGLES, 0, 3); + + // check these VBs are OK + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + ConfigureDefaultVAO(); + glDrawArrays(GL_TRIANGLES, 0, 3); + + if(curFrame == 10) + { + // resize this VB up to size in the captured frame + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), DefaultTri, GL_DYNAMIC_DRAW); + ConfigureDefaultVAO(); + if(glGetError() == GL_NO_ERROR) + glDrawArrays(GL_TRIANGLES, 0, 3); + else + glDrawArrays(GL_TRIANGLES, 0, 0); + + // resize this VB down to size in the captured frame + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), DefaultTri, GL_DYNAMIC_DRAW); + ConfigureDefaultVAO(); + if(glGetError() == GL_NO_ERROR) + glDrawArrays(GL_TRIANGLES, 0, 3); + else + glDrawArrays(GL_TRIANGLES, 0, 0); + + // resize this VB several times in the captured frame + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, 16, NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, 8, NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, 8, NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, 9999, NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), DefaultTri, GL_DYNAMIC_DRAW); + ConfigureDefaultVAO(); + if(glGetError() == GL_NO_ERROR) + glDrawArrays(GL_TRIANGLES, 0, 3); + else + glDrawArrays(GL_TRIANGLES, 0, 0); + + // resize down and map this VB + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), NULL, GL_DYNAMIC_DRAW); + void *ptr = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY); + memcpy(ptr, DefaultTri, sizeof(DefaultTri)); + glUnmapBuffer(GL_ARRAY_BUFFER); + ConfigureDefaultVAO(); + if(glGetError() == GL_NO_ERROR) + glDrawArrays(GL_TRIANGLES, 0, 3); + else + glDrawArrays(GL_TRIANGLES, 0, 0); + + // resize up and map this VB + glClearBufferfv(GL_COLOR, 0, col); + glBindBuffer(GL_ARRAY_BUFFER, vbs[i++]); + glBufferData(GL_ARRAY_BUFFER, sizeof(DefaultTri), NULL, GL_DYNAMIC_DRAW); + ptr = glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY); + memcpy(ptr, DefaultTri, sizeof(DefaultTri)); + glUnmapBuffer(GL_ARRAY_BUFFER); + ConfigureDefaultVAO(); + if(glGetError() == GL_NO_ERROR) + glDrawArrays(GL_TRIANGLES, 0, 3); + else + glDrawArrays(GL_TRIANGLES, 0, 0); + + // now trash the VBs that had important data at the start of the frame, to ensure that this + // resize doesn't invalid any of the data that was in them and used. + glBindBuffer(GL_ARRAY_BUFFER, vbs[0]); + glBufferData(GL_ARRAY_BUFFER, 50, NULL, GL_DYNAMIC_DRAW); + glBindBuffer(GL_ARRAY_BUFFER, vbs[1]); + glBufferData(GL_ARRAY_BUFFER, 50, NULL, GL_DYNAMIC_DRAW); + } + + Present(); + } + + return 0; + } +}; + +REGISTER_TEST(); diff --git a/util/test/tests/GL/GL_Buffer_Resizing.py b/util/test/tests/GL/GL_Buffer_Resizing.py new file mode 100644 index 000000000..6fb259171 --- /dev/null +++ b/util/test/tests/GL/GL_Buffer_Resizing.py @@ -0,0 +1,56 @@ +import renderdoc as rd +import rdtest + + +class GL_Buffer_Resizing(rdtest.TestCase): + demos_test_name = 'GL_Buffer_Resizing' + demos_frame_cap = 10 + + def check_capture(self): + postvs_ref = { + 0: { + 'vtx': 0, + 'idx': 0, + 'gl_Position': [-0.5, -0.5, 0.0, 1.0], + 'v2f_block.pos': [-0.5, -0.5, 0.0, 1.0], + 'v2f_block.col': [0.0, 1.0, 0.0, 1.0], + 'v2f_block.uv': [0.0, 0.0, 0.0, 1.0], + }, + 1: { + 'vtx': 1, + 'idx': 1, + 'gl_Position': [0.0, 0.5, 0.0, 1.0], + 'v2f_block.pos': [0.0, 0.5, 0.0, 1.0], + 'v2f_block.col': [0.0, 1.0, 0.0, 1.0], + 'v2f_block.uv': [0.0, 1.0, 0.0, 1.0], + }, + 2: { + 'vtx': 2, + 'idx': 2, + 'gl_Position': [0.5, -0.5, 0.0, 1.0], + 'v2f_block.pos': [0.5, -0.5, 0.0, 1.0], + 'v2f_block.col': [0.0, 1.0, 0.0, 1.0], + 'v2f_block.uv': [1.0, 0.0, 0.0, 1.0], + }, + } + + draw = self.get_first_draw() + + idx = 0 + + while True: + draw: rd.DrawcallDescription = self.find_draw('glDraw', draw.eventId+1) + + if draw is None: + break + + self.controller.SetFrameEvent(draw.eventId, True) + + self.check_triangle(out=draw.outputs[0]) + + postvs_data = self.get_postvs(draw, rd.MeshDataStage.VSOut, 0, draw.numIndices) + + self.check_mesh_data(postvs_ref, postvs_data) + + idx = idx + 1 + rdtest.log.success('Draw {} at {} is correct'.format(idx, draw.eventId))