From 037b69c8a7971c829e3622d04f56d5c2fb1df71e Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 9 Jun 2019 20:37:11 +0200 Subject: [PATCH] - reworked buffer binding logic. This shouldn't be in the hardware independent interface because the semantics on OpenGL and Vulkan are too different, so a common implementation is not possible. Most bind calls were in the GL interface anyway, so these no longer pass through hardware independent code. This also moves the bind calls in the shadowmap code into the GL interface - these never did anything useful in Vulkan and aren't needed there. Last but not least, this moves the legacy buffer binding handling into FGLRenderState and performs the initial binding for the light buffer in a more suitable place so that this doesn't have to pollute the render state. --- src/rendering/gl/renderer/gl_postprocess.cpp | 3 +- .../gl/renderer/gl_renderbuffers.cpp | 3 +- src/rendering/gl/renderer/gl_renderer.cpp | 8 ++++- src/rendering/gl/renderer/gl_renderstate.cpp | 16 ++++++++-- src/rendering/gl/renderer/gl_renderstate.h | 1 + src/rendering/gl/renderer/gl_scene.cpp | 1 + src/rendering/gl/renderer/gl_stereo3d.cpp | 13 ++++++--- src/rendering/gl/system/gl_buffers.cpp | 5 ++-- src/rendering/gl/system/gl_buffers.h | 4 +-- src/rendering/gl/system/gl_framebuffer.cpp | 2 ++ src/rendering/hwrenderer/data/buffers.h | 5 ++-- .../hwrenderer/data/hw_viewpointbuffer.cpp | 2 +- .../hwrenderer/data/shaderuniforms.h | 10 ++++--- .../hwrenderer/dynlights/hw_lightbuffer.cpp | 10 ++----- .../hwrenderer/dynlights/hw_lightbuffer.h | 29 +++---------------- .../hwrenderer/dynlights/hw_shadowmap.cpp | 3 -- .../hwrenderer/dynlights/hw_shadowmap.h | 2 ++ .../hwrenderer/scene/hw_drawinfo.cpp | 1 - src/rendering/vulkan/system/vk_buffers.cpp | 9 ++---- src/rendering/vulkan/system/vk_buffers.h | 3 +- 20 files changed, 65 insertions(+), 65 deletions(-) diff --git a/src/rendering/gl/renderer/gl_postprocess.cpp b/src/rendering/gl/renderer/gl_postprocess.cpp index 85e802a4f..4e0744fb4 100644 --- a/src/rendering/gl/renderer/gl_postprocess.cpp +++ b/src/rendering/gl/renderer/gl_postprocess.cpp @@ -235,7 +235,8 @@ void FGLRenderer::DrawPresentTexture(const IntRect &box, bool applyGamma) } mPresentShader->Uniforms->Scale = { screen->mScreenViewport.width / (float)mBuffers->GetWidth(), screen->mScreenViewport.height / (float)mBuffers->GetHeight() }; mPresentShader->Uniforms->Offset = { 0.0f, 0.0f }; - mPresentShader->Uniforms.Set(); + mPresentShader->Uniforms.SetData(); + static_cast(mPresentShader->Uniforms.GetBuffer())->BindBase(); RenderScreenQuad(); } diff --git a/src/rendering/gl/renderer/gl_renderbuffers.cpp b/src/rendering/gl/renderer/gl_renderbuffers.cpp index fafd6b794..eeb79ae40 100644 --- a/src/rendering/gl/renderer/gl_renderbuffers.cpp +++ b/src/rendering/gl/renderer/gl_renderbuffers.cpp @@ -34,6 +34,7 @@ #include "gl/renderer/gl_renderbuffers.h" #include "gl/renderer/gl_postprocessstate.h" #include "gl/shaders/gl_shaderprogram.h" +#include "gl/system/gl_buffers.h" #include CVAR(Int, gl_multisample, 1, CVAR_ARCHIVE|CVAR_GLOBALCONFIG); @@ -955,7 +956,7 @@ void GLPPRenderState::Draw() if (!shader->Uniforms) shader->Uniforms.reset(screen->CreateDataBuffer(POSTPROCESS_BINDINGPOINT, false, false)); shader->Uniforms->SetData(Uniforms.Data.Size(), Uniforms.Data.Data()); - shader->Uniforms->BindBase(); + static_cast(shader->Uniforms.get())->BindBase(); } // Set shader diff --git a/src/rendering/gl/renderer/gl_renderer.cpp b/src/rendering/gl/renderer/gl_renderer.cpp index 73758d5a2..49ea8b0f4 100644 --- a/src/rendering/gl/renderer/gl_renderer.cpp +++ b/src/rendering/gl/renderer/gl_renderer.cpp @@ -58,6 +58,7 @@ #include "r_videoscale.h" #include "r_data/models/models.h" #include "gl/renderer/gl_postprocessstate.h" +#include "gl/system/gl_buffers.h" EXTERN_CVAR(Int, screenblocks) EXTERN_CVAR(Bool, cl_capfps) @@ -191,12 +192,17 @@ void FGLRenderer::UpdateShadowMap() FGLPostProcessState savedState; + static_cast(screen->mShadowMap.mLightList)->BindBase(); + static_cast(screen->mShadowMap.mNodesBuffer)->BindBase(); + static_cast(screen->mShadowMap.mLinesBuffer)->BindBase(); + mBuffers->BindShadowMapFB(); mShadowMapShader->Bind(); mShadowMapShader->Uniforms->ShadowmapQuality = gl_shadowmap_quality; mShadowMapShader->Uniforms->NodesCount = screen->mShadowMap.NodesCount(); - mShadowMapShader->Uniforms.Set(); + mShadowMapShader->Uniforms.SetData(); + static_cast(mShadowMapShader->Uniforms.GetBuffer())->BindBase(); glViewport(0, 0, gl_shadowmap_quality, 1024); RenderScreenQuad(); diff --git a/src/rendering/gl/renderer/gl_renderstate.cpp b/src/rendering/gl/renderer/gl_renderstate.cpp index 28f6cd5d3..29883afc8 100644 --- a/src/rendering/gl/renderer/gl_renderstate.cpp +++ b/src/rendering/gl/renderer/gl_renderstate.cpp @@ -189,9 +189,21 @@ bool FGLRenderState::ApplyShader() matrixToGL(identityMatrix, activeShader->normalmodelmatrix_index); } - auto index = screen->mLights->BindUBO(mLightIndex); - activeShader->muLightIndex.Set(index); + int index = mLightIndex; + // Mess alert for crappy AncientGL! + if (!screen->mLights->GetBufferType() && index >= 0) + { + size_t start, size; + index = screen->mLights->GetBinding(index, &start, &size); + if (start != mLastMappedLightIndex) + { + mLastMappedLightIndex = start; + static_cast(screen->mLights->GetBuffer())->BindRange(nullptr, start, size); + } + } + + activeShader->muLightIndex.Set(index); return true; } diff --git a/src/rendering/gl/renderer/gl_renderstate.h b/src/rendering/gl/renderer/gl_renderstate.h index 7dce53980..5d2e0d383 100644 --- a/src/rendering/gl/renderer/gl_renderstate.h +++ b/src/rendering/gl/renderer/gl_renderstate.h @@ -68,6 +68,7 @@ class FGLRenderState : public FRenderState int lastClamp = 0; int lastTranslation = 0; int maxBoundMaterial = -1; + size_t mLastMappedLightIndex = SIZE_MAX; IVertexBuffer *mCurrentVertexBuffer; int mCurrentVertexOffsets[2]; // one per binding point diff --git a/src/rendering/gl/renderer/gl_scene.cpp b/src/rendering/gl/renderer/gl_scene.cpp index 5102b3a0a..af6c6e581 100644 --- a/src/rendering/gl/renderer/gl_scene.cpp +++ b/src/rendering/gl/renderer/gl_scene.cpp @@ -57,6 +57,7 @@ #include "hwrenderer/scene/hw_portal.h" #include "hwrenderer/utility/hw_vrmodes.h" #include "gl/renderer/gl_renderer.h" +#include "gl/system/gl_buffers.h" //========================================================================== // diff --git a/src/rendering/gl/renderer/gl_stereo3d.cpp b/src/rendering/gl/renderer/gl_stereo3d.cpp index 9483310ba..fe5b29b22 100644 --- a/src/rendering/gl/renderer/gl_stereo3d.cpp +++ b/src/rendering/gl/renderer/gl_stereo3d.cpp @@ -33,6 +33,7 @@ #include "gl/renderer/gl_postprocessstate.h" #include "gl/system/gl_framebuffer.h" #include "gl/shaders/gl_shaderprogram.h" +#include "gl/system/gl_buffers.h" #include "menu/menu.h" EXTERN_CVAR(Int, vr_mode) @@ -174,7 +175,8 @@ void FGLRenderer::prepareInterleavedPresent(FPresentShaderBase& shader) screen->mScreenViewport.height / (float)mBuffers->GetHeight() }; shader.Uniforms->Offset = { 0.0f, 0.0f }; - shader.Uniforms.Set(); + shader.Uniforms.SetData(); + static_cast(shader.Uniforms.GetBuffer())->BindBase(); } //========================================================================== @@ -198,7 +200,8 @@ void FGLRenderer::PresentColumnInterleaved() int windowHOffset = 0; mPresent3dColumnShader->Uniforms->WindowPositionParity = windowHOffset; - mPresent3dColumnShader->Uniforms.Set(); + mPresent3dColumnShader->Uniforms.SetData(); + static_cast(mPresent3dColumnShader->Uniforms.GetBuffer())->BindBase(); RenderScreenQuad(); } @@ -225,7 +228,8 @@ void FGLRenderer::PresentRowInterleaved() + screen->mOutputLetterbox.height + 1 // +1 because of origin at bottom ) % 2; - mPresent3dRowShader->Uniforms.Set(); + mPresent3dRowShader->Uniforms.SetData(); + static_cast(mPresent3dRowShader->Uniforms.GetBuffer())->BindBase(); RenderScreenQuad(); } @@ -256,7 +260,8 @@ void FGLRenderer::PresentCheckerInterleaved() + screen->mOutputLetterbox.height + 1 // +1 because of origin at bottom ) % 2; // because we want the top pixel offset, but gl_FragCoord.y is the bottom pixel offset - mPresent3dCheckerShader->Uniforms.Set(); + mPresent3dCheckerShader->Uniforms.SetData(); + static_cast(mPresent3dCheckerShader->Uniforms.GetBuffer())->BindBase(); RenderScreenQuad(); } diff --git a/src/rendering/gl/system/gl_buffers.cpp b/src/rendering/gl/system/gl_buffers.cpp index 5e1617edb..f7cd34a3e 100644 --- a/src/rendering/gl/system/gl_buffers.cpp +++ b/src/rendering/gl/system/gl_buffers.cpp @@ -210,10 +210,9 @@ void GLVertexBuffer::Bind(int *offsets) } } -void GLDataBuffer::BindRange(size_t start, size_t length) +void GLDataBuffer::BindRange(FRenderState *state, size_t start, size_t length) { - if (mUseType == GL_UNIFORM_BUFFER) // SSBO's cannot be rebound. - glBindBufferRange(mUseType, mBindingPoint, mBufferId, start, length); + glBindBufferRange(mUseType, mBindingPoint, mBufferId, start, length); } void GLDataBuffer::BindBase() diff --git a/src/rendering/gl/system/gl_buffers.h b/src/rendering/gl/system/gl_buffers.h index 8231855b8..a9e44061d 100644 --- a/src/rendering/gl/system/gl_buffers.h +++ b/src/rendering/gl/system/gl_buffers.h @@ -66,8 +66,8 @@ class GLDataBuffer : public IDataBuffer, public GLBuffer int mBindingPoint; public: GLDataBuffer(int bindingpoint, bool is_ssbo) : GLBuffer(is_ssbo? GL_SHADER_STORAGE_BUFFER : GL_UNIFORM_BUFFER), mBindingPoint(bindingpoint) {} - void BindRange(size_t start, size_t length) override; - void BindBase() override; + void BindRange(FRenderState* state, size_t start, size_t length); + void BindBase(); }; } \ No newline at end of file diff --git a/src/rendering/gl/system/gl_framebuffer.cpp b/src/rendering/gl/system/gl_framebuffer.cpp index 68c3da58d..67ce8c2d3 100644 --- a/src/rendering/gl/system/gl_framebuffer.cpp +++ b/src/rendering/gl/system/gl_framebuffer.cpp @@ -160,6 +160,8 @@ void OpenGLFrameBuffer::InitializeState() GLRenderer = new FGLRenderer(this); GLRenderer->Initialize(GetWidth(), GetHeight()); + static_cast(mLights->GetBuffer())->BindBase(); + mDebug = std::make_shared(); mDebug->Update(); } diff --git a/src/rendering/hwrenderer/data/buffers.h b/src/rendering/hwrenderer/data/buffers.h index b5707c229..51110c72c 100644 --- a/src/rendering/hwrenderer/data/buffers.h +++ b/src/rendering/hwrenderer/data/buffers.h @@ -3,6 +3,8 @@ #include #include +class FRenderState; + // The low level code needs to know which attributes exist. // OpenGL needs to change the state of all of them per buffer binding. // VAOs are mostly useless for this because they lump buffer and binding state together which the model code does not want. @@ -76,7 +78,6 @@ class IDataBuffer : virtual public IBuffer { // Can be either uniform or shader storage buffer, depending on its needs. public: - virtual void BindRange(size_t start, size_t length) = 0; - virtual void BindBase() = 0; + virtual void BindRange(FRenderState *state, size_t start, size_t length) = 0; }; diff --git a/src/rendering/hwrenderer/data/hw_viewpointbuffer.cpp b/src/rendering/hwrenderer/data/hw_viewpointbuffer.cpp index f916e8213..5ed4a5183 100644 --- a/src/rendering/hwrenderer/data/hw_viewpointbuffer.cpp +++ b/src/rendering/hwrenderer/data/hw_viewpointbuffer.cpp @@ -67,7 +67,7 @@ int GLViewpointBuffer::Bind(FRenderState &di, unsigned int index) if (index != mLastMappedIndex) { mLastMappedIndex = index; - mBuffer->BindRange(index * mBlockAlign, mBlockAlign); + mBuffer->BindRange(&di, index * mBlockAlign, mBlockAlign); di.EnableClipDistance(0, mClipPlaneInfo[index]); } return index; diff --git a/src/rendering/hwrenderer/data/shaderuniforms.h b/src/rendering/hwrenderer/data/shaderuniforms.h index 636f41aea..2543b52b1 100644 --- a/src/rendering/hwrenderer/data/shaderuniforms.h +++ b/src/rendering/hwrenderer/data/shaderuniforms.h @@ -126,14 +126,16 @@ public: mBuffer = screen->CreateDataBuffer(bindingpoint, false, false); } - void Set(bool bind = true) + void SetData() { if (mBuffer != nullptr) mBuffer->SetData(sizeof(T), &Values); + } - // Let's hope this can be done better when things have moved further ahead. - // This really is not the best place to add something that depends on API behavior. - if (bind) mBuffer->BindBase(); + IDataBuffer* GetBuffer() const + { + // OpenGL needs to mess around with this in ways that should not be part of the interface. + return mBuffer; } T *operator->() { return &Values; } diff --git a/src/rendering/hwrenderer/dynlights/hw_lightbuffer.cpp b/src/rendering/hwrenderer/dynlights/hw_lightbuffer.cpp index 1cb9b0d3c..68a1fad2d 100644 --- a/src/rendering/hwrenderer/dynlights/hw_lightbuffer.cpp +++ b/src/rendering/hwrenderer/dynlights/hw_lightbuffer.cpp @@ -74,7 +74,6 @@ FLightBuffer::~FLightBuffer() void FLightBuffer::Clear() { mIndex = 0; - mLastMappedIndex = UINT_MAX; } int FLightBuffer::UploadLights(FDynLightData &data) @@ -127,16 +126,13 @@ int FLightBuffer::UploadLights(FDynLightData &data) } } -int FLightBuffer::DoBindUBO(unsigned int index) +int FLightBuffer::GetBinding(unsigned int index, size_t* pOffset, size_t* pSize) { // this function will only get called if a uniform buffer is used. For a shader storage buffer we only need to bind the buffer once at the start. unsigned int offset = (index / mBlockAlign) * mBlockAlign; - if (offset != mLastMappedIndex) - { - mLastMappedIndex = offset; - mBuffer->BindRange(offset * ELEMENT_SIZE, mBlockSize * ELEMENT_SIZE); - } + *pOffset = offset * ELEMENT_SIZE; + *pSize = mBlockSize * ELEMENT_SIZE; return (index - offset); } diff --git a/src/rendering/hwrenderer/dynlights/hw_lightbuffer.h b/src/rendering/hwrenderer/dynlights/hw_lightbuffer.h index 1db32f646..df9d679a7 100644 --- a/src/rendering/hwrenderer/dynlights/hw_lightbuffer.h +++ b/src/rendering/hwrenderer/dynlights/hw_lightbuffer.h @@ -15,7 +15,6 @@ class FLightBuffer bool mBufferType; std::atomic mIndex; - unsigned int mLastMappedIndex; unsigned int mBlockAlign; unsigned int mBlockSize; unsigned int mBufferSize; @@ -34,32 +33,12 @@ public: void Unmap() { mBuffer->Unmap(); } unsigned int GetBlockSize() const { return mBlockSize; } bool GetBufferType() const { return mBufferType; } + int GetBinding(unsigned int index, size_t* pOffset, size_t* pSize); - int DoBindUBO(unsigned int index); - - int ShaderIndex(unsigned int index) const - { - if (mBlockAlign == 0) return index; - // This must match the math in BindUBO. - unsigned int offset = (index / mBlockAlign) * mBlockAlign; - return int(index-offset); - } - - // Only relevant for OpenGL, so this does not need access to the render state. - int BindUBO(int index) + // OpenGL needs the buffer to mess around with the binding. + IDataBuffer* GetBuffer() const { - if (!mBufferType && index > -1) - { - index = DoBindUBO(index); - } - return index; - } - - // The parameter is a reminder for Vulkan. - void BindBase() - { - mBuffer->BindBase(); - mLastMappedIndex = UINT_MAX; + return mBuffer; } }; diff --git a/src/rendering/hwrenderer/dynlights/hw_shadowmap.cpp b/src/rendering/hwrenderer/dynlights/hw_shadowmap.cpp index a005812ba..aba51140a 100644 --- a/src/rendering/hwrenderer/dynlights/hw_shadowmap.cpp +++ b/src/rendering/hwrenderer/dynlights/hw_shadowmap.cpp @@ -176,9 +176,6 @@ bool IShadowMap::PerformUpdate() UpdateCycles.Clock(); UploadAABBTree(); UploadLights(); - mLightList->BindBase(); - mNodesBuffer->BindBase(); - mLinesBuffer->BindBase(); return true; } return false; diff --git a/src/rendering/hwrenderer/dynlights/hw_shadowmap.h b/src/rendering/hwrenderer/dynlights/hw_shadowmap.h index a0181363f..bc11f99ba 100644 --- a/src/rendering/hwrenderer/dynlights/hw_shadowmap.h +++ b/src/rendering/hwrenderer/dynlights/hw_shadowmap.h @@ -65,6 +65,8 @@ protected: IShadowMap &operator=(IShadowMap &) = delete; // OpenGL storage buffer with the list of lights in the shadow map texture + // These buffers need to be accessed by the OpenGL backend directly so that they can be bound. +public: IDataBuffer *mLightList = nullptr; // OpenGL storage buffers for the AABB tree diff --git a/src/rendering/hwrenderer/scene/hw_drawinfo.cpp b/src/rendering/hwrenderer/scene/hw_drawinfo.cpp index 595a03ec6..63dc1234e 100644 --- a/src/rendering/hwrenderer/scene/hw_drawinfo.cpp +++ b/src/rendering/hwrenderer/scene/hw_drawinfo.cpp @@ -468,7 +468,6 @@ void HWDrawInfo::RenderScene(FRenderState &state) state.SetDepthMask(true); - screen->mLights->BindBase(); state.EnableFog(true); state.SetRenderStyle(STYLE_Source); diff --git a/src/rendering/vulkan/system/vk_buffers.cpp b/src/rendering/vulkan/system/vk_buffers.cpp index cc570d812..7e02ab8c0 100644 --- a/src/rendering/vulkan/system/vk_buffers.cpp +++ b/src/rendering/vulkan/system/vk_buffers.cpp @@ -204,12 +204,9 @@ void VKVertexBuffer::SetFormat(int numBindingPoints, int numAttributes, size_t s ///////////////////////////////////////////////////////////////////////////// -void VKDataBuffer::BindRange(size_t start, size_t length) + +void VKDataBuffer::BindRange(FRenderState* state, size_t start, size_t length) { - GetVulkanFrameBuffer()->GetRenderState()->Bind(bindingpoint, (uint32_t)start); + static_cast(state)->Bind(bindingpoint, (uint32_t)start); } -void VKDataBuffer::BindBase() -{ - GetVulkanFrameBuffer()->GetRenderState()->Bind(bindingpoint, 0); -} diff --git a/src/rendering/vulkan/system/vk_buffers.h b/src/rendering/vulkan/system/vk_buffers.h index b0bedfd42..e162c2367 100644 --- a/src/rendering/vulkan/system/vk_buffers.h +++ b/src/rendering/vulkan/system/vk_buffers.h @@ -68,8 +68,7 @@ public: } } - void BindRange(size_t start, size_t length) override; - void BindBase() override; + void BindRange(FRenderState *state, size_t start, size_t length) override; int bindingpoint; };