From b50c4e9ffb95e20a299f0d845171ea4effc7805e Mon Sep 17 00:00:00 2001 From: Laura Hermanns Date: Thu, 4 Jul 2024 15:11:47 -0400 Subject: [PATCH] [GL] Fixed resolving of multi-sampled render-target on EndRenderPass(). Multi-sampled render-targets must be resolved on EndRenderPass(), not just on the next BeginRenderPass() call. Otherwise, rendering to a texture once (with a multi-sampled render target) and then reading its content via ReadTexture() will fail. --- sources/Renderer/OpenGL/Command/GLCommand.h | 5 +++++ .../OpenGL/Command/GLCommandAssembler.cpp | 6 +++++ .../OpenGL/Command/GLCommandExecutor.cpp | 6 +++++ .../Renderer/OpenGL/Command/GLCommandOpcode.h | 1 + .../Command/GLDeferredCommandBuffer.cpp | 22 ++++++++++++++++++- .../OpenGL/Command/GLDeferredCommandBuffer.h | 1 + .../Command/GLImmediateCommandBuffer.cpp | 4 +++- .../Renderer/OpenGL/GLStaticAssertions.cpp | 1 + .../OpenGL/RenderState/GLStateManager.cpp | 9 -------- .../OpenGL/RenderState/GLStateManager.h | 3 --- .../OpenGL/Texture/GLRenderTarget.cpp | 7 +++++- .../Renderer/OpenGL/Texture/GLRenderTarget.h | 3 +++ 12 files changed, 53 insertions(+), 15 deletions(-) diff --git a/sources/Renderer/OpenGL/Command/GLCommand.h b/sources/Renderer/OpenGL/Command/GLCommand.h index 2c6a26ea14..610597f741 100644 --- a/sources/Renderer/OpenGL/Command/GLCommand.h +++ b/sources/Renderer/OpenGL/Command/GLCommand.h @@ -179,6 +179,11 @@ struct GLCmdClearBuffers // AttachmentClear attachments[numAttachments]; }; +struct GLCmdResolveRenderTarget +{ + GLRenderTarget* renderTarget; +}; + struct GLCmdBindVertexArray { GLSharedContextVertexArray* vertexArray; diff --git a/sources/Renderer/OpenGL/Command/GLCommandAssembler.cpp b/sources/Renderer/OpenGL/Command/GLCommandAssembler.cpp index 7a59ff56d8..3c669eed21 100644 --- a/sources/Renderer/OpenGL/Command/GLCommandAssembler.cpp +++ b/sources/Renderer/OpenGL/Command/GLCommandAssembler.cpp @@ -193,6 +193,12 @@ static std::size_t AssembleGLCommand(const GLOpcode opcode, const void* pc, JITC compiler.CallMember(&GLStateManager::ClearBuffers, g_stateMngrArg, cmd->numAttachments, (cmd + 1)); return sizeof(*cmd); } + case GLOpcodeResolveRenderTarget: + { + auto cmd = reinterpret_cast(pc); + compiler.CallMember(&GLRenderTarget::ResolveMultisampled, pc->renderTarget, g_stateMngrArg); + return sizeof(*cmd); + } case GLOpcodeBindVertexArray: { auto cmd = reinterpret_cast(pc); diff --git a/sources/Renderer/OpenGL/Command/GLCommandExecutor.cpp b/sources/Renderer/OpenGL/Command/GLCommandExecutor.cpp index 7c9a0641aa..16532e34c0 100644 --- a/sources/Renderer/OpenGL/Command/GLCommandExecutor.cpp +++ b/sources/Renderer/OpenGL/Command/GLCommandExecutor.cpp @@ -192,6 +192,12 @@ static std::size_t ExecuteGLCommand(const GLOpcode opcode, const void* pc, GLSta stateMngr->ClearBuffers(cmd->numAttachments, reinterpret_cast(cmd + 1)); return (sizeof(*cmd) + sizeof(AttachmentClear)*cmd->numAttachments); } + case GLOpcodeResolveRenderTarget: + { + auto cmd = reinterpret_cast(pc); + cmd->renderTarget->ResolveMultisampled(*stateMngr); + return sizeof(*cmd); + } case GLOpcodeBindVertexArray: { auto cmd = reinterpret_cast(pc); diff --git a/sources/Renderer/OpenGL/Command/GLCommandOpcode.h b/sources/Renderer/OpenGL/Command/GLCommandOpcode.h index 65fde3e626..ffcff87bc6 100644 --- a/sources/Renderer/OpenGL/Command/GLCommandOpcode.h +++ b/sources/Renderer/OpenGL/Command/GLCommandOpcode.h @@ -39,6 +39,7 @@ enum GLOpcode : std::uint8_t GLOpcodeClear, GLOpcodeClearAttachmentsWithRenderPass, GLOpcodeClearBuffers, + GLOpcodeResolveRenderTarget, GLOpcodeBindVertexArray, GLOpcodeBindElementArrayBufferToVAO, GLOpcodeBindBufferBase, diff --git a/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.cpp b/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.cpp index 4dc2812d83..d25a3e5a7d 100644 --- a/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.cpp +++ b/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.cpp @@ -8,6 +8,7 @@ #include "GLDeferredCommandBuffer.h" #include "GLCommand.h" #include +#include #include "../../TextureUtils.h" #include "../GLSwapChain.h" @@ -502,11 +503,30 @@ void GLDeferredCommandBuffer::BeginRenderPass( ::memcpy(cmd + 1, clearValues, sizeof(ClearValue)*numClearValues); } } + + /* + Cache render target if it needs to be resolved at the following EndRenderPass() call. + This is one of the few states the deferred GL command buffer caches + as it must be guaranteed that this render pass is followed by a call to EndRenderPass() operating on the same render-target. + */ + if (!LLGL::IsInstanceOf(renderTarget)) + { + auto& renderTargetGL = LLGL_CAST(GLRenderTarget&, renderTarget); + if (renderTargetGL.CanResolveMultisampledFBO()) + renderTargetToResolve_ = &renderTargetGL; + } } void GLDeferredCommandBuffer::EndRenderPass() { - // dummy + if (renderTargetToResolve_ != nullptr) + { + auto cmd = AllocCommand(GLOpcodeResolveRenderTarget); + { + cmd->renderTarget = renderTargetToResolve_; + } + renderTargetToResolve_ = nullptr; + } } void GLDeferredCommandBuffer::Clear(long flags, const ClearValue& clearValue) diff --git a/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.h b/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.h index 464503434d..a9ef02e300 100644 --- a/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.h +++ b/sources/Renderer/OpenGL/Command/GLDeferredCommandBuffer.h @@ -116,6 +116,7 @@ class GLDeferredCommandBuffer final : public GLCommandBuffer long flags_ = 0; GLVirtualCommandBuffer buffer_; + GLRenderTarget* renderTargetToResolve_ = nullptr; #ifdef LLGL_ENABLE_JIT_COMPILER std::unique_ptr executable_; diff --git a/sources/Renderer/OpenGL/Command/GLImmediateCommandBuffer.cpp b/sources/Renderer/OpenGL/Command/GLImmediateCommandBuffer.cpp index 07e849def8..b63115f3e8 100644 --- a/sources/Renderer/OpenGL/Command/GLImmediateCommandBuffer.cpp +++ b/sources/Renderer/OpenGL/Command/GLImmediateCommandBuffer.cpp @@ -431,7 +431,9 @@ void GLImmediateCommandBuffer::BeginRenderPass( void GLImmediateCommandBuffer::EndRenderPass() { - // dummy + /* Resolve previously bound render target */ + if (GLRenderTarget* renderTarget = stateMngr_->GetBoundRenderTarget()) + renderTarget->ResolveMultisampled(*stateMngr_); } void GLImmediateCommandBuffer::Clear(long flags, const ClearValue& clearValue) diff --git a/sources/Renderer/OpenGL/GLStaticAssertions.cpp b/sources/Renderer/OpenGL/GLStaticAssertions.cpp index aa988162f4..f0db96bda8 100644 --- a/sources/Renderer/OpenGL/GLStaticAssertions.cpp +++ b/sources/Renderer/OpenGL/GLStaticAssertions.cpp @@ -47,6 +47,7 @@ LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdClearStencil ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdClear ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdClearAttachmentsWithRenderPass ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdClearBuffers ); +LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdResolveRenderTarget ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdBindVertexArray ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdBindElementArrayBufferToVAO ); LLGL_ASSERT_STDLAYOUT_STRUCT( GLCmdBindBufferBase ); diff --git a/sources/Renderer/OpenGL/RenderState/GLStateManager.cpp b/sources/Renderer/OpenGL/RenderState/GLStateManager.cpp index 23d24f9e5c..2e92b22ca3 100644 --- a/sources/Renderer/OpenGL/RenderState/GLStateManager.cpp +++ b/sources/Renderer/OpenGL/RenderState/GLStateManager.cpp @@ -1524,9 +1524,6 @@ GLuint GLStateManager::GetBoundProgramPipeline() const void GLStateManager::BindRenderTarget(RenderTarget& renderTarget, GLStateManager** nextStateManager) { - /* Resolve previously bound render target */ - ResolveMultisampledRenderTarget(); - /* Bind render target/context */ if (LLGL::IsInstanceOf(renderTarget)) { @@ -1824,12 +1821,6 @@ void GLStateManager::RestoreWriteMasks(GLIntermediateBufferWriteMasks& intermedi /* ----- Render pass ----- */ -void GLStateManager::ResolveMultisampledRenderTarget() -{ - if (auto* renderTarget = GetBoundRenderTarget()) - renderTarget->ResolveMultisampled(*this); -} - void GLStateManager::ClearAttachmentsWithRenderPass( const GLRenderPass& renderPassGL, std::uint32_t numClearValues, diff --git a/sources/Renderer/OpenGL/RenderState/GLStateManager.h b/sources/Renderer/OpenGL/RenderState/GLStateManager.h index 5b29e0b187..ae4cc53e3d 100644 --- a/sources/Renderer/OpenGL/RenderState/GLStateManager.h +++ b/sources/Renderer/OpenGL/RenderState/GLStateManager.h @@ -344,9 +344,6 @@ class GLStateManager /* ----- Render pass ----- */ - // Blits the currently bound render target - void ResolveMultisampledRenderTarget(); - std::uint32_t ClearColorBuffers( const std::uint8_t* colorBuffers, std::uint32_t numClearValues, diff --git a/sources/Renderer/OpenGL/Texture/GLRenderTarget.cpp b/sources/Renderer/OpenGL/Texture/GLRenderTarget.cpp index f244af17c8..cf4ff1fedf 100644 --- a/sources/Renderer/OpenGL/Texture/GLRenderTarget.cpp +++ b/sources/Renderer/OpenGL/Texture/GLRenderTarget.cpp @@ -85,9 +85,14 @@ const RenderPass* GLRenderTarget::GetRenderPass() const return renderPass_; } +bool GLRenderTarget::CanResolveMultisampledFBO() const +{ + return (framebufferResolve_.Valid() && !drawBuffersResolve_.empty()); +} + void GLRenderTarget::ResolveMultisampled(GLStateManager& stateMngr) { - if (framebufferResolve_.Valid() && !drawBuffersResolve_.empty()) + if (CanResolveMultisampledFBO()) { stateMngr.BindFramebuffer(GLFramebufferTarget::DrawFramebuffer, framebufferResolve_.GetID()); stateMngr.BindFramebuffer(GLFramebufferTarget::ReadFramebuffer, framebuffer_.GetID()); diff --git a/sources/Renderer/OpenGL/Texture/GLRenderTarget.h b/sources/Renderer/OpenGL/Texture/GLRenderTarget.h index fea4cc9b48..64bd604e39 100644 --- a/sources/Renderer/OpenGL/Texture/GLRenderTarget.h +++ b/sources/Renderer/OpenGL/Texture/GLRenderTarget.h @@ -41,6 +41,9 @@ class GLRenderTarget final : public RenderTarget GLRenderTarget(const RenderingLimits& limits, const RenderTargetDescriptor& desc); ~GLRenderTarget(); + // Returns true if this render-target can resolve its multi-sampled FBO into a single sampled FBO. + bool CanResolveMultisampledFBO() const; + // Blits the multi-sample framebuffer onto the default framebuffer. void ResolveMultisampled(GLStateManager& stateMngr);