- consolidated buffer implementations.

Since this is nearly identical for different buffer types they should share the same code wherever possible.

src/gl/system/glsys_vertexbuffer.cpp

@@ -30,78 +30,104 @@
 
 //==========================================================================
 //
-// Vertex buffer implementation
+// basic buffer implementation
 //
 //==========================================================================
 
-GLVertexBuffer::GLVertexBuffer()
+GLBuffer::GLBuffer(int usetype)
+	: mUseType(usetype)
 {
-	glGenBuffers(1, &vbo_id);
+	glGenBuffers(1, &mBufferId);
 }
 
-GLVertexBuffer::~GLVertexBuffer()
+GLBuffer::~GLBuffer()
 {
-	if (vbo_id != 0)
+	if (mBufferId != 0)
 	{
-		glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+		glBindBuffer(mUseType, mBufferId);
-		glUnmapBuffer(GL_ARRAY_BUFFER);
+		glUnmapBuffer(mUseType);
-		glBindBuffer(GL_ARRAY_BUFFER, 0);
+		glBindBuffer(mUseType, 0);
-		glDeleteBuffers(1, &vbo_id);
+		glDeleteBuffers(1, &mBufferId);
-		gl_RenderState.ResetVertexBuffer();
+		gl_RenderState.ResetVertexBuffer();	// force rebinding of buffers on next Apply call.
 	}
 }
 
-void GLVertexBuffer::SetData(size_t size, void *data, bool staticdata)
+void GLBuffer::Bind()
 {
+	glBindBuffer(mUseType, mBufferId);
+}
+
+
+void GLBuffer::SetData(size_t size, void *data, bool staticdata)
+{
+	assert(nomap);	// once it's mappable, it cannot be recreated anymore.
+	Bind();
 	if (data != nullptr)
 	{
-		glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+		glBufferData(mUseType, size, data, staticdata? GL_STATIC_DRAW : GL_STREAM_DRAW);
-		glBufferData(GL_ARRAY_BUFFER, size, data, staticdata? GL_STATIC_DRAW : GL_STREAM_DRAW);
 	}
 	else
 	{
 		mPersistent = screen->BuffersArePersistent() && !staticdata;
 		if (mPersistent)
 		{
-			glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+			glBufferStorage(mUseType, size, nullptr, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT);
-			glBufferStorage(GL_ARRAY_BUFFER, size, NULL, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT);
+			map = glMapBufferRange(mUseType, 0, size, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT);
-			map = glMapBufferRange(GL_ARRAY_BUFFER, 0, size, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_COHERENT_BIT);
 		}
 		else
 		{
-			glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+			glBufferData(mUseType, size, nullptr, staticdata ? GL_STATIC_DRAW : GL_STREAM_DRAW);
-			glBufferData(GL_ARRAY_BUFFER, size, NULL, staticdata ? GL_STATIC_DRAW : GL_STREAM_DRAW);
 			map = nullptr;
 		}
-		nomap = false;
+		if (!staticdata) nomap = false;
 	}
 	buffersize = size;
-	gl_RenderState.ResetVertexBuffer();	// This is needed because glBindBuffer overwrites the setting stored in the render state.
+	gl_RenderState.ResetVertexBuffer();	// force rebinding of buffers on next Apply call.
 }
 
-void GLVertexBuffer::Map()
+void GLBuffer::Map()
 {
-	assert(nomap == false);
+	assert(nomap == false);	// do not allow mapping of static buffers. Vulkan cannot do that so it should be blocked in OpenGL, too.
-	if (!mPersistent)
+	if (!mPersistent && !nomap)
 	{
-		glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+		Bind();
+		map = (FFlatVertex*)glMapBufferRange(mUseType, 0, buffersize, GL_MAP_WRITE_BIT|GL_MAP_UNSYNCHRONIZED_BIT);
 		gl_RenderState.ResetVertexBuffer();
-		map = (FFlatVertex*)glMapBufferRange(GL_ARRAY_BUFFER, 0, buffersize, GL_MAP_WRITE_BIT|GL_MAP_UNSYNCHRONIZED_BIT);
 	}
 }
 
-void GLVertexBuffer::Unmap()
+void GLBuffer::Unmap()
 {
 	assert(nomap == false);
-	if (!mPersistent)
+	if (!mPersistent && map != nullptr)
 	{
-		glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+		Bind();
+		glUnmapBuffer(mUseType);
 		gl_RenderState.ResetVertexBuffer();
-		glUnmapBuffer(GL_ARRAY_BUFFER);
 		map = nullptr;
 	}
 }
 
