From cdc56cb7d6fdd15cfd711ef069f6dad4ab042d78 Mon Sep 17 00:00:00 2001 From: assiduous Date: Thu, 14 Sep 2023 15:04:43 -0700 Subject: [PATCH] Enabled render passes without attachments (close #437) --- .../include/DeviceContextBase.hpp | 2 +- .../GraphicsEngineOpenGL/include/FBOCache.hpp | 9 +- .../include/GLContextState.hpp | 1 + .../src/DeviceContextGLImpl.cpp | 29 ++++ .../GraphicsEngineOpenGL/src/FBOCache.cpp | 134 ++++++++++++------ .../src/FramebufferGLImpl.cpp | 4 +- .../src/ComputeShaderTest.cpp | 62 +++++++- 7 files changed, 190 insertions(+), 51 deletions(-) diff --git a/Graphics/GraphicsEngine/include/DeviceContextBase.hpp b/Graphics/GraphicsEngine/include/DeviceContextBase.hpp index 9a1e3c47a..02caea7f8 100644 --- a/Graphics/GraphicsEngine/include/DeviceContextBase.hpp +++ b/Graphics/GraphicsEngine/include/DeviceContextBase.hpp @@ -1204,7 +1204,7 @@ inline bool DeviceContextBase::SetSubpassRenderTargets() m_FramebufferWidth = FBDesc.Width; m_FramebufferHeight = FBDesc.Height; m_FramebufferSlices = FBDesc.NumArraySlices; - VERIFY_EXPR(m_FramebufferSamples > 0); + VERIFY_EXPR((m_FramebufferSamples > 0) || (Subpass.RenderTargetAttachmentCount == 0 && Subpass.pDepthStencilAttachment == nullptr)); return BindRenderTargets; } diff --git a/Graphics/GraphicsEngineOpenGL/include/FBOCache.hpp b/Graphics/GraphicsEngineOpenGL/include/FBOCache.hpp index 4a4ba28ba..f044c89ec 100644 --- a/Graphics/GraphicsEngineOpenGL/include/FBOCache.hpp +++ b/Graphics/GraphicsEngineOpenGL/include/FBOCache.hpp @@ -55,13 +55,17 @@ class FBOCache static GLObjectWrappers::GLFrameBufferObj CreateFBO(GLContextState& ContextState, Uint32 NumRenderTargets, TextureViewGLImpl* ppRTVs[], - TextureViewGLImpl* pDSV); + TextureViewGLImpl* pDSV, + Uint32 DefaultWidth = 0, + Uint32 DefaultHeight = 0); const GLObjectWrappers::GLFrameBufferObj& GetFBO(Uint32 NumRenderTargets, TextureViewGLImpl* ppRTVs[], TextureViewGLImpl* pDSV, GLContextState& ContextState); + const GLObjectWrappers::GLFrameBufferObj& GetFBO(Uint32 Width, Uint32 Height, GLContextState& ContextState); + void OnReleaseTexture(ITexture* pTexture); private: @@ -80,6 +84,9 @@ class FBOCache UniqueIdentifier DSId = 0; TextureViewDesc DSVDesc = {}; + Uint32 Width = 0; + Uint32 Height = 0; + mutable size_t Hash = 0; bool operator==(const FBOCacheKey& Key) const noexcept; diff --git a/Graphics/GraphicsEngineOpenGL/include/GLContextState.hpp b/Graphics/GraphicsEngineOpenGL/include/GLContextState.hpp index cf5c89a12..5c4e878af 100644 --- a/Graphics/GraphicsEngineOpenGL/include/GLContextState.hpp +++ b/Graphics/GraphicsEngineOpenGL/include/GLContextState.hpp @@ -104,6 +104,7 @@ class GLContextState m_FBOId = -1; } bool IsValidVAOBound() const { return m_VAOId > 0; } + bool IsValidFBOBound() const { return m_FBOId >= 0; } void SetCurrentGLContext(GLContext::NativeGLContextType Context) { m_CurrentGLContext = Context; } diff --git a/Graphics/GraphicsEngineOpenGL/src/DeviceContextGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/DeviceContextGLImpl.cpp index 5617b66dc..b7dc08f57 100644 --- a/Graphics/GraphicsEngineOpenGL/src/DeviceContextGLImpl.cpp +++ b/Graphics/GraphicsEngineOpenGL/src/DeviceContextGLImpl.cpp @@ -294,6 +294,25 @@ void DeviceContextGLImpl::SetViewports(Uint32 NumViewports, const Viewport* pVie DEV_CHECK_GL_ERROR("Failed to set depth range for viewport #", i); } } + + if (m_NumBoundRenderTargets == 0 && !m_pBoundDepthStencil) + { + // Rendering without render targets + + DEV_CHECK_ERR(m_NumViewports == 1, "Only a single viewport is supported when rendering without render targets"); + + const auto VPWidth = static_cast(m_Viewports[0].Width); + const auto VPHeight = static_cast(m_Viewports[0].Height); + if (m_FramebufferWidth != VPWidth || m_FramebufferHeight != VPHeight) + { + // We need to bind another framebuffer since the size has changed + m_ContextState.InvalidateFBO(); + } + m_FramebufferWidth = VPWidth; + m_FramebufferHeight = VPHeight; + m_FramebufferSlices = 1; + m_FramebufferSamples = 1; + } } void DeviceContextGLImpl::SetScissorRects(Uint32 NumRects, const Rect* pRects, Uint32 RTWidth, Uint32 RTHeight) @@ -719,6 +738,16 @@ void DeviceContextGLImpl::BindProgramResources(Uint32 BindSRBMask) void DeviceContextGLImpl::PrepareForDraw(DRAW_FLAGS Flags, bool IsIndexed, GLenum& GlTopology) { + if (!m_ContextState.IsValidFBOBound() && m_NumBoundRenderTargets == 0 && !m_pBoundDepthStencil) + { + // Framebuffer without attachments + DEV_CHECK_ERR(m_FramebufferWidth > 0 && m_FramebufferHeight > 0, + "Framebuffer width and height must be positive when rendering without attachments. Call SetViewports() to set the framebuffer size."); + auto& FBOCache = m_pDevice->GetFBOCache(m_ContextState.GetCurrentGLContext()); + const auto& FBO = FBOCache.GetFBO(m_FramebufferWidth, m_FramebufferHeight, m_ContextState); + m_ContextState.BindFBO(FBO); + } + #ifdef DILIGENT_DEVELOPMENT if ((Flags & DRAW_FLAG_VERIFY_RENDER_TARGETS) != 0) DvpVerifyRenderTargets(); diff --git a/Graphics/GraphicsEngineOpenGL/src/FBOCache.cpp b/Graphics/GraphicsEngineOpenGL/src/FBOCache.cpp index a12c4c80c..bdc51ca02 100644 --- a/Graphics/GraphicsEngineOpenGL/src/FBOCache.cpp +++ b/Graphics/GraphicsEngineOpenGL/src/FBOCache.cpp @@ -40,7 +40,7 @@ bool FBOCache::FBOCacheKey::operator==(const FBOCacheKey& Key) const noexcept if (Hash != 0 && Key.Hash != 0 && Hash != Key.Hash) return false; - if (NumRenderTargets != Key.NumRenderTargets) + if (NumRenderTargets != Key.NumRenderTargets || Width != Key.Width || Height != Key.Height) return false; for (Uint32 rt = 0; rt < NumRenderTargets; ++rt) { @@ -68,7 +68,7 @@ std::size_t FBOCache::FBOCacheKeyHashFunc::operator()(const FBOCacheKey& Key) co { std::hash TexViewDescHasher; Key.Hash = 0; - HashCombine(Key.Hash, Key.NumRenderTargets); + HashCombine(Key.Hash, Key.NumRenderTargets, Key.Width, Key.Height); for (Uint32 rt = 0; rt < Key.NumRenderTargets; ++rt) { HashCombine(Key.Hash, Key.RTIds[rt]); @@ -91,7 +91,13 @@ FBOCache::FBOCache() FBOCache::~FBOCache() { - VERIFY(m_Cache.empty(), "FBO cache is not empty. Are there any unreleased objects?"); +#ifdef DILIGENT_DEBUG + for (const auto& fbo_it : m_Cache) + { + const auto& Key = fbo_it.first; + VERIFY(Key.NumRenderTargets == 0 && Key.DSId == 0, "Only framebuffers without attachments can be left in the cache"); + } +#endif VERIFY(m_TexIdToKey.empty(), "TexIdToKey cache is not empty."); } @@ -112,7 +118,9 @@ void FBOCache::OnReleaseTexture(ITexture* pTexture) GLObjectWrappers::GLFrameBufferObj FBOCache::CreateFBO(GLContextState& ContextState, Uint32 NumRenderTargets, TextureViewGLImpl* ppRTVs[], - TextureViewGLImpl* pDSV) + TextureViewGLImpl* pDSV, + Uint32 DefaultWidth, + Uint32 DefaultHeight) { GLObjectWrappers::GLFrameBufferObj FBO{true}; @@ -167,35 +175,56 @@ GLObjectWrappers::GLFrameBufferObj FBOCache::CreateFBO(GLContextState& Contex pDepthTexGL->AttachToFramebuffer(DSVDesc, AttachmentPoint); } - // We now need to set mapping between shader outputs and - // color attachments. This largely redundant step is performed - // by glDrawBuffers() - // clang-format off - static constexpr GLenum DrawBuffers[] = + if (NumRenderTargets > 0) + { + // We now need to set mapping between shader outputs and + // color attachments. This largely redundant step is performed + // by glDrawBuffers() + static constexpr GLenum DrawBuffers[] = + { + GL_COLOR_ATTACHMENT0, + GL_COLOR_ATTACHMENT1, + GL_COLOR_ATTACHMENT2, + GL_COLOR_ATTACHMENT3, + GL_COLOR_ATTACHMENT4, + GL_COLOR_ATTACHMENT5, + GL_COLOR_ATTACHMENT6, + GL_COLOR_ATTACHMENT7, + GL_COLOR_ATTACHMENT8, + GL_COLOR_ATTACHMENT9, + GL_COLOR_ATTACHMENT10, + GL_COLOR_ATTACHMENT11, + GL_COLOR_ATTACHMENT12, + GL_COLOR_ATTACHMENT13, + GL_COLOR_ATTACHMENT14, + GL_COLOR_ATTACHMENT15, + }; + + // The state set by glDrawBuffers() is part of the state of the framebuffer. + // So it can be set up once and left it set. + glDrawBuffers(NumRenderTargets, DrawBuffers); + CHECK_GL_ERROR("Failed to set draw buffers via glDrawBuffers()"); + } + else if (pDSV == nullptr) { - GL_COLOR_ATTACHMENT0, - GL_COLOR_ATTACHMENT1, - GL_COLOR_ATTACHMENT2, - GL_COLOR_ATTACHMENT3, - GL_COLOR_ATTACHMENT4, - GL_COLOR_ATTACHMENT5, - GL_COLOR_ATTACHMENT6, - GL_COLOR_ATTACHMENT7, - GL_COLOR_ATTACHMENT8, - GL_COLOR_ATTACHMENT9, - GL_COLOR_ATTACHMENT10, - GL_COLOR_ATTACHMENT11, - GL_COLOR_ATTACHMENT12, - GL_COLOR_ATTACHMENT13, - GL_COLOR_ATTACHMENT14, - GL_COLOR_ATTACHMENT15 - }; - // clang-format on - - // The state set by glDrawBuffers() is part of the state of the framebuffer. - // So it can be set up once and left it set. - glDrawBuffers(NumRenderTargets, DrawBuffers); - CHECK_GL_ERROR("Failed to set draw buffers via glDrawBuffers()"); + // Framebuffer without attachments + DEV_CHECK_ERR(DefaultWidth > 0 && DefaultHeight > 0, "Framebuffer without attachment requires non-zero default width and height"); +#ifdef GL_ARB_framebuffer_no_attachments + glFramebufferParameteri(GL_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_WIDTH, DefaultWidth); + CHECK_GL_ERROR("Failed to set framebuffer default width"); + + glFramebufferParameteri(GL_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_HEIGHT, DefaultHeight); + CHECK_GL_ERROR("Failed to set framebuffer default height"); + + glFramebufferParameteri(GL_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_LAYERS, 1); + CHECK_GL_ERROR("Failed to set framebuffer default layer count"); + + glFramebufferParameteri(GL_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_SAMPLES, 1); + CHECK_GL_ERROR("Failed to set framebuffer default sample count"); +#else + DEV_ERROR("Framebuffers without attachments are not supported on this platform"); +#endif + } GLenum Status = glCheckFramebufferStatus(GL_FRAMEBUFFER); if (Status != GL_FRAMEBUFFER_COMPLETE) @@ -230,9 +259,6 @@ const GLObjectWrappers::GLFrameBufferObj& FBOCache::GetFBO(Uint32 Nu VERIFY(NumRenderTargets != 0 || pDSV != nullptr, "At least one render target or a depth-stencil buffer must be provided"); - // Lock the cache - Threading::SpinLockGuard CacheGuard{m_CacheLock}; - // Construct the key FBOCacheKey Key; VERIFY(NumRenderTargets < MAX_RENDER_TARGETS, "Too many render targets are being set"); @@ -264,13 +290,12 @@ const GLObjectWrappers::GLFrameBufferObj& FBOCache::GetFBO(Uint32 Nu Key.DSVDesc = pDSV->GetDesc(); } + // Lock the cache + Threading::SpinLockGuard CacheGuard{m_CacheLock}; + // Try to find FBO in the map - auto It = m_Cache.find(Key); - if (It != m_Cache.end()) - { - return It->second; - } - else + auto fbo_it = m_Cache.find(Key); + if (fbo_it == m_Cache.end()) { // Create a new FBO auto NewFBO = CreateFBO(ContextState, NumRenderTargets, ppRTVs, pDSV); @@ -286,8 +311,33 @@ const GLObjectWrappers::GLFrameBufferObj& FBOCache::GetFBO(Uint32 Nu m_TexIdToKey.insert(std::make_pair(Key.RTIds[rt], Key)); } - return NewElems.first->second; + fbo_it = NewElems.first; + } + return fbo_it->second; +} + +const GLObjectWrappers::GLFrameBufferObj& FBOCache::GetFBO(Uint32 Width, Uint32 Height, GLContextState& ContextState) +{ + FBOCacheKey Key; + Key.Width = Width; + Key.Height = Height; + + // Lock the cache + Threading::SpinLockGuard CacheGuard{m_CacheLock}; + + // Try to find FBO in the map + auto fbo_it = m_Cache.find(Key); + if (fbo_it == m_Cache.end()) + { + // Create a new FBO + auto NewFBO = CreateFBO(ContextState, 0, nullptr, nullptr, Width, Height); + + auto NewElems = m_Cache.emplace(std::make_pair(Key, std::move(NewFBO))); + // New element must be actually inserted + VERIFY(NewElems.second, "New element was not inserted"); + fbo_it = NewElems.first; } + return fbo_it->second; } } // namespace Diligent diff --git a/Graphics/GraphicsEngineOpenGL/src/FramebufferGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/FramebufferGLImpl.cpp index 96af518b6..e81e626a1 100644 --- a/Graphics/GraphicsEngineOpenGL/src/FramebufferGLImpl.cpp +++ b/Graphics/GraphicsEngineOpenGL/src/FramebufferGLImpl.cpp @@ -110,7 +110,7 @@ FramebufferGLImpl::FramebufferGLImpl(IReferenceCounters* pRefCounters, } auto RenderTargetFBO = UseDefaultFBO(SubpassDesc.RenderTargetAttachmentCount, ppRTVs, pDSV) ? GLObjectWrappers::GLFrameBufferObj{false} : - FBOCache::CreateFBO(CtxState, SubpassDesc.RenderTargetAttachmentCount, ppRTVs, pDSV); + FBOCache::CreateFBO(CtxState, SubpassDesc.RenderTargetAttachmentCount, ppRTVs, pDSV, Desc.Width, Desc.Height); GLObjectWrappers::GLFrameBufferObj ResolveFBO{false}; if (SubpassDesc.pResolveAttachments != nullptr) @@ -126,7 +126,7 @@ FramebufferGLImpl::FramebufferGLImpl(IReferenceCounters* pRefCounters, } ResolveFBO = UseDefaultFBO(SubpassDesc.RenderTargetAttachmentCount, ppRsvlViews, nullptr) ? GLObjectWrappers::GLFrameBufferObj{false} : - FBOCache::CreateFBO(CtxState, SubpassDesc.RenderTargetAttachmentCount, ppRsvlViews, nullptr); + FBOCache::CreateFBO(CtxState, SubpassDesc.RenderTargetAttachmentCount, ppRsvlViews, nullptr, Desc.Width, Desc.Height); } RenderTargetFBO.SetName(m_Desc.Name); diff --git a/Tests/DiligentCoreAPITest/src/ComputeShaderTest.cpp b/Tests/DiligentCoreAPITest/src/ComputeShaderTest.cpp index 7261fa2a1..a4ecfc1e2 100644 --- a/Tests/DiligentCoreAPITest/src/ComputeShaderTest.cpp +++ b/Tests/DiligentCoreAPITest/src/ComputeShaderTest.cpp @@ -282,8 +282,7 @@ TEST(ComputeShaderTest, GenerateMips_CSInterference) pSwapChain->Present(); } - -TEST(ComputeShaderTest, FillTexturePS) +static void TestFillTexturePS(bool UseRenderPass) { auto* pEnv = GPUTestingEnvironment::GetInstance(); auto* pDevice = pEnv->GetDevice(); @@ -295,6 +294,8 @@ TEST(ComputeShaderTest, FillTexturePS) auto* pSwapChain = pEnv->GetSwapChain(); auto* pContext = pEnv->GetDeviceContext(); + const auto& SCDesc = pSwapChain->GetDesc(); + GPUTestingEnvironment::ScopedReset EnvironmentAutoReset; RefCntAutoPtr pTestingSwapChain{pSwapChain, IID_TestingSwapChain}; @@ -335,6 +336,30 @@ TEST(ComputeShaderTest, FillTexturePS) PSOCreateInfo.pVS = pVS; PSOCreateInfo.pPS = pPS; + RefCntAutoPtr pRenderPass; + RefCntAutoPtr pFramebuffer; + if (UseRenderPass) + { + SubpassDesc Subpass; + RenderPassDesc RPDesc; + RPDesc.Name = "Compute shader test - render pass"; + RPDesc.pSubpasses = &Subpass; + RPDesc.SubpassCount = 1; + pDevice->CreateRenderPass(RPDesc, &pRenderPass); + ASSERT_TRUE(pRenderPass != nullptr); + + PSOCreateInfo.GraphicsPipeline.pRenderPass = pRenderPass; + + FramebufferDesc FBDesc; + FBDesc.Name = "Compute shader test - framebuffer"; + FBDesc.pRenderPass = pRenderPass; + FBDesc.Width = SCDesc.Width; + FBDesc.Height = SCDesc.Height; + FBDesc.NumArraySlices = 1; + pDevice->CreateFramebuffer(FBDesc, &pFramebuffer); + ASSERT_TRUE(pFramebuffer != nullptr); + } + RefCntAutoPtr pPSO; pDevice->CreateGraphicsPipelineState(PSOCreateInfo, &pPSO); ASSERT_NE(pPSO, nullptr); @@ -345,18 +370,45 @@ TEST(ComputeShaderTest, FillTexturePS) pPSO->CreateShaderResourceBinding(&pSRB, true); ASSERT_NE(pSRB, nullptr); + ITextureView* pRTVs[] = {pSwapChain->GetCurrentBackBufferRTV()}; + pContext->SetRenderTargets(1, pRTVs, pSwapChain->GetDepthBufferDSV(), RESOURCE_STATE_TRANSITION_MODE_TRANSITION); + pContext->SetPipelineState(pPSO); pContext->CommitShaderResources(pSRB, RESOURCE_STATE_TRANSITION_MODE_TRANSITION); - pContext->SetRenderTargets(0, nullptr, nullptr, RESOURCE_STATE_TRANSITION_MODE_NONE); + if (UseRenderPass) + { + BeginRenderPassAttribs BeginRPAttribs; + BeginRPAttribs.pRenderPass = pRenderPass; + BeginRPAttribs.pFramebuffer = pFramebuffer; + pContext->BeginRenderPass(BeginRPAttribs); + } + else + { + pContext->SetRenderTargets(0, nullptr, nullptr, RESOURCE_STATE_TRANSITION_MODE_NONE); + } - const auto& SCDesc = pSwapChain->GetDesc(); - Viewport VP{SCDesc}; + Viewport VP{SCDesc}; pContext->SetViewports(1, &VP, SCDesc.Width, SCDesc.Height); pContext->Draw(DrawAttribs{3, DRAW_FLAG_VERIFY_ALL}); + if (UseRenderPass) + { + pContext->EndRenderPass(); + } + pSwapChain->Present(); } +TEST(ComputeShaderTest, FillTexturePS) +{ + TestFillTexturePS(false); +} + +TEST(ComputeShaderTest, FillTexturePS_InRenderPass) +{ + TestFillTexturePS(true); +} + } // namespace