From feafbde8bca1e69d00fea18f31909f637a1a3fb3 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 2 Apr 2020 13:08:21 +0100 Subject: [PATCH] Add GL hack to check for implicit thread switching * This is a very big blunt hammer for fixing the problem of multithreaded submission from GL. Every GL call checks to see if the context changed (which would only happen from a thread switch to a different context) and if detected it inserts a manual MakeCurrent call equivalent. * It's slow to capture (when this happens - checking is not particularly slow) and slow to replay, but it's functional which is an improvement. --- renderdoc/driver/gl/gl_common.h | 2 + renderdoc/driver/gl/gl_driver.cpp | 38 +++ renderdoc/driver/gl/gl_driver.h | 5 + renderdoc/driver/gl/gl_hooks.cpp | 14 +- renderdoc/driver/gl/gl_stringise.cpp | 2 + util/test/demos/CMakeLists.txt | 1 + util/test/demos/demos.vcxproj | 1 + util/test/demos/demos.vcxproj.filters | 3 + .../demos/gl/gl_multithread_rendering.cpp | 221 ++++++++++++++++++ .../test/tests/GL/GL_Multithread_Rendering.py | 33 +++ 10 files changed, 314 insertions(+), 6 deletions(-) create mode 100644 util/test/demos/gl/gl_multithread_rendering.cpp create mode 100644 util/test/tests/GL/GL_Multithread_Rendering.py diff --git a/renderdoc/driver/gl/gl_common.h b/renderdoc/driver/gl/gl_common.h index 175019e60..8c6b92824 100644 --- a/renderdoc/driver/gl/gl_common.h +++ b/renderdoc/driver/gl/gl_common.h @@ -2196,6 +2196,8 @@ enum class GLChunk : uint32_t eglSwapBuffersWithDamageEXT, eglSwapBuffersWithDamageKHR, + ImplicitThreadSwitch, + Max, }; diff --git a/renderdoc/driver/gl/gl_driver.cpp b/renderdoc/driver/gl/gl_driver.cpp index e83f5fbc1..fdd2c1f05 100644 --- a/renderdoc/driver/gl/gl_driver.cpp +++ b/renderdoc/driver/gl/gl_driver.cpp @@ -993,6 +993,20 @@ GLResourceRecord *WrappedOpenGL::GetContextRecord() } } +void WrappedOpenGL::CheckImplicitThread() +{ + if(IsActiveCapturing(m_State) && m_LastCtx != GetCtx().ctx) + { + USE_SCRATCH_SERIALISER(); + SCOPED_SERIALISE_CHUNK(GLChunk::ImplicitThreadSwitch); + Serialise_ContextConfiguration(ser, m_LastCtx); + Serialise_BeginCaptureFrame(ser); + GetContextRecord()->AddChunk(scope.Get()); + + m_LastCtx = GetCtx().ctx; + } +} + WrappedOpenGL::ContextData &WrappedOpenGL::GetCtxData() { return m_ContextData[GetCtx().ctx]; @@ -1527,6 +1541,9 @@ void WrappedOpenGL::ActivateContext(GLWindowingData winData) Serialise_ContextConfiguration(ser, winData.ctx); GetContextRecord()->AddChunk(scope.Get()); } + + // update the last context so we don't record an implicit switch + m_LastCtx = GetCtx().ctx; } // we create these buffers last after serialising the apply of the new state, so that in the @@ -2179,6 +2196,8 @@ void WrappedOpenGL::StartFrameCapture(void *dev, void *wnd) AttemptCapture(); BeginCaptureFrame(); + m_LastCtx = GetCtx().ctx; + // serialise out the context configuration for this current context first { USE_SCRATCH_SERIALISER(); @@ -3321,6 +3340,16 @@ ReplayStatus WrappedOpenGL::ReadLogInitialisation(RDCFile *rdc, bool storeStruct break; } + if(m_ImplicitThreadSwitches > 2) + { + AddDebugMessage( + MessageCategory::Performance, MessageSeverity::Medium, MessageSource::GeneralPerformance, + StringFormat::Fmt( + "%d implicit thread switches detected. Multithreaded submission from GL is not " + "generally supported and is very inefficient to capture and replay.", + m_ImplicitThreadSwitches)); + } + #if ENABLED(RDOC_DEVEL) for(auto it = chunkInfos.begin(); it != chunkInfos.end(); ++it) { @@ -4544,6 +4573,15 @@ bool WrappedOpenGL::ProcessChunk(ReadSerialiser &ser, GLChunk chunk) // re-use the serialisation for beginning of the frame return Serialise_BeginCaptureFrame(ser); + case GLChunk::ImplicitThreadSwitch: + { + m_ImplicitThreadSwitches++; + bool ret = Serialise_ContextConfiguration(ser, NULL); + if(!ret) + return false; + return Serialise_BeginCaptureFrame(ser); + } + case GLChunk::ContextConfiguration: return Serialise_ContextConfiguration(ser, NULL); case GLChunk::glIndirectSubCommand: diff --git a/renderdoc/driver/gl/gl_driver.h b/renderdoc/driver/gl/gl_driver.h index 355036a53..bf4c8cf0c 100644 --- a/renderdoc/driver/gl/gl_driver.h +++ b/renderdoc/driver/gl/gl_driver.h @@ -173,6 +173,9 @@ private: static std::map m_ActiveContexts; + void *m_LastCtx; + int m_ImplicitThreadSwitches = 0; + GLContextTLSData m_EmptyTLSData; uint64_t m_CurCtxDataTLS; rdcarray m_CtxDataVector; @@ -596,6 +599,8 @@ public: ContextPair &GetCtx(); GLResourceRecord *GetContextRecord(); + void CheckImplicitThread(); + void CreateTextureImage(GLuint tex, GLenum internalFormat, GLenum internalFormatHint, GLenum textype, GLint dim, GLint width, GLint height, GLint depth, GLint samples, int mips); diff --git a/renderdoc/driver/gl/gl_hooks.cpp b/renderdoc/driver/gl/gl_hooks.cpp index 59ca22c65..68831e1b0 100644 --- a/renderdoc/driver/gl/gl_hooks.cpp +++ b/renderdoc/driver/gl/gl_hooks.cpp @@ -87,16 +87,18 @@ int ScopedPrinter::depth = 0; // This checks that we're not infinite looping by calling our own hooks from ourselves. Mostly // useful on android where you can only debug by printf and the stack dumps are often corrupted when // the callstack overflows. -#define SCOPED_GLCALL(funcname) \ - SCOPED_LOCK(glLock); \ - gl_CurChunk = GLChunk::funcname; \ +#define SCOPED_GLCALL(funcname) \ + SCOPED_LOCK(glLock); \ + gl_CurChunk = GLChunk::funcname; \ + glhook.driver->CheckImplicitThread(); \ ScopedPrinter CONCAT(scopedprint, __LINE__)(STRINGIZE(funcname)); #else -#define SCOPED_GLCALL(funcname) \ - SCOPED_LOCK(glLock); \ - gl_CurChunk = GLChunk::funcname; +#define SCOPED_GLCALL(funcname) \ + SCOPED_LOCK(glLock); \ + gl_CurChunk = GLChunk::funcname; \ + glhook.driver->CheckImplicitThread(); #endif diff --git a/renderdoc/driver/gl/gl_stringise.cpp b/renderdoc/driver/gl/gl_stringise.cpp index c77fa2186..4dd7dc97e 100644 --- a/renderdoc/driver/gl/gl_stringise.cpp +++ b/renderdoc/driver/gl/gl_stringise.cpp @@ -54,6 +54,8 @@ rdcstr DoStringise(const GLChunk &el) STRINGISE_ENUM_CLASS(eglSwapBuffersWithDamageEXT); STRINGISE_ENUM_CLASS(eglSwapBuffersWithDamageKHR); + STRINGISE_ENUM_CLASS_NAMED(ImplicitThreadSwitch, "Internal: Implicit thread context-switch"); + // re-use list of GL functions as chunks. Many of these will be aliased. This may not appear in the // same order as the definition, but that's OK. #define StringiseFunction(function, alias) STRINGISE_ENUM_CLASS_NAMED(alias, STRINGIZE(alias)); diff --git a/util/test/demos/CMakeLists.txt b/util/test/demos/CMakeLists.txt index 669507274..d0f7f3aa1 100644 --- a/util/test/demos/CMakeLists.txt +++ b/util/test/demos/CMakeLists.txt @@ -60,6 +60,7 @@ set(OPENGL_SRC gl/gl_midframe_context_create.cpp gl/gl_mip_gen_rt.cpp gl/gl_multi_window.cpp + gl/gl_multithread_rendering.cpp gl/gl_overlay_test.cpp gl/gl_parameter_zoo.cpp gl/gl_per_type_tex_units.cpp diff --git a/util/test/demos/demos.vcxproj b/util/test/demos/demos.vcxproj index 5c98acc42..d4d9043e2 100644 --- a/util/test/demos/demos.vcxproj +++ b/util/test/demos/demos.vcxproj @@ -213,6 +213,7 @@ + diff --git a/util/test/demos/demos.vcxproj.filters b/util/test/demos/demos.vcxproj.filters index 7db4a1c7c..a74fbd540 100644 --- a/util/test/demos/demos.vcxproj.filters +++ b/util/test/demos/demos.vcxproj.filters @@ -469,6 +469,9 @@ D3D11\demos + + OpenGL\demos + diff --git a/util/test/demos/gl/gl_multithread_rendering.cpp b/util/test/demos/gl/gl_multithread_rendering.cpp new file mode 100644 index 000000000..3b71de724 --- /dev/null +++ b/util/test/demos/gl/gl_multithread_rendering.cpp @@ -0,0 +1,221 @@ +/****************************************************************************** + * The MIT License (MIT) + * + * Copyright (c) 2019-2020 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 +#include +#include +#include "gl_test.h" + +RD_TEST(GL_Multithread_Rendering, OpenGLGraphicsTest) +{ + static constexpr const char *Description = + "Draws from two threads simultaneously, to test automatic catching of thread switching."; + + std::string pixel = R"EOSHADER( +#version 420 core + +in v2f_block +{ + vec4 pos; + vec4 col; + vec4 uv; +} vertIn; + +layout(location = 0, index = 0) out vec4 Color; + +void main() +{ + Color = vertIn.col; + Color.b = + +)EOSHADER"; + + int main() + { + // initialise, create window, create context, etc + if(!Init()) + return 3; + + const DefaultA2V GreenTri[3] = { + {Vec3f(-1.0f, -1.0f, 0.0f), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(0.0f, 0.0f)}, + {Vec3f(0.0f, 1.0f, 0.0f), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(0.0f, 1.0f)}, + {Vec3f(1.0f, -1.0f, 0.0f), Vec4f(0.0f, 1.0f, 0.0f, 1.0f), Vec2f(1.0f, 0.0f)}, + }; + + const DefaultA2V RedTri[3] = { + {Vec3f(-1.0f, 1.0f, 0.0f), Vec4f(1.0f, 0.0f, 0.0f, 1.0f), Vec2f(0.0f, 0.0f)}, + {Vec3f(1.0f, 1.0f, 0.0f), Vec4f(1.0f, 0.0f, 0.0f, 1.0f), Vec2f(1.0f, 0.0f)}, + {Vec3f(0.0f, -1.0f, 0.0f), Vec4f(1.0f, 0.0f, 0.0f, 1.0f), Vec2f(0.0f, 1.0f)}, + }; + + struct ctxdata + { + void *ctx; + + std::atomic_bool rendering = true; + + GLuint VB, VAO, prog, FBO, tex; + } A, B; + + A.VB = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, A.VB); + glBufferStorage(GL_ARRAY_BUFFER, sizeof(RedTri), RedTri, 0); + + A.prog = MakeProgram(GLDefaultVertex, pixel + "0.25f;\n}"); + + A.tex = MakeTexture(); + glBindTexture(GL_TEXTURE_2D, A.tex); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, screenWidth, screenHeight); + + B.VB = MakeBuffer(); + glBindBuffer(GL_ARRAY_BUFFER, B.VB); + glBufferStorage(GL_ARRAY_BUFFER, sizeof(GreenTri), GreenTri, 0); + + B.prog = MakeProgram(GLDefaultVertex, pixel + "0.75f;\n}"); + + B.tex = MakeTexture(); + glBindTexture(GL_TEXTURE_2D, B.tex); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, screenWidth, screenHeight); + + // make FBOs on the main context for reading + GLuint Afbo = MakeFBO(); + glBindFramebuffer(GL_FRAMEBUFFER, Afbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, A.tex, 0); + + GLuint Bfbo = MakeFBO(); + glBindFramebuffer(GL_FRAMEBUFFER, Bfbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, B.tex, 0); + + glBindFramebuffer(GL_FRAMEBUFFER, 0); + glBindTexture(GL_TEXTURE_2D, 0); + glBindBuffer(GL_ARRAY_BUFFER, 0); + + A.ctx = MakeContext(mainWindow, mainContext); + B.ctx = MakeContext(mainWindow, mainContext); + + std::atomic_bool quit = false; + + auto windowThread = [&](int idx) { + ctxdata &ctx = (idx == 0 ? A : B); + + ActivateContext(mainWindow, ctx.ctx); + + glGenVertexArrays(1, &ctx.VAO); + glBindVertexArray(ctx.VAO); + + glGenFramebuffers(1, &ctx.FBO); + glBindFramebuffer(GL_FRAMEBUFFER, ctx.FBO); + + while(true) + { + if(quit) + break; + + glBindFramebuffer(GL_FRAMEBUFFER, ctx.FBO); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, ctx.tex, 0); + float col[] = {0.2f + (1 - idx) * 0.1f, 0.2f + idx * 0.1f, 0.2f, 1.0f}; + glClearBufferfv(GL_COLOR, 0, col); + + const int div = 40; + + GLsizei w = GLsizei(screenWidth) / div; + GLsizei h = GLsizei(screenHeight) / div; + + for(GLsizei y = 0; y < div; y++) + { + for(GLsizei x = 0; x < div / 2; x++) + { + glBindFramebuffer(GL_FRAMEBUFFER, ctx.FBO); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, ctx.tex, 0); + glBindVertexArray(ctx.VAO); + glBindBuffer(GL_ARRAY_BUFFER, ctx.VB); + ConfigureDefaultVAO(); + glUseProgram(ctx.prog); + glViewport((GLsizei(idx * screenWidth) / 2) + w * x, h * y, w, h); + glDrawArrays(GL_TRIANGLES, 0, 3); + } + } + + glFinish(); + + // don't present until both contexts are done. This isn't necessary but ensures captures + // always start at the same point for both and only diverge within a frame. + ctx.rendering = false; + while(!quit && !ctx.rendering) + { + // busy loop waiting to be woken up. ha ha. + } + } + + glDeleteFramebuffers(1, &ctx.FBO); + glDeleteVertexArrays(1, &ctx.VAO); + + ActivateContext(mainWindow, NULL); + }; + + std::thread thread_A(windowThread, 0); + std::thread thread_B(windowThread, 1); + + while(Running()) + { + if(!A.rendering && !B.rendering) + { + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); + glBindFramebuffer(GL_READ_FRAMEBUFFER, Afbo); + + float black[4] = {}; + glClearBufferfv(GL_COLOR, 0, black); + + glBlitFramebuffer(0, 0, screenWidth / 2, screenHeight - 10, 0, 0, screenWidth / 2, + screenHeight - 10, GL_COLOR_BUFFER_BIT, GL_NEAREST); + + glBindFramebuffer(GL_READ_FRAMEBUFFER, Bfbo); + + glBlitFramebuffer(screenWidth / 2, 0, screenWidth, screenHeight - 10, screenWidth / 2, 0, + screenWidth, screenHeight - 10, GL_COLOR_BUFFER_BIT, GL_NEAREST); + + glBindFramebuffer(GL_FRAMEBUFFER, 0); + + glFinish(); + + Present(mainWindow); + + A.rendering = true; + B.rendering = true; + } + } + + quit = true; + + thread_A.join(); + thread_B.join(); + + DestroyContext(A.ctx); + DestroyContext(B.ctx); + + return 0; + } +}; + +REGISTER_TEST(); diff --git a/util/test/tests/GL/GL_Multithread_Rendering.py b/util/test/tests/GL/GL_Multithread_Rendering.py new file mode 100644 index 000000000..7a7e50ce1 --- /dev/null +++ b/util/test/tests/GL/GL_Multithread_Rendering.py @@ -0,0 +1,33 @@ +import renderdoc as rd +import rdtest + + +class GL_Multithread_Rendering(rdtest.TestCase): + demos_test_name = 'GL_Multithread_Rendering' + + def check_capture(self): + draw = self.get_last_draw() + + self.controller.SetFrameEvent(draw.eventId, False) + + pipe: rd.PipeState = self.controller.GetPipelineState() + + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 0.25, 0.0, [0.0, 0.0, 0.0, 0.0]) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 0.75, 0.0, [0.0, 0.0, 0.0, 0.0]) + + tex_details: rd.TextureDescription = self.get_texture(pipe.GetOutputTargets()[0].resourceId) + + w = int(tex_details.width / 40) + h = int(tex_details.height / 40) + + # Left side of the screen should be 20x40 red V shaped triangles on red background + for y in range(40): + for x in range(20): + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, int(x*w), int(tex_details.height - 1 - y*h), [0.3, 0.2, 0.2, 1.0]) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, int(x*w + w/2), int(tex_details.height - 1 - y*h - h/2), [1.0, 0.0, 0.25, 1.0]) + + # Right side of the screen should be delta shaped blue triangles + for y in range(40): + for x in range(20): + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, int(tex_details.width/2) + int(x*w), int(tex_details.height - 1 - y*h - (h-1)), [0.2, 0.3, 0.2, 1.0]) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, int(tex_details.width/2) + int(x*w + w/2), int(tex_details.height - 1 - y*h - h/2), [0.0, 1.0, 0.75, 1.0]) \ No newline at end of file