diff --git a/Common/GPU/OpenGL/GLRenderManager.cpp b/Common/GPU/OpenGL/GLRenderManager.cpp index e6385773a8..50216756b5 100644 --- a/Common/GPU/OpenGL/GLRenderManager.cpp +++ b/Common/GPU/OpenGL/GLRenderManager.cpp @@ -397,10 +397,13 @@ void GLRenderManager::Finish() { task->frame = curFrame; { std::unique_lock lock(pushMutex_); - task->initSteps = std::move(initSteps_); + { + std::lock_guard initLock(initStepsMutex_); + task->initSteps = std::move(initSteps_); + initSteps_.clear(); + } task->steps = std::move(steps_); renderThreadQueue_.push(task); - initSteps_.clear(); steps_.clear(); pushCondVar_.notify_one(); } @@ -474,6 +477,7 @@ bool GLRenderManager::Run(GLRRenderThreadTask &task) { // Run this after RunInitSteps so any fresh GLRBuffers for the pushbuffers can get created. if (!skipGLCalls_) { + std::lock_guard lock(pushBuffersMutex_); for (auto iter : frameData.activePushBuffers) { iter->Flush(); iter->UnmapDevice(); @@ -500,6 +504,7 @@ bool GLRenderManager::Run(GLRRenderThreadTask &task) { } if (!skipGLCalls_) { + std::lock_guard lock(pushBuffersMutex_); for (auto iter : frameData.activePushBuffers) { iter->MapDevice(bufferStrategy_); } @@ -537,7 +542,11 @@ void GLRenderManager::FlushSync() { std::unique_lock lock(pushMutex_); renderThreadQueue_.push(task); - renderThreadQueue_.back()->initSteps = std::move(initSteps_); + { + std::lock_guard initLock(initStepsMutex_); + renderThreadQueue_.back()->initSteps = std::move(initSteps_); + initSteps_.clear(); + } renderThreadQueue_.back()->steps = std::move(steps_); pushCondVar_.notify_one(); steps_.clear(); diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h index 805a55fca8..a58a49c3fb 100644 --- a/Common/GPU/OpenGL/GLRenderManager.h +++ b/Common/GPU/OpenGL/GLRenderManager.h @@ -263,6 +263,7 @@ public: // and then we'll also need formats and stuff. GLRTexture *CreateTexture(GLenum target, int width, int height, int depth, int numMips) { _dbg_assert_(target != 0); + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_TEXTURE; step.create_texture.texture = new GLRTexture(caps_, width, height, depth, numMips); @@ -271,6 +272,7 @@ public: } GLRBuffer *CreateBuffer(GLuint target, size_t size, GLuint usage) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_BUFFER; step.create_buffer.buffer = new GLRBuffer(target, size); @@ -280,6 +282,7 @@ public: } GLRShader *CreateShader(GLuint stage, const std::string &code, std::string_view desc) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_SHADER; step.create_shader.shader = new GLRShader(desc); @@ -292,6 +295,7 @@ public: GLRFramebuffer *CreateFramebuffer(int width, int height, bool z_stencil, const char *tag) { _dbg_assert_(width > 0 && height > 0 && tag != nullptr); + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_FRAMEBUFFER; step.create_framebuffer.framebuffer = new GLRFramebuffer(caps_, width, height, z_stencil, tag); @@ -303,6 +307,7 @@ public: GLRProgram *CreateProgram( std::vector shaders, std::vector semantics, std::vector queries, std::vector initializers, GLRProgramLocData *locData, const GLRProgramFlags &flags) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_PROGRAM; _assert_(shaders.size() <= ARRAY_SIZE(step.create_program.shaders)); @@ -332,6 +337,7 @@ public: } GLRInputLayout *CreateInputLayout(const std::vector &entries, int stride) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::CREATE_INPUT_LAYOUT; step.create_input_layout.inputLayout = new GLRInputLayout(); @@ -411,6 +417,7 @@ public: void BufferSubdata(GLRBuffer *buffer, size_t offset, size_t size, uint8_t *data, bool deleteData = true) { // TODO: Maybe should be a render command instead of an init command? When possible it's better as // an init command, that's for sure. + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::BUFFER_SUBDATA; _dbg_assert_(offset <= buffer->size_ - size); @@ -423,6 +430,7 @@ public: // Takes ownership over the data pointer and delete[]-s it. void TextureImage(GLRTexture *texture, int level, int width, int height, int depth, Draw::DataFormat format, uint8_t *data, GLRAllocType allocType = GLRAllocType::NEW, bool linearFilter = false) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::TEXTURE_IMAGE; step.texture_image.texture = texture; @@ -453,6 +461,7 @@ public: } void FinalizeTexture(GLRTexture *texture, int loadedLevels, bool genMips) { + std::lock_guard lock(initStepsMutex_); GLRInitStep &step = initSteps_.push_uninitialized(); step.stepType = GLRInitStepType::TEXTURE_FINALIZE; step.texture_finalize.texture = texture; @@ -800,6 +809,7 @@ public: } void UnregisterPushBuffer(GLPushBuffer *buffer) { + std::lock_guard lock(pushBuffersMutex_); int foundCount = 0; for (int i = 0; i < MAX_INFLIGHT_FRAMES; i++) { auto iter = frameData_[i].activePushBuffers.find(buffer); @@ -850,6 +860,7 @@ private: // When using legacy functionality for push buffers (glBufferData), we need to flush them // before actually making the glDraw* calls. It's best if the render manager handles that. void RegisterPushBuffer(int frame, GLPushBuffer *buffer) { + std::lock_guard lock(pushBuffersMutex_); frameData_[frame].activePushBuffers.insert(buffer); } @@ -860,8 +871,18 @@ private: GLRStep *curRenderStep_ = nullptr; std::vector steps_; + // Guards initSteps_. Recorded into from the emu thread, but also from the loader thread during + // boot (InitGPU runs there, and GL has to record device object creation rather than just doing + // it), and moved out on the emu thread in Finish/FlushSync. Uncontended in practice. + // Lock ordering: taken while pushMutex_ is held, never the other way around. + std::mutex initStepsMutex_; FastVec initSteps_; + // Guards frameData_[].activePushBuffers, which is inserted into from whichever thread creates a + // push buffer, erased from on the render thread (GLDeleter), and walked on the render thread. + // Lock ordering: taken before initStepsMutex_ (Flush() below records init steps), never after. + std::mutex pushBuffersMutex_; + // Execution time state // Thread is managed elsewhere, and should call ThreadFrame.