+void *GLBuffer::Lock(unsigned int size)
+{
+	// This initializes this buffer as a static object with no data.
+	SetData(size, nullptr, true);
+	return glMapBufferRange(mUseType, 0, size, GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT | GL_MAP_UNSYNCHRONIZED_BIT);
+}
+
+void GLBuffer::Unlock()
+{
+	Bind();
+	glUnmapBuffer(mUseType);
+	gl_RenderState.ResetVertexBuffer();
+}
+
+//===========================================================================
+//
+// Vertex buffer implementation
+//
+//===========================================================================
+
 void GLVertexBuffer::SetFormat(int numBindingPoints, int numAttributes, size_t stride, const FVertexBufferAttribute *attrs)
 {
 	static int VFmtToGLFmt[] = { GL_FLOAT, GL_FLOAT, GL_FLOAT, GL_FLOAT, GL_UNSIGNED_BYTE, GL_INT_2_10_10_10_REV };
@@ -118,6 +144,7 @@ void GLVertexBuffer::SetFormat(int numBindingPoints, int numAttributes, size_t s
 			attrinf.format = VFmtToGLFmt[attrs[i].format];
 			attrinf.size = VFmtToSize[attrs[i].format];
 			attrinf.offset = attrs[i].offset;
+			attrinf.bindingpoint = attrs[i].binding;
 		}
 	}
 }
@@ -126,7 +153,8 @@ void GLVertexBuffer::Bind(int *offsets)
 {
 	int i = 0;
 
-	glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
+	// This is what gets called from RenderState.Apply. It shouldn't be called anywhere else.
+	GLBuffer::Bind();
 	for(auto &attrinf : mAttributeInfo)
 	{
 		if (attrinf.size == 0)
@@ -136,95 +164,10 @@ void GLVertexBuffer::Bind(int *offsets)
 		else
 		{
 			glEnableVertexAttribArray(i);
-			size_t ofs = offsets == nullptr? attrinf.offset : attrinf.offset + mStride * offsets[attrinf.bindingpoint];
+			size_t ofs = offsets == nullptr ? attrinf.offset : attrinf.offset + mStride * offsets[attrinf.bindingpoint];
 			glVertexAttribPointer(i, attrinf.size, attrinf.format, attrinf.format != GL_FLOAT, (GLsizei)mStride, (void*)(intptr_t)ofs);
 		}
 		i++;
 	}
 }
 
-//===========================================================================
-//
-//
-//
-//===========================================================================
-
-void *GLVertexBuffer::Lock(unsigned int size)
-{
-	SetData(size, nullptr, true);
-	return glMapBufferRange(GL_ARRAY_BUFFER, 0, size, GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT | GL_MAP_UNSYNCHRONIZED_BIT);
-}
-
-//===========================================================================
-//
-//
-//
-//===========================================================================
-
-void GLVertexBuffer::Unlock()
-{
-	glBindBuffer(GL_ARRAY_BUFFER, vbo_id);
-	glUnmapBuffer(GL_ARRAY_BUFFER);
-	gl_RenderState.ResetVertexBuffer();
-}
-
-//==========================================================================
-//
-// Index buffer implementation
-//
-//==========================================================================
-
-GLIndexBuffer::GLIndexBuffer()
-{
-	glGenBuffers(1, &ibo_id);
-}
-
-GLIndexBuffer::~GLIndexBuffer()
-{
-	if (ibo_id != 0)
-	{
-		glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo_id);
-		glUnmapBuffer(GL_ELEMENT_ARRAY_BUFFER);
-		glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
-		glDeleteBuffers(1, &ibo_id);
-	}
-}
-
-void GLIndexBuffer::SetData(size_t size, void *data, bool staticdata)
-{
-	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo_id);
-	glBufferData(GL_ELEMENT_ARRAY_BUFFER, size, data, staticdata? GL_STATIC_DRAW : GL_STREAM_DRAW);
-	buffersize = size;
-	gl_RenderState.ResetVertexBuffer();	// This is needed because glBindBuffer overwrites the setting stored in the render state.
-}
-
-void GLIndexBuffer::Bind()
-{
-	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo_id);
-}
-
-//===========================================================================
-//
-//
-//
-//===========================================================================
-
-void *GLIndexBuffer::Lock(unsigned int size)
-{
-	SetData(size, nullptr, true);
-	return glMapBufferRange(GL_ELEMENT_ARRAY_BUFFER, 0, size, GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT | GL_MAP_UNSYNCHRONIZED_BIT);
-}
-
-//===========================================================================
-//
-//
-//
-//===========================================================================
-
-void GLIndexBuffer::Unlock()
-{
-	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo_id);
-	glUnmapBuffer(GL_ELEMENT_ARRAY_BUFFER);
-	gl_RenderState.ResetVertexBuffer();
-}
-

src/gl/system/glsys_vertexbuffer.h

@@ -2,7 +2,34 @@
 
 #include "hwrenderer/data/vertexbuffer.h"
 
