From 8abf09afe2b5d2b1bd2cf9c7be7d956ba66c2011 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 28 Oct 2018 08:58:41 +0100 Subject: [PATCH] - 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 | 175 +++++++++------------------ src/gl/system/glsys_vertexbuffer.h | 52 +++++--- src/hwrenderer/data/vertexbuffer.h | 28 +++-- src/hwrenderer/models/hw_models.cpp | 2 +- 4 files changed, 107 insertions(+), 150 deletions(-) diff --git a/src/gl/system/glsys_vertexbuffer.cpp b/src/gl/system/glsys_vertexbuffer.cpp index 2a37d4ceba..c68cad69ae 100644 --- a/src/gl/system/glsys_vertexbuffer.cpp +++ b/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); - glUnmapBuffer(GL_ARRAY_BUFFER); - glBindBuffer(GL_ARRAY_BUFFER, 0); - glDeleteBuffers(1, &vbo_id); - gl_RenderState.ResetVertexBuffer(); + glBindBuffer(mUseType, mBufferId); + glUnmapBuffer(mUseType); + glBindBuffer(mUseType, 0); + glDeleteBuffers(1, &mBufferId); + 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(GL_ARRAY_BUFFER, size, data, staticdata? GL_STATIC_DRAW : GL_STREAM_DRAW); + glBufferData(mUseType, size, data, staticdata? GL_STATIC_DRAW : GL_STREAM_DRAW); } else { mPersistent = screen->BuffersArePersistent() && !staticdata; if (mPersistent) { - glBindBuffer(GL_ARRAY_BUFFER, vbo_id); - glBufferStorage(GL_ARRAY_BUFFER, size, NULL, 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); + glBufferStorage(mUseType, size, nullptr, 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); } else { - glBindBuffer(GL_ARRAY_BUFFER, vbo_id); - glBufferData(GL_ARRAY_BUFFER, size, NULL, staticdata ? GL_STATIC_DRAW : GL_STREAM_DRAW); + glBufferData(mUseType, size, nullptr, 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); - if (!mPersistent) + assert(nomap == false); // do not allow mapping of static buffers. Vulkan cannot do that so it should be blocked in OpenGL, too. + 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(); -} - diff --git a/src/gl/system/glsys_vertexbuffer.h b/src/gl/system/glsys_vertexbuffer.h index c3c836142c..7085bafc50 100644 --- a/src/gl/system/glsys_vertexbuffer.h +++ b/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(); - void SetData(size_t size, void *data, bool staticdata) override; + GLVertexBuffer() : GLBuffer(GL_ARRAY_BUFFER) {} 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(); - void SetData(size_t size, void *data, bool staticdata) override; - void Bind(); - void *Lock(unsigned int size) override; - void Unlock() override; + GLIndexBuffer() : GLBuffer(GL_ELEMENT_ARRAY_BUFFER) {} }; diff --git a/src/hwrenderer/data/vertexbuffer.h b/src/hwrenderer/data/vertexbuffer.h index d16c3cdbc6..ae3883d173 100644 --- a/src/hwrenderer/data/vertexbuffer.h +++ b/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 IIndexBuffer +class IVertexBuffer : virtual public IBuffer +{ +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; }; diff --git a/src/hwrenderer/models/hw_models.cpp b/src/hwrenderer/models/hw_models.cpp index b6df1ee206..13952cbfc5 100644 --- a/src/hwrenderer/models/hw_models.cpp +++ b/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); } //===========================================================================