From 776d4747b2fc17fce67a8a8ef34c6e77f0adfb63 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Fri, 24 Jul 2015 14:47:44 -0700 Subject: [PATCH] Cleaning up the FBO cache and the output stage in general --- libraries/gpu/src/gpu/Batch.cpp | 59 ++++++++------- libraries/gpu/src/gpu/Batch.h | 25 +++---- libraries/gpu/src/gpu/GLBackend.cpp | 67 +---------------- libraries/gpu/src/gpu/GLBackend.h | 3 +- libraries/gpu/src/gpu/GLBackendOutput.cpp | 91 +++++++++++++++++++++-- 5 files changed, 127 insertions(+), 118 deletions(-) diff --git a/libraries/gpu/src/gpu/Batch.cpp b/libraries/gpu/src/gpu/Batch.cpp index 567ce66cd8..4ac33d8f14 100644 --- a/libraries/gpu/src/gpu/Batch.cpp +++ b/libraries/gpu/src/gpu/Batch.cpp @@ -106,36 +106,6 @@ void Batch::drawIndexedInstanced(uint32 nbInstances, Primitive primitiveType, ui _params.push_back(nbInstances); } -void Batch::clearFramebuffer(Framebuffer::Masks targets, const Vec4& color, float depth, int stencil, bool enableScissor) { - ADD_COMMAND(clearFramebuffer); - - _params.push_back(enableScissor); - _params.push_back(stencil); - _params.push_back(depth); - _params.push_back(color.w); - _params.push_back(color.z); - _params.push_back(color.y); - _params.push_back(color.x); - _params.push_back(targets); -} - -void Batch::clearColorFramebuffer(Framebuffer::Masks targets, const Vec4& color, bool enableScissor) { - clearFramebuffer(targets & Framebuffer::BUFFER_COLORS, color, 1.0f, 0, enableScissor); -} - -void Batch::clearDepthFramebuffer(float depth, bool enableScissor) { - clearFramebuffer(Framebuffer::BUFFER_DEPTH, Vec4(0.0f), depth, 0, enableScissor); -} - -void Batch::clearStencilFramebuffer(int stencil, bool enableScissor) { - clearFramebuffer(Framebuffer::BUFFER_STENCIL, Vec4(0.0f), 1.0f, stencil, enableScissor); -} - -void Batch::clearDepthStencilFramebuffer(float depth, int stencil, bool enableScissor) { - clearFramebuffer(Framebuffer::BUFFER_DEPTHSTENCIL, Vec4(0.0f), depth, stencil, enableScissor); -} - - void Batch::setInputFormat(const Stream::FormatPointer& format) { ADD_COMMAND(setInputFormat); @@ -255,6 +225,35 @@ void Batch::setFramebuffer(const FramebufferPointer& framebuffer) { } +void Batch::clearFramebuffer(Framebuffer::Masks targets, const Vec4& color, float depth, int stencil, bool enableScissor) { + ADD_COMMAND(clearFramebuffer); + + _params.push_back(enableScissor); + _params.push_back(stencil); + _params.push_back(depth); + _params.push_back(color.w); + _params.push_back(color.z); + _params.push_back(color.y); + _params.push_back(color.x); + _params.push_back(targets); +} + +void Batch::clearColorFramebuffer(Framebuffer::Masks targets, const Vec4& color, bool enableScissor) { + clearFramebuffer(targets & Framebuffer::BUFFER_COLORS, color, 1.0f, 0, enableScissor); +} + +void Batch::clearDepthFramebuffer(float depth, bool enableScissor) { + clearFramebuffer(Framebuffer::BUFFER_DEPTH, Vec4(0.0f), depth, 0, enableScissor); +} + +void Batch::clearStencilFramebuffer(int stencil, bool enableScissor) { + clearFramebuffer(Framebuffer::BUFFER_STENCIL, Vec4(0.0f), 1.0f, stencil, enableScissor); +} + +void Batch::clearDepthStencilFramebuffer(float depth, int stencil, bool enableScissor) { + clearFramebuffer(Framebuffer::BUFFER_DEPTHSTENCIL, Vec4(0.0f), depth, stencil, enableScissor); +} + void Batch::blit(const FramebufferPointer& src, const Vec4i& srcViewport, const FramebufferPointer& dst, const Vec4i& dstViewport) { ADD_COMMAND(blit); diff --git a/libraries/gpu/src/gpu/Batch.h b/libraries/gpu/src/gpu/Batch.h index acc1f6fdac..58fe03c9cf 100644 --- a/libraries/gpu/src/gpu/Batch.h +++ b/libraries/gpu/src/gpu/Batch.h @@ -54,15 +54,6 @@ public: void drawInstanced(uint32 nbInstances, Primitive primitiveType, uint32 nbVertices, uint32 startVertex = 0, uint32 startInstance = 0); void drawIndexedInstanced(uint32 nbInstances, Primitive primitiveType, uint32 nbIndices, uint32 startIndex = 0, uint32 startInstance = 0); - // Clear framebuffer layers - // Targets can be any of the render buffers contained in the Framebuffer - // Optionally the scissor test can be enabled locally for this command and to restrict the clearing command to the pixels contained in the scissor rectangle - void clearFramebuffer(Framebuffer::Masks targets, const Vec4& color, float depth, int stencil, bool enableScissor = false); - void clearColorFramebuffer(Framebuffer::Masks targets, const Vec4& color, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, mask out targets to make sure it touches only color targets - void clearDepthFramebuffer(float depth, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches only depth target - void clearStencilFramebuffer(int stencil, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches only stencil target - void clearDepthStencilFramebuffer(float depth, int stencil, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches depth and stencil target - // Input Stage // InputFormat // InputBuffers @@ -105,8 +96,17 @@ public: // Framebuffer Stage void setFramebuffer(const FramebufferPointer& framebuffer); - void blit(const FramebufferPointer& src, const Vec4i& srcViewport, - const FramebufferPointer& dst, const Vec4i& dstViewport); + + // Clear framebuffer layers + // Targets can be any of the render buffers contained in the currnetly bound Framebuffer + // Optionally the scissor test can be enabled locally for this command and to restrict the clearing command to the pixels contained in the scissor rectangle + void clearFramebuffer(Framebuffer::Masks targets, const Vec4& color, float depth, int stencil, bool enableScissor = false); + void clearColorFramebuffer(Framebuffer::Masks targets, const Vec4& color, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, mask out targets to make sure it touches only color targets + void clearDepthFramebuffer(float depth, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches only depth target + void clearStencilFramebuffer(int stencil, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches only stencil target + void clearDepthStencilFramebuffer(float depth, int stencil, bool enableScissor = false); // not a command, just a shortcut for clearFramebuffer, it touches depth and stencil target + + void blit(const FramebufferPointer& src, const Vec4i& srcViewport, const FramebufferPointer& dst, const Vec4i& dstViewport); // Query Section void beginQuery(const QueryPointer& query); @@ -162,8 +162,6 @@ public: COMMAND_drawInstanced, COMMAND_drawIndexedInstanced, - COMMAND_clearFramebuffer, - COMMAND_setInputFormat, COMMAND_setInputBuffer, COMMAND_setIndexBuffer, @@ -181,6 +179,7 @@ public: COMMAND_setResourceTexture, COMMAND_setFramebuffer, + COMMAND_clearFramebuffer, COMMAND_blit, COMMAND_beginQuery, diff --git a/libraries/gpu/src/gpu/GLBackend.cpp b/libraries/gpu/src/gpu/GLBackend.cpp index ae50b96bc5..6b1d552be9 100644 --- a/libraries/gpu/src/gpu/GLBackend.cpp +++ b/libraries/gpu/src/gpu/GLBackend.cpp @@ -21,7 +21,6 @@ GLBackend::CommandCall GLBackend::_commandCalls[Batch::NUM_COMMANDS] = (&::gpu::GLBackend::do_drawIndexed), (&::gpu::GLBackend::do_drawInstanced), (&::gpu::GLBackend::do_drawIndexedInstanced), - (&::gpu::GLBackend::do_clearFramebuffer), (&::gpu::GLBackend::do_setInputFormat), (&::gpu::GLBackend::do_setInputBuffer), @@ -40,6 +39,7 @@ GLBackend::CommandCall GLBackend::_commandCalls[Batch::NUM_COMMANDS] = (&::gpu::GLBackend::do_setResourceTexture), (&::gpu::GLBackend::do_setFramebuffer), + (&::gpu::GLBackend::do_clearFramebuffer), (&::gpu::GLBackend::do_blit), (&::gpu::GLBackend::do_beginQuery), @@ -246,71 +246,6 @@ void GLBackend::do_drawIndexedInstanced(Batch& batch, uint32 paramOffset) { (void) CHECK_GL_ERROR(); } -void GLBackend::do_clearFramebuffer(Batch& batch, uint32 paramOffset) { - - uint32 masks = batch._params[paramOffset + 7]._uint; - Vec4 color; - color.x = batch._params[paramOffset + 6]._float; - color.y = batch._params[paramOffset + 5]._float; - color.z = batch._params[paramOffset + 4]._float; - color.w = batch._params[paramOffset + 3]._float; - float depth = batch._params[paramOffset + 2]._float; - int stencil = batch._params[paramOffset + 1]._int; - int useScissor = batch._params[paramOffset + 0]._int; - - GLuint glmask = 0; - if (masks & Framebuffer::BUFFER_STENCIL) { - glClearStencil(stencil); - glmask |= GL_STENCIL_BUFFER_BIT; - } - - if (masks & Framebuffer::BUFFER_DEPTH) { - glClearDepth(depth); - glmask |= GL_DEPTH_BUFFER_BIT; - } - - std::vector drawBuffers; - if (masks & Framebuffer::BUFFER_COLORS) { - for (unsigned int i = 0; i < Framebuffer::MAX_NUM_RENDER_BUFFERS; i++) { - if (masks & (1 << i)) { - drawBuffers.push_back(GL_COLOR_ATTACHMENT0 + i); - } - } - - if (!drawBuffers.empty()) { - glDrawBuffers(drawBuffers.size(), drawBuffers.data()); - glClearColor(color.x, color.y, color.z, color.w); - glmask |= GL_COLOR_BUFFER_BIT; - } - - // Force the color mask cache to WRITE_ALL if not the case - do_setStateColorWriteMask(State::ColorMask::WRITE_ALL); - } - - // Apply scissor if needed and if not already on - bool doEnableScissor = (useScissor && (!_pipeline._stateCache.scissorEnable)); - if (doEnableScissor) { - glEnable(GL_SCISSOR_TEST); - } - - glClear(glmask); - - // Restore scissor if needed - if (doEnableScissor) { - glDisable(GL_SCISSOR_TEST); - } - - // Restore the color draw buffers only if a frmaebuffer is bound - if (_output._framebuffer && !drawBuffers.empty()) { - auto glFramebuffer = syncGPUObject(*_output._framebuffer); - if (glFramebuffer) { - glDrawBuffers(glFramebuffer->_colorBuffers.size(), glFramebuffer->_colorBuffers.data()); - } - } - - (void) CHECK_GL_ERROR(); -} - // TODO: As long as we have gl calls explicitely issued from interface // code, we need to be able to record and batch these calls. THe long // term strategy is to get rid of any GL calls in favor of the HIFI GPU API diff --git a/libraries/gpu/src/gpu/GLBackend.h b/libraries/gpu/src/gpu/GLBackend.h index 894e2c4548..9d8c9ef805 100644 --- a/libraries/gpu/src/gpu/GLBackend.h +++ b/libraries/gpu/src/gpu/GLBackend.h @@ -241,8 +241,6 @@ protected: void do_drawInstanced(Batch& batch, uint32 paramOffset); void do_drawIndexedInstanced(Batch& batch, uint32 paramOffset); - void do_clearFramebuffer(Batch& batch, uint32 paramOffset); - // Input Stage void do_setInputFormat(Batch& batch, uint32 paramOffset); void do_setInputBuffer(Batch& batch, uint32 paramOffset); @@ -385,6 +383,7 @@ protected: // Output stage void do_setFramebuffer(Batch& batch, uint32 paramOffset); + void do_clearFramebuffer(Batch& batch, uint32 paramOffset); void do_blit(Batch& batch, uint32 paramOffset); // Synchronize the state cache of this Backend with the actual real state of the GL Context diff --git a/libraries/gpu/src/gpu/GLBackendOutput.cpp b/libraries/gpu/src/gpu/GLBackendOutput.cpp index 1b22649ad6..b37def48e7 100755 --- a/libraries/gpu/src/gpu/GLBackendOutput.cpp +++ b/libraries/gpu/src/gpu/GLBackendOutput.cpp @@ -180,7 +180,6 @@ void GLBackend::syncOutputStateCache() { void GLBackend::do_setFramebuffer(Batch& batch, uint32 paramOffset) { auto framebuffer = batch._framebuffers.get(batch._params[paramOffset]._uint); - if (_output._framebuffer != framebuffer) { auto newFBO = getFramebufferID(framebuffer); if (_output._drawFBO != newFBO) { @@ -191,6 +190,72 @@ void GLBackend::do_setFramebuffer(Batch& batch, uint32 paramOffset) { } } +void GLBackend::do_clearFramebuffer(Batch& batch, uint32 paramOffset) { + + uint32 masks = batch._params[paramOffset + 7]._uint; + Vec4 color; + color.x = batch._params[paramOffset + 6]._float; + color.y = batch._params[paramOffset + 5]._float; + color.z = batch._params[paramOffset + 4]._float; + color.w = batch._params[paramOffset + 3]._float; + float depth = batch._params[paramOffset + 2]._float; + int stencil = batch._params[paramOffset + 1]._int; + int useScissor = batch._params[paramOffset + 0]._int; + + GLuint glmask = 0; + if (masks & Framebuffer::BUFFER_STENCIL) { + glClearStencil(stencil); + glmask |= GL_STENCIL_BUFFER_BIT; + } + + if (masks & Framebuffer::BUFFER_DEPTH) { + glClearDepth(depth); + glmask |= GL_DEPTH_BUFFER_BIT; + } + + std::vector drawBuffers; + if (masks & Framebuffer::BUFFER_COLORS) { + for (unsigned int i = 0; i < Framebuffer::MAX_NUM_RENDER_BUFFERS; i++) { + if (masks & (1 << i)) { + drawBuffers.push_back(GL_COLOR_ATTACHMENT0 + i); + } + } + + if (!drawBuffers.empty()) { + glDrawBuffers(drawBuffers.size(), drawBuffers.data()); + glClearColor(color.x, color.y, color.z, color.w); + glmask |= GL_COLOR_BUFFER_BIT; + } + + // Force the color mask cache to WRITE_ALL if not the case + do_setStateColorWriteMask(State::ColorMask::WRITE_ALL); + } + + // Apply scissor if needed and if not already on + bool doEnableScissor = (useScissor && (!_pipeline._stateCache.scissorEnable)); + if (doEnableScissor) { + glEnable(GL_SCISSOR_TEST); + } + + // Clear! + glClear(glmask); + + // Restore scissor if needed + if (doEnableScissor) { + glDisable(GL_SCISSOR_TEST); + } + + // Restore the color draw buffers only if a frmaebuffer is bound + if (_output._framebuffer && !drawBuffers.empty()) { + auto glFramebuffer = syncGPUObject(*_output._framebuffer); + if (glFramebuffer) { + glDrawBuffers(glFramebuffer->_colorBuffers.size(), glFramebuffer->_colorBuffers.data()); + } + } + + (void) CHECK_GL_ERROR(); +} + void GLBackend::do_blit(Batch& batch, uint32 paramOffset) { auto srcframebuffer = batch._framebuffers.get(batch._params[paramOffset]._uint); Vec4i srcvp; @@ -203,19 +268,31 @@ void GLBackend::do_blit(Batch& batch, uint32 paramOffset) { for (size_t i = 0; i < 4; ++i) { dstvp[i] = batch._params[paramOffset + 6 + i]._int; } - glBindFramebuffer(GL_DRAW_FRAMEBUFFER, getFramebufferID(dstframebuffer)); + + // Assign dest framebuffer if not bound already + auto newDrawFBO = getFramebufferID(dstframebuffer); + if (_output._drawFBO != newDrawFBO) { + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, newDrawFBO); + } + + // always bind the read fbo glBindFramebuffer(GL_READ_FRAMEBUFFER, getFramebufferID(srcframebuffer)); + + // Blit! glBlitFramebuffer(srcvp.x, srcvp.y, srcvp.z, srcvp.w, dstvp.x, dstvp.y, dstvp.z, dstvp.w, GL_COLOR_BUFFER_BIT, GL_LINEAR); - - (void) CHECK_GL_ERROR(); - if (_output._framebuffer) { - glBindFramebuffer(GL_DRAW_FRAMEBUFFER, getFramebufferID(_output._framebuffer)); + // Always clean the read fbo to 0 + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); + + // Restore draw fbo if changed + if (_output._drawFBO != newDrawFBO) { + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, _output._drawFBO); } -} + (void) CHECK_GL_ERROR(); +} void GLBackend::downloadFramebuffer(const FramebufferPointer& srcFramebuffer, const Vec4i& region, QImage& destImage) { auto readFBO = gpu::GLBackend::getFramebufferID(srcFramebuffer);