-class GLVertexBuffer : public IVertexBuffer
+#ifdef _MSC_VER
+// silence bogus warning C4250: 'GLVertexBuffer': inherits 'GLBuffer::GLBuffer::SetData' via dominance
+// According to internet infos, the warning is erroneously emitted in this case.
+#pragma warning(disable:4250) 
+#endif
+
+class GLBuffer : virtual public IBuffer
+{
+	const int mUseType;
+	unsigned int mBufferId;
+protected:
+	int mAllocationSize = 0;
+	bool mPersistent = false;
+	bool nomap = true;
+
+	GLBuffer(int usetype);
+	~GLBuffer();
+	void SetData(size_t size, void *data, bool staticdata) override;
+	void Map() override;
+	void Unmap() override;
+	void *Lock(unsigned int size) override;
+	void Unlock() override;
+public:
+	void Bind();
+};
+
+
+class GLVertexBuffer : public IVertexBuffer, public GLBuffer
 {
 	// If this could use the modern (since GL 4.3) binding system, things would be simpler... :(
 	struct GLVertexBufferAttribute
@@ -12,35 +39,20 @@ class GLVertexBuffer : public IVertexBuffer
 		int size;
 		int offset;
 	};
-	
+
-	unsigned int vbo_id = 0;
 	int mNumBindingPoints;
-	bool mPersistent = false; 
 	GLVertexBufferAttribute mAttributeInfo[VATTR_MAX] = {};	// Thanks to OpenGL's state system this needs to contain info about every attribute that may ever be in use throughout the entire renderer.
 	size_t mStride = 0;
 
 public:
-	GLVertexBuffer();
+	GLVertexBuffer() : GLBuffer(GL_ARRAY_BUFFER) {}
-	~GLVertexBuffer();
-	void SetData(size_t size, void *data, bool staticdata) override;
 	void SetFormat(int numBindingPoints, int numAttributes, size_t stride, const FVertexBufferAttribute *attrs) override;
 	void Bind(int *offsets);
-	void Map() override;
-	void Unmap() override;
-	void *Lock(unsigned int size) override;
-	void Unlock() override;
 };
 
-class GLIndexBuffer : public IIndexBuffer
+class GLIndexBuffer : public IIndexBuffer, public GLBuffer
 {
-	unsigned int ibo_id = 0;
-
 public:
-	GLIndexBuffer();
+	GLIndexBuffer() : GLBuffer(GL_ELEMENT_ARRAY_BUFFER) {}
-	~GLIndexBuffer();
-	void SetData(size_t size, void *data, bool staticdata) override;
-	void Bind();
-	void *Lock(unsigned int size) override;
-	void Unlock() override;
 };
 

src/hwrenderer/data/vertexbuffer.h

@@ -33,35 +33,37 @@ struct FVertexBufferAttribute
 	int offset;
 };
 
-class IVertexBuffer
+class IBuffer
 {
 protected:
 	size_t buffersize = 0;
 	void *map = nullptr;
-	bool nomap = true;
 public:
-	virtual ~IVertexBuffer() {}
+	IBuffer() = default;
+	IBuffer(const IBuffer &) = delete;
+	IBuffer &operator=(const IBuffer &) = delete;
+	virtual ~IBuffer() = default;
+
 	virtual void SetData(size_t size, void *data, bool staticdata = true) = 0;
-	virtual void SetFormat(int numBindingPoints, int numAttributes, size_t stride, const FVertexBufferAttribute *attrs) = 0;
 	virtual void *Lock(unsigned int size) = 0;
 	virtual void Unlock() = 0;
 	virtual void Map() {}		// Only needed by old OpenGL but this needs to be in the interface.
 	virtual void Unmap() {}
 	void *Memory() { assert(map); return map; }
+	size_t Size() { return buffersize; }
 };
 
-
+class IVertexBuffer : virtual public IBuffer
-class IIndexBuffer
+{
+public:
+	virtual void SetFormat(int numBindingPoints, int numAttributes, size_t stride, const FVertexBufferAttribute *attrs) = 0;
+};
+
+// This merely exists to have a dedicated type for index buffers to inherit from.
+class IIndexBuffer : virtual public IBuffer
 {
-protected:
 	// Element size is fixed to 4, thanks to OpenGL requiring this info to be coded into the glDrawElements call.
 	// This mostly prohibits a more flexible buffer setup but GZDoom doesn't use any other format anyway.
 	// Ob Vulkam, element size is a buffer property and of no concern to the drawing functions (as it should be.)
-	size_t buffersize = 0;
-public:
-	virtual ~IIndexBuffer() {}
-	virtual void SetData(size_t size, void *data, bool staticdata = true) = 0;
-	virtual void *Lock(unsigned int size) = 0;
-	virtual void Unlock() = 0;
 };
 

src/hwrenderer/models/hw_models.cpp

@@ -126,7 +126,7 @@ void FGLModelRenderer::DrawArrays(int start, int count)
 
 void FGLModelRenderer::DrawElements(int numIndices, size_t offset)
 {
-	di->DrawIndexed(DT_Triangles, state, offset / sizeof(unsigned int), numIndices);
+	di->DrawIndexed(DT_Triangles, state, int(offset / sizeof(unsigned int)), numIndices);
 }
 
 //===========================================================================