From 1d6263d5eb42c38a79f4e9b472d2dbb9b2c111da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 11:25:49 +0200 Subject: [PATCH 1/8] More GL shutdown fixes. Partially fixes #10868 --- GPU/GLES/GPU_GLES.cpp | 13 +++++++------ GPU/GLES/TextureCacheGLES.cpp | 21 ++++++++++++++++++++- GPU/GLES/TextureCacheGLES.h | 3 ++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/GPU/GLES/GPU_GLES.cpp b/GPU/GLES/GPU_GLES.cpp index ca64acae56..6fe38db384 100644 --- a/GPU/GLES/GPU_GLES.cpp +++ b/GPU/GLES/GPU_GLES.cpp @@ -118,8 +118,10 @@ GPU_GLES::GPU_GLES(GraphicsContext *gfxCtx, Draw::DrawContext *draw) } GPU_GLES::~GPU_GLES() { - GLRenderManager *render = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); - render->Wipe(); + if (draw_) { + GLRenderManager *render = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); + render->Wipe(); + } // If we're here during app shutdown (exiting the Windows app in-game, for example) // everything should already be cleared since DeviceLost has been run. @@ -135,9 +137,6 @@ GPU_GLES::~GPU_GLES() { shaderManagerGL_ = nullptr; delete framebufferManagerGL_; delete textureCacheGL_; -#ifdef _WIN32 - gfxCtx_->SwapInterval(0); -#endif } static constexpr int MakeIntelSimpleVer(int v1, int v2, int v3) { @@ -332,11 +331,13 @@ void GPU_GLES::DeviceLost() { // TransformDraw has registered as a GfxResourceHolder. drawEngine_.ClearInputLayoutMap(); shaderManagerGL_->ClearCache(false); - textureCacheGL_->Clear(false); + textureCacheGL_->DeviceLost(); fragmentTestCache_.Clear(false); depalShaderCache_.Clear(); drawEngine_.DeviceLost(); framebufferManagerGL_->DeviceLost(); + // Don't even try to access the lost device. + draw_ = nullptr; } void GPU_GLES::DeviceRestore() { diff --git a/GPU/GLES/TextureCacheGLES.cpp b/GPU/GLES/TextureCacheGLES.cpp index e1b45f40c4..ed802319a8 100644 --- a/GPU/GLES/TextureCacheGLES.cpp +++ b/GPU/GLES/TextureCacheGLES.cpp @@ -53,6 +53,7 @@ TextureCacheGLES::TextureCacheGLES(Draw::DrawContext *draw) SetupTextureDecoder(); nextTexture_ = nullptr; + std::vector entries; entries.push_back({ 0, 3, GL_FLOAT, GL_FALSE, 20, 0 }); entries.push_back({ 1, 2, GL_FLOAT, GL_FALSE, 20, 12 }); @@ -60,7 +61,9 @@ TextureCacheGLES::TextureCacheGLES(Draw::DrawContext *draw) } TextureCacheGLES::~TextureCacheGLES() { - render_->DeleteInputLayout(shadeInputLayout_); + if (shadeInputLayout_) { + render_->DeleteInputLayout(shadeInputLayout_); + } Clear(true); } @@ -839,6 +842,22 @@ bool TextureCacheGLES::GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level) #endif } +void TextureCacheGLES::DeviceLost() { + if (shadeInputLayout_) { + render_->DeleteInputLayout(shadeInputLayout_); + shadeInputLayout_ = nullptr; + } + Clear(false); + draw_ = nullptr; + render_ = nullptr; +} + void TextureCacheGLES::DeviceRestore(Draw::DrawContext *draw) { draw_ = draw; + if (!shadeInputLayout_) { + std::vector entries; + entries.push_back({ 0, 3, GL_FLOAT, GL_FALSE, 20, 0 }); + entries.push_back({ 1, 2, GL_FLOAT, GL_FALSE, 20, 12 }); + shadeInputLayout_ = render_->CreateInputLayout(entries); + } } diff --git a/GPU/GLES/TextureCacheGLES.h b/GPU/GLES/TextureCacheGLES.h index 36f7e600e0..a88eb8d7ec 100644 --- a/GPU/GLES/TextureCacheGLES.h +++ b/GPU/GLES/TextureCacheGLES.h @@ -66,6 +66,7 @@ public: void SetFramebufferSamplingParams(u16 bufferWidth, u16 bufferHeight); bool GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level) override; + void DeviceLost(); void DeviceRestore(Draw::DrawContext *draw); protected: @@ -95,7 +96,7 @@ private: ShaderManagerGLES *shaderManager_; DrawEngineGLES *drawEngine_; - GLRInputLayout *shadeInputLayout_; + GLRInputLayout *shadeInputLayout_ = nullptr; enum { INVALID_TEX = -1 }; }; From 513ea72d5db8ec25c1fbfb157b0b1f71b3b64f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 12:26:58 +0200 Subject: [PATCH 2/8] OpenGL debug log: Filter out some buffer mapping info from the NV driver --- Windows/GPU/WindowsGLContext.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Windows/GPU/WindowsGLContext.cpp b/Windows/GPU/WindowsGLContext.cpp index bed1349cf3..43eeaca913 100644 --- a/Windows/GPU/WindowsGLContext.cpp +++ b/Windows/GPU/WindowsGLContext.cpp @@ -85,8 +85,9 @@ void WindowsGLContext::Resume() { void FormatDebugOutputARB(char outStr[], size_t outStrSize, GLenum source, GLenum type, GLuint id, GLenum severity, const char *msg) { + char sourceStr[32]; - const char *sourceFmt = "UNDEFINED(0x%04X)"; + const char *sourceFmt; switch(source) { case GL_DEBUG_SOURCE_API_ARB: sourceFmt = "API"; break; case GL_DEBUG_SOURCE_WINDOW_SYSTEM_ARB: sourceFmt = "WINDOW_SYSTEM"; break; @@ -94,11 +95,12 @@ void FormatDebugOutputARB(char outStr[], size_t outStrSize, GLenum source, GLenu case GL_DEBUG_SOURCE_THIRD_PARTY_ARB: sourceFmt = "THIRD_PARTY"; break; case GL_DEBUG_SOURCE_APPLICATION_ARB: sourceFmt = "APPLICATION"; break; case GL_DEBUG_SOURCE_OTHER_ARB: sourceFmt = "OTHER"; break; + default: sourceFmt = "UNDEFINED(0x%04X)"; break; } snprintf(sourceStr, sizeof(sourceStr), sourceFmt, source); char typeStr[32]; - const char *typeFmt = "UNDEFINED(0x%04X)"; + const char *typeFmt; switch(type) { case GL_DEBUG_TYPE_ERROR_ARB: typeFmt = "ERROR"; break; case GL_DEBUG_TYPE_DEPRECATED_BEHAVIOR_ARB: typeFmt = "DEPRECATED_BEHAVIOR"; break; @@ -106,15 +108,17 @@ void FormatDebugOutputARB(char outStr[], size_t outStrSize, GLenum source, GLenu case GL_DEBUG_TYPE_PORTABILITY_ARB: typeFmt = "PORTABILITY"; break; case GL_DEBUG_TYPE_PERFORMANCE_ARB: typeFmt = "PERFORMANCE"; break; case GL_DEBUG_TYPE_OTHER_ARB: typeFmt = "OTHER"; break; + default: typeFmt = "UNDEFINED(0x%04X)"; break; } snprintf(typeStr, sizeof(typeStr), typeFmt, type); char severityStr[32]; - const char *severityFmt = "UNDEFINED"; + const char *severityFmt; switch (severity) { case GL_DEBUG_SEVERITY_HIGH_ARB: severityFmt = "HIGH"; break; case GL_DEBUG_SEVERITY_MEDIUM_ARB: severityFmt = "MEDIUM"; break; case GL_DEBUG_SEVERITY_LOW_ARB: severityFmt = "LOW"; break; + default: severityFmt = "UNDEFINED(%d)"; break; } snprintf(severityStr, sizeof(severityStr), severityFmt, severity); @@ -123,6 +127,10 @@ void FormatDebugOutputARB(char outStr[], size_t outStrSize, GLenum source, GLenu void DebugCallbackARB(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar *message, GLvoid *userParam) { + // Ignore buffer mapping messages from NVIDIA + if (source == GL_DEBUG_SOURCE_API_ARB && type == GL_DEBUG_TYPE_OTHER_ARB && id == 131185) + return; + (void)length; FILE *outFile = (FILE *)userParam; char finalMessage[1024]; From 8d6bbc54a9e86a1a249abf4c9214b803129c6fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 17:31:12 +0200 Subject: [PATCH 3/8] Minor refactoring moving creation/deletion of GLPushBuffer to GLRenderManager --- GPU/GLES/DrawEngineGLES.cpp | 10 +- ext/native/thin3d/GLRenderManager.cpp | 3 +- ext/native/thin3d/GLRenderManager.h | 245 +++++++++++++------------- ext/native/thin3d/thin3d_gl.cpp | 5 +- 4 files changed, 134 insertions(+), 129 deletions(-) diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index 24c84b616a..b0dc1e946f 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -109,8 +109,8 @@ void DrawEngineGLES::DeviceRestore() { void DrawEngineGLES::InitDeviceObjects() { for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - frameData_[i].pushVertex = new GLPushBuffer(render_, GL_ARRAY_BUFFER, 1024 * 1024); - frameData_[i].pushIndex = new GLPushBuffer(render_, GL_ELEMENT_ARRAY_BUFFER, 256 * 1024); + frameData_[i].pushVertex = render_->CreatePushBuffer(GL_ARRAY_BUFFER, 1024 * 1024); + frameData_[i].pushIndex = render_->CreatePushBuffer(GL_ELEMENT_ARRAY_BUFFER, 256 * 1024); render_->RegisterPushBuffer(i, frameData_[i].pushVertex); render_->RegisterPushBuffer(i, frameData_[i].pushIndex); @@ -133,10 +133,8 @@ void DrawEngineGLES::DestroyDeviceObjects() { render_->UnregisterPushBuffer(i, frameData_[i].pushVertex); render_->UnregisterPushBuffer(i, frameData_[i].pushIndex); - frameData_[i].pushVertex->Destroy(); - frameData_[i].pushIndex->Destroy(); - delete frameData_[i].pushVertex; - delete frameData_[i].pushIndex; + render_->DeletePushBuffer(frameData_[i].pushVertex); + render_->DeletePushBuffer(frameData_[i].pushIndex); frameData_[i].pushVertex = nullptr; frameData_[i].pushIndex = nullptr; } diff --git a/ext/native/thin3d/GLRenderManager.cpp b/ext/native/thin3d/GLRenderManager.cpp index d5f648b86a..f7cc21e87d 100644 --- a/ext/native/thin3d/GLRenderManager.cpp +++ b/ext/native/thin3d/GLRenderManager.cpp @@ -568,7 +568,7 @@ GLPushBuffer::GLPushBuffer(GLRenderManager *render, GLuint target, size_t size) } GLPushBuffer::~GLPushBuffer() { - assert(buffers_.empty()); + Destroy(); } void GLPushBuffer::Map() { @@ -645,6 +645,7 @@ void GLPushBuffer::Destroy() { FreeAlignedMemory(info.localMemory); } buffers_.clear(); + buf_ = -1; } void GLPushBuffer::NextBuffer(size_t minSize) { diff --git a/ext/native/thin3d/GLRenderManager.h b/ext/native/thin3d/GLRenderManager.h index 37279ec750..953af0bfbf 100644 --- a/ext/native/thin3d/GLRenderManager.h +++ b/ext/native/thin3d/GLRenderManager.h @@ -177,6 +177,125 @@ private: bool hasStorage_ = false; }; +class GLRenderManager; + +// Similar to VulkanPushBuffer but is currently less efficient - it collects all the data in +// RAM then does a big memcpy/buffer upload at the end of the frame. This is at least a lot +// faster than the hundreds of buffer uploads or memory array buffers we used before. +// On modern GL we could avoid the copy using glBufferStorage but not sure it's worth the +// trouble. +class GLPushBuffer { +public: + struct BufInfo { + GLRBuffer *buffer = nullptr; + uint8_t *localMemory = nullptr; + uint8_t *deviceMemory = nullptr; + size_t flushOffset = 0; + }; + + GLPushBuffer(GLRenderManager *render, GLuint target, size_t size); + ~GLPushBuffer(); + + void Reset() { offset_ = 0; } + + // Needs context in case of defragment. + void Begin() { + buf_ = 0; + offset_ = 0; + // Note: we must defrag because some buffers may be smaller than size_. + Defragment(); + Map(); + assert(writePtr_); + } + + void BeginNoReset() { + Map(); + } + + void End() { + Unmap(); + } + + void Map(); + void Unmap(); + + // When using the returned memory, make sure to bind the returned vkbuf. + // This will later allow for handling overflow correctly. + size_t Allocate(size_t numBytes, GLRBuffer **vkbuf) { + size_t out = offset_; + if (offset_ + ((numBytes + 3) & ~3) >= size_) { + NextBuffer(numBytes); + out = offset_; + offset_ += (numBytes + 3) & ~3; + } else { + offset_ += (numBytes + 3) & ~3; // Round up to 4 bytes. + } + *vkbuf = buffers_[buf_].buffer; + return out; + } + + // Returns the offset that should be used when binding this buffer to get this data. + size_t Push(const void *data, size_t size, GLRBuffer **vkbuf) { + assert(writePtr_); + size_t off = Allocate(size, vkbuf); + memcpy(writePtr_ + off, data, size); + return off; + } + + uint32_t PushAligned(const void *data, size_t size, int align, GLRBuffer **vkbuf) { + assert(writePtr_); + offset_ = (offset_ + align - 1) & ~(align - 1); + size_t off = Allocate(size, vkbuf); + memcpy(writePtr_ + off, data, size); + return (uint32_t)off; + } + + size_t GetOffset() const { + return offset_; + } + + // "Zero-copy" variant - you can write the data directly as you compute it. + // Recommended. + void *Push(size_t size, uint32_t *bindOffset, GLRBuffer **vkbuf) { + assert(writePtr_); + size_t off = Allocate(size, vkbuf); + *bindOffset = (uint32_t)off; + return writePtr_ + off; + } + void *PushAligned(size_t size, uint32_t *bindOffset, GLRBuffer **vkbuf, int align) { + assert(writePtr_); + offset_ = (offset_ + align - 1) & ~(align - 1); + size_t off = Allocate(size, vkbuf); + *bindOffset = (uint32_t)off; + return writePtr_ + off; + } + + size_t GetTotalSize() const; + + void Flush(); + +protected: + void MapDevice(GLBufferStrategy strategy); + void UnmapDevice(); + +private: + bool AddBuffer(); + void NextBuffer(size_t minSize); + void Defragment(); + void Destroy(); + + GLRenderManager *render_; + std::vector buffers_; + size_t buf_ = 0; + size_t offset_ = 0; + size_t size_ = 0; + uint8_t *writePtr_ = nullptr; + GLuint target_; + GLBufferStrategy strategy_ = GLBufferStrategy::SUBDATA; + + friend class GLRenderManager; +}; + enum class GLRRunType { END, SYNC, @@ -327,6 +446,10 @@ public: return step.create_input_layout.inputLayout; } + GLPushBuffer *CreatePushBuffer(GLuint target, size_t size) { + return new GLPushBuffer(this, target, size); + } + void DeleteShader(GLRShader *shader) { deleter_.shaders.push_back(shader); } @@ -345,6 +468,9 @@ public: void DeleteFramebuffer(GLRFramebuffer *framebuffer) { deleter_.framebuffers.push_back(framebuffer); } + void DeletePushBuffer(GLPushBuffer *pushbuffer) { + delete pushbuffer; + } void BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRenderPassAction color, GLRRenderPassAction depth, GLRRenderPassAction stencil, uint32_t clearColor, float clearDepth, uint8_t clearStencil); void BindFramebufferAsTexture(GLRFramebuffer *fb, int binding, int aspectBit, int attachment); @@ -788,122 +914,3 @@ private: int targetWidth_ = 0; int targetHeight_ = 0; }; - -// Similar to VulkanPushBuffer but is currently less efficient - it collects all the data in -// RAM then does a big memcpy/buffer upload at the end of the frame. This is at least a lot -// faster than the hundreds of buffer uploads or memory array buffers we used before. -// On modern GL we could avoid the copy using glBufferStorage but not sure it's worth the -// trouble. -class GLPushBuffer { -public: - struct BufInfo { - GLRBuffer *buffer = nullptr; - uint8_t *localMemory = nullptr; - uint8_t *deviceMemory = nullptr; - size_t flushOffset = 0; - }; - -public: - GLPushBuffer(GLRenderManager *render, GLuint target, size_t size); - ~GLPushBuffer(); - - void Destroy(); - - void Reset() { offset_ = 0; } - - // Needs context in case of defragment. - void Begin() { - buf_ = 0; - offset_ = 0; - // Note: we must defrag because some buffers may be smaller than size_. - Defragment(); - Map(); - assert(writePtr_); - } - - void BeginNoReset() { - Map(); - } - - void End() { - Unmap(); - } - - void Map(); - void Unmap(); - - // When using the returned memory, make sure to bind the returned vkbuf. - // This will later allow for handling overflow correctly. - size_t Allocate(size_t numBytes, GLRBuffer **vkbuf) { - size_t out = offset_; - if (offset_ + ((numBytes + 3) & ~3) >= size_) { - NextBuffer(numBytes); - out = offset_; - offset_ += (numBytes + 3) & ~3; - } else { - offset_ += (numBytes + 3) & ~3; // Round up to 4 bytes. - } - *vkbuf = buffers_[buf_].buffer; - return out; - } - - // Returns the offset that should be used when binding this buffer to get this data. - size_t Push(const void *data, size_t size, GLRBuffer **vkbuf) { - assert(writePtr_); - size_t off = Allocate(size, vkbuf); - memcpy(writePtr_ + off, data, size); - return off; - } - - uint32_t PushAligned(const void *data, size_t size, int align, GLRBuffer **vkbuf) { - assert(writePtr_); - offset_ = (offset_ + align - 1) & ~(align - 1); - size_t off = Allocate(size, vkbuf); - memcpy(writePtr_ + off, data, size); - return (uint32_t)off; - } - - size_t GetOffset() const { - return offset_; - } - - // "Zero-copy" variant - you can write the data directly as you compute it. - // Recommended. - void *Push(size_t size, uint32_t *bindOffset, GLRBuffer **vkbuf) { - assert(writePtr_); - size_t off = Allocate(size, vkbuf); - *bindOffset = (uint32_t)off; - return writePtr_ + off; - } - void *PushAligned(size_t size, uint32_t *bindOffset, GLRBuffer **vkbuf, int align) { - assert(writePtr_); - offset_ = (offset_ + align - 1) & ~(align - 1); - size_t off = Allocate(size, vkbuf); - *bindOffset = (uint32_t)off; - return writePtr_ + off; - } - - size_t GetTotalSize() const; - - void Flush(); - -protected: - void MapDevice(GLBufferStrategy strategy); - void UnmapDevice(); - -private: - bool AddBuffer(); - void NextBuffer(size_t minSize); - void Defragment(); - - GLRenderManager *render_; - std::vector buffers_; - size_t buf_ = 0; - size_t offset_ = 0; - size_t size_ = 0; - uint8_t *writePtr_ = nullptr; - GLuint target_; - GLBufferStrategy strategy_ = GLBufferStrategy::SUBDATA; - - friend class GLRenderManager; -}; diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index 84e28a2dc7..70e3472af4 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -523,7 +523,7 @@ OpenGLContext::OpenGLContext() { break; } for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - frameData_[i].push = new GLPushBuffer(&renderManager_, GL_ARRAY_BUFFER, 64 * 1024); + frameData_[i].push = renderManager_.CreatePushBuffer(GL_ARRAY_BUFFER, 64 * 1024); renderManager_.RegisterPushBuffer(i, frameData_[i].push); } } @@ -532,8 +532,7 @@ OpenGLContext::~OpenGLContext() { DestroyPresets(); for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { renderManager_.UnregisterPushBuffer(i, frameData_[i].push); - frameData_[i].push->Destroy(); - delete frameData_[i].push; + renderManager_.DeletePushBuffer(frameData_[i].push); } boundSamplers_.clear(); } From feb3870f7d9520ea34df8f9047594e447c759796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 17:34:32 +0200 Subject: [PATCH 4/8] Also take control over Begin/End --- GPU/GLES/DrawEngineGLES.cpp | 8 ++++---- ext/native/thin3d/GLRenderManager.h | 8 ++++++++ ext/native/thin3d/thin3d_gl.cpp | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index b0dc1e946f..e1883d9fb6 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -157,14 +157,14 @@ void DrawEngineGLES::ClearInputLayoutMap() { void DrawEngineGLES::BeginFrame() { FrameData &frameData = frameData_[render_->GetCurFrame()]; - frameData.pushIndex->Begin(); - frameData.pushVertex->Begin(); + render_->BeginPushBuffer(frameData.pushIndex); + render_->BeginPushBuffer(frameData.pushVertex); } void DrawEngineGLES::EndFrame() { FrameData &frameData = frameData_[render_->GetCurFrame()]; - frameData.pushIndex->End(); - frameData.pushVertex->End(); + render_->EndPushBuffer(frameData.pushIndex); + render_->EndPushBuffer(frameData.pushVertex); tessDataTransfer->EndFrame(); } diff --git a/ext/native/thin3d/GLRenderManager.h b/ext/native/thin3d/GLRenderManager.h index 953af0bfbf..7bfd0fab4a 100644 --- a/ext/native/thin3d/GLRenderManager.h +++ b/ext/native/thin3d/GLRenderManager.h @@ -472,6 +472,14 @@ public: delete pushbuffer; } + void BeginPushBuffer(GLPushBuffer *pushbuffer) { + pushbuffer->Begin(); + } + + void EndPushBuffer(GLPushBuffer *pushbuffer) { + pushbuffer->End(); + } + void BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRenderPassAction color, GLRRenderPassAction depth, GLRRenderPassAction stencil, uint32_t clearColor, float clearDepth, uint8_t clearStencil); void BindFramebufferAsTexture(GLRFramebuffer *fb, int binding, int aspectBit, int attachment); bool CopyFramebufferToMemorySync(GLRFramebuffer *src, int aspectBits, int x, int y, int w, int h, Draw::DataFormat destFormat, uint8_t *pixels, int pixelStride); diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index 70e3472af4..13a2585c0e 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -540,12 +540,12 @@ OpenGLContext::~OpenGLContext() { void OpenGLContext::BeginFrame() { renderManager_.BeginFrame(); FrameData &frameData = frameData_[renderManager_.GetCurFrame()]; - frameData.push->Begin(); + renderManager_.BeginPushBuffer(frameData.push); } void OpenGLContext::EndFrame() { FrameData &frameData = frameData_[renderManager_.GetCurFrame()]; - frameData.push->End(); // upload the data! + renderManager_.EndPushBuffer(frameData.push); // upload the data! renderManager_.Finish(); } From 76138fee6da9364f4f1a9e3d377689a89c86751e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 17:47:08 +0200 Subject: [PATCH 5/8] Move pushbuffer deletion to the render manager. --- ext/native/thin3d/GLRenderManager.cpp | 10 +++++++++- ext/native/thin3d/GLRenderManager.h | 13 +++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/ext/native/thin3d/GLRenderManager.cpp b/ext/native/thin3d/GLRenderManager.cpp index f7cc21e87d..2ba1057031 100644 --- a/ext/native/thin3d/GLRenderManager.cpp +++ b/ext/native/thin3d/GLRenderManager.cpp @@ -13,7 +13,12 @@ #define VLOG(...) #endif +// Runs on the GPU thread. void GLDeleter::Perform() { + for (auto pushBuffer : pushBuffers) { + delete pushBuffer; + } + pushBuffers.clear(); for (auto shader : shaders) { delete shader; } @@ -638,10 +643,13 @@ bool GLPushBuffer::AddBuffer() { return true; } +// Executed on the render thread! void GLPushBuffer::Destroy() { for (BufInfo &info : buffers_) { // This will automatically unmap device memory, if needed. - render_->DeleteBuffer(info.buffer); + // NOTE: We immediately delete the buffer, don't go through the deleter, since we're on the render thread. + // render_->DeleteBuffer(info.buffer); + delete info.buffer; FreeAlignedMemory(info.localMemory); } buffers_.clear(); diff --git a/ext/native/thin3d/GLRenderManager.h b/ext/native/thin3d/GLRenderManager.h index 7bfd0fab4a..ad8de14584 100644 --- a/ext/native/thin3d/GLRenderManager.h +++ b/ext/native/thin3d/GLRenderManager.h @@ -184,8 +184,12 @@ class GLRenderManager; // faster than the hundreds of buffer uploads or memory array buffers we used before. // On modern GL we could avoid the copy using glBufferStorage but not sure it's worth the // trouble. +// We need to manage the lifetime of this together with the other resources so its destructor +// runs on the render thread. class GLPushBuffer { public: + friend class GLRenderManager; + struct BufInfo { GLRBuffer *buffer = nullptr; uint8_t *localMemory = nullptr; @@ -198,6 +202,7 @@ public: void Reset() { offset_ = 0; } +private: // Needs context in case of defragment. void Begin() { buf_ = 0; @@ -216,6 +221,7 @@ public: Unmap(); } +public: void Map(); void Unmap(); @@ -306,7 +312,7 @@ public: void Perform(); bool IsEmpty() const { - return shaders.empty() && programs.empty() && buffers.empty() && textures.empty() && inputLayouts.empty() && framebuffers.empty(); + return shaders.empty() && programs.empty() && buffers.empty() && textures.empty() && inputLayouts.empty() && framebuffers.empty() && pushBuffers.empty(); } void Take(GLDeleter &other) { _assert_msg_(G3D, IsEmpty(), "Deleter already has stuff"); @@ -316,12 +322,14 @@ public: textures = std::move(other.textures); inputLayouts = std::move(other.inputLayouts); framebuffers = std::move(other.framebuffers); + pushBuffers = std::move(other.pushBuffers); other.shaders.clear(); other.programs.clear(); other.buffers.clear(); other.textures.clear(); other.inputLayouts.clear(); other.framebuffers.clear(); + other.pushBuffers.clear(); } std::vector shaders; @@ -330,6 +338,7 @@ public: std::vector textures; std::vector inputLayouts; std::vector framebuffers; + std::vector pushBuffers; }; class GLRInputLayout { @@ -469,7 +478,7 @@ public: deleter_.framebuffers.push_back(framebuffer); } void DeletePushBuffer(GLPushBuffer *pushbuffer) { - delete pushbuffer; + deleter_.pushBuffers.push_back(pushbuffer); } void BeginPushBuffer(GLPushBuffer *pushbuffer) { From c7eb259c0c2b3be3433ac634e70a54874f959bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 17:50:37 +0200 Subject: [PATCH 6/8] Auto register new push buffers on creation --- ext/native/thin3d/GLRenderManager.h | 32 ++++++++++++++++++----------- ext/native/thin3d/thin3d_gl.cpp | 5 ++--- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/ext/native/thin3d/GLRenderManager.h b/ext/native/thin3d/GLRenderManager.h index ad8de14584..ec5cbd6a14 100644 --- a/ext/native/thin3d/GLRenderManager.h +++ b/ext/native/thin3d/GLRenderManager.h @@ -455,8 +455,10 @@ public: return step.create_input_layout.inputLayout; } - GLPushBuffer *CreatePushBuffer(GLuint target, size_t size) { - return new GLPushBuffer(this, target, size); + GLPushBuffer *CreatePushBuffer(int frame, GLuint target, size_t size) { + GLPushBuffer *push = new GLPushBuffer(this, target, size); + RegisterPushBuffer(frame, push); + return push; } void DeleteShader(GLRShader *shader) { @@ -822,16 +824,16 @@ public: queueRunner_.Resize(width, height); } - // 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) { - frameData_[frame].activePushBuffers.insert(buffer); - } - - void UnregisterPushBuffer(int frame, GLPushBuffer *buffer) { - auto iter = frameData_[frame].activePushBuffers.find(buffer); - _assert_(iter != frameData_[frame].activePushBuffers.end()); - frameData_[frame].activePushBuffers.erase(iter); + void UnregisterPushBuffer(GLPushBuffer *buffer) { + bool found = false; + for (int i = 0; i < MAX_INFLIGHT_FRAMES; i++) { + auto iter = frameData_[i].activePushBuffers.find(buffer); + if (iter != frameData_[i].activePushBuffers.end()) { + frameData_[i].activePushBuffers.erase(iter); + found = true; + } + } + assert(found); } void SetSwapFunction(std::function swapFunction) { @@ -869,6 +871,12 @@ private: void FlushSync(); void EndSyncFrame(int frame); + // 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) { + frameData_[frame].activePushBuffers.insert(buffer); + } + // Per-frame data, round-robin so we can overlap submission with execution of the previous frame. struct FrameData { std::mutex push_mutex; diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index 13a2585c0e..8e10e04204 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -523,15 +523,14 @@ OpenGLContext::OpenGLContext() { break; } for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - frameData_[i].push = renderManager_.CreatePushBuffer(GL_ARRAY_BUFFER, 64 * 1024); - renderManager_.RegisterPushBuffer(i, frameData_[i].push); + frameData_[i].push = renderManager_.CreatePushBuffer(i, GL_ARRAY_BUFFER, 64 * 1024); } } OpenGLContext::~OpenGLContext() { DestroyPresets(); for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - renderManager_.UnregisterPushBuffer(i, frameData_[i].push); + renderManager_.UnregisterPushBuffer(frameData_[i].push); renderManager_.DeletePushBuffer(frameData_[i].push); } boundSamplers_.clear(); From b06df3536e2acf6195889613f8446220efdd6b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 17:53:03 +0200 Subject: [PATCH 7/8] Auto unregister push buffer on deletion. --- GPU/GLES/DrawEngineGLES.cpp | 8 ++------ ext/native/thin3d/GLRenderManager.cpp | 13 +++++++------ ext/native/thin3d/GLRenderManager.h | 2 +- ext/native/thin3d/thin3d_gl.cpp | 1 - 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index e1883d9fb6..b8499b50c8 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -109,11 +109,9 @@ void DrawEngineGLES::DeviceRestore() { void DrawEngineGLES::InitDeviceObjects() { for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - frameData_[i].pushVertex = render_->CreatePushBuffer(GL_ARRAY_BUFFER, 1024 * 1024); - frameData_[i].pushIndex = render_->CreatePushBuffer(GL_ELEMENT_ARRAY_BUFFER, 256 * 1024); + frameData_[i].pushVertex = render_->CreatePushBuffer(i, GL_ARRAY_BUFFER, 1024 * 1024); + frameData_[i].pushIndex = render_->CreatePushBuffer(i, GL_ELEMENT_ARRAY_BUFFER, 256 * 1024); - render_->RegisterPushBuffer(i, frameData_[i].pushVertex); - render_->RegisterPushBuffer(i, frameData_[i].pushIndex); } int vertexSize = sizeof(TransformedVertex); @@ -131,8 +129,6 @@ void DrawEngineGLES::DestroyDeviceObjects() { if (!frameData_[i].pushVertex && !frameData_[i].pushIndex) continue; - render_->UnregisterPushBuffer(i, frameData_[i].pushVertex); - render_->UnregisterPushBuffer(i, frameData_[i].pushIndex); render_->DeletePushBuffer(frameData_[i].pushVertex); render_->DeletePushBuffer(frameData_[i].pushIndex); frameData_[i].pushVertex = nullptr; diff --git a/ext/native/thin3d/GLRenderManager.cpp b/ext/native/thin3d/GLRenderManager.cpp index 2ba1057031..da06102ccd 100644 --- a/ext/native/thin3d/GLRenderManager.cpp +++ b/ext/native/thin3d/GLRenderManager.cpp @@ -14,8 +14,9 @@ #endif // Runs on the GPU thread. -void GLDeleter::Perform() { +void GLDeleter::Perform(GLRenderManager *renderManager) { for (auto pushBuffer : pushBuffers) { + renderManager->UnregisterPushBuffer(pushBuffer); delete pushBuffer; } pushBuffers.clear(); @@ -57,7 +58,7 @@ GLRenderManager::~GLRenderManager() { _assert_(frameData_[i].deleter_prev.IsEmpty()); } // Was anything deleted during shutdown? - deleter_.Perform(); + deleter_.Perform(this); _assert_(deleter_.IsEmpty()); } @@ -103,15 +104,15 @@ void GLRenderManager::ThreadEnd() { // Good point to run all the deleters to get rid of leftover objects. for (int i = 0; i < MAX_INFLIGHT_FRAMES; i++) { - frameData_[i].deleter.Perform(); - frameData_[i].deleter_prev.Perform(); + frameData_[i].deleter.Perform(this); + frameData_[i].deleter_prev.Perform(this); for (int j = 0; j < (int)frameData_[i].steps.size(); j++) { delete frameData_[i].steps[j]; } frameData_[i].steps.clear(); frameData_[i].initSteps.clear(); } - deleter_.Perform(); + deleter_.Perform(this); for (int i = 0; i < (int)steps_.size(); i++) { delete steps_[i]; @@ -145,7 +146,7 @@ bool GLRenderManager::ThreadFrame() { } VLOG("PULL: Setting frame[%d].readyForRun = false", threadFrame_); frameData.readyForRun = false; - frameData.deleter_prev.Perform(); + frameData.deleter_prev.Perform(this); frameData.deleter_prev.Take(frameData.deleter); // Previously we had a quick exit here that avoided calling Run() if run_ was suddenly false, // but that created a race condition where frames could end up not finished properly on resize etc. diff --git a/ext/native/thin3d/GLRenderManager.h b/ext/native/thin3d/GLRenderManager.h index ec5cbd6a14..164cdc4dc4 100644 --- a/ext/native/thin3d/GLRenderManager.h +++ b/ext/native/thin3d/GLRenderManager.h @@ -309,7 +309,7 @@ enum class GLRRunType { class GLDeleter { public: - void Perform(); + void Perform(GLRenderManager *renderManager); bool IsEmpty() const { return shaders.empty() && programs.empty() && buffers.empty() && textures.empty() && inputLayouts.empty() && framebuffers.empty() && pushBuffers.empty(); diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index 8e10e04204..18891fded6 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -530,7 +530,6 @@ OpenGLContext::OpenGLContext() { OpenGLContext::~OpenGLContext() { DestroyPresets(); for (int i = 0; i < GLRenderManager::MAX_INFLIGHT_FRAMES; i++) { - renderManager_.UnregisterPushBuffer(frameData_[i].push); renderManager_.DeletePushBuffer(frameData_[i].push); } boundSamplers_.clear(); From 398d3755682c91514e90f9ef2bb10f2e1861e7a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Apr 2018 20:17:19 +0200 Subject: [PATCH 8/8] Fix issue in TextureCacheGLES::DeviceRestore --- GPU/GLES/TextureCacheGLES.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/GPU/GLES/TextureCacheGLES.cpp b/GPU/GLES/TextureCacheGLES.cpp index ed802319a8..ae57b3b472 100644 --- a/GPU/GLES/TextureCacheGLES.cpp +++ b/GPU/GLES/TextureCacheGLES.cpp @@ -854,6 +854,7 @@ void TextureCacheGLES::DeviceLost() { void TextureCacheGLES::DeviceRestore(Draw::DrawContext *draw) { draw_ = draw; + render_ = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); if (!shadeInputLayout_) { std::vector entries; entries.push_back({ 0, 3, GL_FLOAT, GL_FALSE, 20, 0 });