- 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

@@ -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<GLDataBuffer*>(mPresentShader->Uniforms.GetBuffer())->BindBase();
 	RenderScreenQuad();
 }
 

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 <random>
 
 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<GLDataBuffer*>(shader->Uniforms.get())->BindBase();
 	}
 
 	// Set shader

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<GLDataBuffer*>(screen->mShadowMap.mLightList)->BindBase();
+		static_cast<GLDataBuffer*>(screen->mShadowMap.mNodesBuffer)->BindBase();
+		static_cast<GLDataBuffer*>(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<GLDataBuffer*>(mShadowMapShader->Uniforms.GetBuffer())->BindBase();
 
 		glViewport(0, 0, gl_shadowmap_quality, 1024);
 		RenderScreenQuad();

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<GLDataBuffer*>(screen->mLights->GetBuffer())->BindRange(nullptr, start, size);
+		}
+	}
+
+	activeShader->muLightIndex.Set(index);
 	return true;
 }
 

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

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"
 
 //==========================================================================
 //

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<GLDataBuffer*>(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<GLDataBuffer*>(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<GLDataBuffer*>(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<GLDataBuffer*>(mPresent3dCheckerShader->Uniforms.GetBuffer())->BindBase();
 	RenderScreenQuad();
 }
 

src/rendering/gl/system/gl_buffers.cpp

@@ -210,9 +210,8 @@ 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);
 }
 

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();
 };
 
 }

src/rendering/gl/system/gl_framebuffer.cpp

@@ -160,6 +160,8 @@ void OpenGLFrameBuffer::InitializeState()
 	GLRenderer = new FGLRenderer(this);
 	GLRenderer->Initialize(GetWidth(), GetHeight());
 
+	static_cast<GLDataBuffer*>(mLights->GetBuffer())->BindBase();
+
 	mDebug = std::make_shared<FGLDebug>();
 	mDebug->Update();
 }

src/rendering/hwrenderer/data/buffers.h

@@ -3,6 +3,8 @@
 #include <stddef.h>
 #include <assert.h>
 
+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;
 
 };

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;

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; }

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);
 }
 

src/rendering/hwrenderer/dynlights/hw_lightbuffer.h

@@ -15,7 +15,6 @@ class FLightBuffer
 
 	bool mBufferType;
     std::atomic<unsigned int> 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
+	// OpenGL needs the buffer to mess around with the binding.
+	IDataBuffer* GetBuffer() 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)
-	{
-		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;
 	}
 
 };

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;

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

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);
 

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<VkRenderState*>(state)->Bind(bindingpoint, (uint32_t)start);
 }
 
-void VKDataBuffer::BindBase()
-{
-	GetVulkanFrameBuffer()->GetRenderState()->Bind(bindingpoint, 0);
-}

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;
 };