Refactor Buffer internals.

Allows buffers created with love.graphics.newBuffer to only allocate VRAM data. Previously they had a copy of their contents in RAM.
Also makes it easier to implement Buffers in other graphics APIs and to implement more types of Buffers in the future.

src/modules/graphics/opengl/Buffer.cpp

@@ -22,6 +22,7 @@
 
 #include "common/Exception.h"
 #include "graphics/vertex.h"
+#include "Graphics.h"
 
 #include <cstdlib>
 #include <cstring>
@@ -80,22 +81,26 @@ Buffer::Buffer(love::graphics::Graphics *gfx, const Settings &settings, const st
 
 	target = OpenGL::getGLBufferType(mapType);
 
-	try
+	if (usage == BUFFERUSAGE_STREAM)
+		ownsMemoryMap = true;
+
+	std::vector<uint8> emptydata;
+	if (settings.zeroInitialize && data == nullptr)
 	{
-		memoryMap = new char[size];
-	}
-	catch (std::bad_alloc &)
-	{
-		throw love::Exception("Out of memory.");
+		try
+		{
+			emptydata.resize(getSize());
+			data = emptydata.data();
+		}
+		catch (std::exception &)
+		{
+			data = nullptr;
+		}
 	}
 
-	if (data != nullptr)
-		memcpy(memoryMap, data, size);
-
-	if (!load(data != nullptr))
+	if (!load(data))
 	{
 		unloadVolatile();
-		delete[] memoryMap;
 		throw love::Exception("Could not create buffer (out of VRAM?)");
 	}
 }
@@ -103,7 +108,8 @@ Buffer::Buffer(love::graphics::Graphics *gfx, const Settings &settings, const st
 Buffer::~Buffer()
 {
 	unloadVolatile();
-	delete[] memoryMap;
+	if (memoryMap != nullptr && ownsMemoryMap)
+		free(memoryMap);
 }
 
 bool Buffer::loadVolatile()
@@ -111,7 +117,7 @@ bool Buffer::loadVolatile()
 	if (buffer != 0)
 		return true;
 
-	return load(true);
+	return load(nullptr);
 }
 
 void Buffer::unloadVolatile()
@@ -125,7 +131,7 @@ void Buffer::unloadVolatile()
 	texture = 0;
 }
 
-bool Buffer::load(bool restore)
+bool Buffer::load(const void *initialdata)
 {
 	while (glGetError() != GL_NO_ERROR)
 		/* Clear the error buffer. */;
@@ -133,11 +139,8 @@ bool Buffer::load(bool restore)
 	glGenBuffers(1, &buffer);
 	gl.bindBuffer(mapType, buffer);
 
-	// Copy the old buffer only if 'restore' was requested.
-	const GLvoid *src = restore ? memoryMap : nullptr;
-
-	// Note that if 'src' is '0', no data will be copied.
-	glBufferData(target, (GLsizeiptr) getSize(), src, OpenGL::getGLBufferUsage(getUsage()));
+	// initialdata can be null.
+	glBufferData(target, (GLsizeiptr) getSize(), initialdata, OpenGL::getGLBufferUsage(getUsage()));
 
 	if (getTypeFlags() & TYPEFLAG_TEXEL)
 	{
@@ -150,137 +153,104 @@ bool Buffer::load(bool restore)
 	return (glGetError() == GL_NO_ERROR);
 }
 
-void *Buffer::map()
-{
-	if (mapped)
-		return memoryMap;
-
-	mapped = true;
-
-	modifiedOffset = 0;
-	modifiedSize = 0;
-	isMappedDataModified = false;
-
-	return memoryMap;
-}
-
-void Buffer::unmapStatic(size_t offset, size_t size)
+void *Buffer::map(MapType /*map*/, size_t offset, size_t size)
 {
 	if (size == 0)
-		return;
+		return nullptr;
 
-	// Upload the mapped data to the buffer.
-	gl.bindBuffer(mapType, buffer);
-	glBufferSubData(target, (GLintptr) offset, (GLsizeiptr) size, memoryMap + offset);
-}
+	Range r(offset, size);
 
-void Buffer::unmapStream()
-{
-	GLenum glusage = OpenGL::getGLBufferUsage(getUsage());
+	if (!Range(0, getSize()).contains(r))
+		return nullptr;
 
-	// "orphan" current buffer to avoid implicit synchronisation on the GPU:
-	// http://www.seas.upenn.edu/~pcozzi/OpenGLInsights/OpenGLInsights-AsynchronousBufferTransfers.pdf
-	gl.bindBuffer(mapType, buffer);
-	glBufferData(target, (GLsizeiptr) getSize(), nullptr, glusage);
+	char *data = nullptr;
 
-#if LOVE_WINDOWS
-	// TODO: Verify that this codepath is a useful optimization.
-	if (gl.getVendor() == OpenGL::VENDOR_INTEL)
-		glBufferData(target, (GLsizeiptr) getSize(), memoryMap, glusage);
+	if (ownsMemoryMap)
+	{
+		if (memoryMap == nullptr)
+			memoryMap = (char *) malloc(getSize());
+		data = memoryMap;
+	}
 	else
-#endif
-		glBufferSubData(target, 0, (GLsizeiptr) getSize(), memoryMap);
+	{
+		auto gfx = Module::getInstance<Graphics>(Module::M_GRAPHICS);
+		data = (char *) gfx->getBufferMapMemory(size);
+	}
+
+	if (data != nullptr)
+	{
+		mapped = true;
+		mappedRange = r;
+		if (!ownsMemoryMap)
+			memoryMap = data;
+	}
+
+	return data;
 }
 
-void Buffer::unmap()
+void Buffer::unmap(size_t usedoffset, size_t usedsize)
 {
-	if (!mapped)
+	Range r(usedoffset, usedsize);
+
+	if (!mapped || !mappedRange.contains(r))
 		return;
 
 	mapped = false;
 
-	if ((mapFlags & MAP_EXPLICIT_RANGE_MODIFY) != 0)
+	// Orphan optimization - see fill().
+	if (usage != BUFFERUSAGE_STATIC && mappedRange.first == 0 && mappedRange.getSize() == getSize())
 	{
-		if (!isMappedDataModified)
-			return;
-
-		modifiedOffset = std::min(modifiedOffset, getSize() - 1);
-		modifiedSize = std::min(modifiedSize, getSize() - modifiedOffset);
-	}
-	else
-	{
-		modifiedOffset = 0;
-		modifiedSize = getSize();
+		usedoffset = 0;
+		usedsize = getSize();
 	}
 
-	if (modifiedSize > 0)
+	char *data = memoryMap + (usedoffset - mappedRange.getOffset());
+
+	fill(usedoffset, usedsize, data);
+
+	if (!ownsMemoryMap)
 	{
-		switch (getUsage())
-		{
-		case BUFFERUSAGE_STATIC:
-			unmapStatic(modifiedOffset, modifiedSize);
-			break;
-		case BUFFERUSAGE_STREAM:
-			unmapStream();
-			break;
-		case BUFFERUSAGE_DYNAMIC:
-		default:
-			// It's probably more efficient to treat it like a streaming buffer if
-			// at least a third of its contents have been modified during the map().
-			if (modifiedSize >= getSize() / 3)
-				unmapStream();
-			else
-				unmapStatic(modifiedOffset, modifiedSize);
-			break;
-		}
+		auto gfx = Module::getInstance<Graphics>(Module::M_GRAPHICS);
+		gfx->releaseBufferMapMemory(memoryMap);
+		memoryMap = nullptr;
 	}
-
-	modifiedOffset = 0;
-	modifiedSize = 0;
-}
-
-void Buffer::setMappedRangeModified(size_t offset, size_t modifiedsize)
-{
-	if (!mapped || !(mapFlags & MAP_EXPLICIT_RANGE_MODIFY))
-		return;
-
-	if (!isMappedDataModified)
-	{
-		modifiedOffset = offset;
-		modifiedSize = modifiedsize;
-		isMappedDataModified = true;
-		return;
-	}
-
-	// We're being conservative right now by internally marking the whole range
-	// from the start of section a to the end of section b as modified if both
-	// a and b are marked as modified.
-
-	size_t oldrangeend = modifiedOffset + modifiedSize;
-	modifiedOffset = std::min(modifiedOffset, offset);
-
-	size_t newrangeend = std::max(offset + modifiedsize, oldrangeend);
-	modifiedSize = newrangeend - modifiedOffset;
 }
 
 void Buffer::fill(size_t offset, size_t size, const void *data)
 {
-	memcpy(memoryMap + offset, data, size);
+	if (size == 0)
+		return;
 
-	if (mapped)
-		setMappedRangeModified(offset, size);
+	size_t buffersize = getSize();
+
+	if (!Range(0, buffersize).contains(Range(offset, size)))
+		return;
+
+	GLenum glusage = OpenGL::getGLBufferUsage(usage);
+
+	gl.bindBuffer(mapType, buffer);
+
+	if (usage != BUFFERUSAGE_STATIC && size == buffersize)
+	{
+		// "orphan" current buffer to avoid implicit synchronisation on the GPU:
+		// http://www.seas.upenn.edu/~pcozzi/OpenGLInsights/OpenGLInsights-AsynchronousBufferTransfers.pdf
+		gl.bindBuffer(mapType, buffer);
+		glBufferData(target, (GLsizeiptr) buffersize, nullptr, glusage);
+
+#if LOVE_WINDOWS
+		// TODO: Verify that this codepath is a useful optimization.
+		if (gl.getVendor() == OpenGL::VENDOR_INTEL)
+			glBufferData(target, (GLsizeiptr) buffersize, data, glusage);
+		else
+#endif
+			glBufferSubData(target, 0, (GLsizeiptr) buffersize, data);
+	}
 	else
 	{
-		gl.bindBuffer(mapType, buffer);
 		glBufferSubData(target, (GLintptr) offset, (GLsizeiptr) size, data);
 	}
 }
 
-void Buffer::copyTo(size_t offset, size_t size, love::graphics::Buffer *other, size_t otheroffset)
-{
-	other->fill(otheroffset, size, memoryMap + offset);
-}
-
 } // opengl
 } // graphics
 } // love

src/modules/graphics/opengl/Buffer.h

@@ -22,6 +22,7 @@
 
 // LOVE
 #include "common/config.h"
+#include "common/Range.h"
 #include "graphics/Buffer.h"
 #include "graphics/Volatile.h"
 
@@ -49,19 +50,16 @@ public:
 	bool loadVolatile() override;
 	void unloadVolatile() override;
 
-	void *map() override;
-	void unmap() override;
-	void setMappedRangeModified(size_t offset, size_t size) override;
+	void *map(MapType map, size_t offset, size_t size) override;
+	void unmap(size_t usedoffset, size_t usedsize) override;
 	void fill(size_t offset, size_t size, const void *data) override;
 
 	ptrdiff_t getHandle() const override { return buffer; };
 	ptrdiff_t getTexelBufferHandle() const override { return texture; };
 
-	void copyTo(size_t offset, size_t size, love::graphics::Buffer *other, size_t otheroffset) override;
-
 private:
 
-	bool load(bool restore);
+	bool load(const void *initialdata);
 
 	void unmapStatic(size_t offset, size_t size);
 	void unmapStream();
@@ -77,10 +75,9 @@ private:
 
 	// A pointer to mapped memory.
 	char *memoryMap = nullptr;
+	bool ownsMemoryMap = false;
 
-	size_t modifiedOffset = 0;
-	size_t modifiedSize = 0;
-	bool isMappedDataModified = false;
+	Range mappedRange;
 
 }; // Buffer
 

src/modules/graphics/opengl/Graphics.cpp

@@ -93,11 +93,22 @@ Graphics::Graphics()
 	, mainVAO(0)
 	, internalBackbufferFBO(0)
 	, requestedBackbufferMSAA(0)
+	, bufferMapMemory(nullptr)
+	, bufferMapMemorySize(2 * 1024 * 1024)
 	, defaultBuffers()
 	, supportedFormats()
 {
 	gl = OpenGL();
 
+	try
+	{
+		bufferMapMemory = new char[bufferMapMemorySize];
+	}
+	catch (std::exception &)
+	{
+		// Handled in getBufferMapMemory.
+	}
+
 	auto window = getInstance<love::window::Window>(M_WINDOW);
 
 	if (window != nullptr)
@@ -121,6 +132,7 @@ Graphics::Graphics()
 
 Graphics::~Graphics()
 {
+	delete[] bufferMapMemory;
 }
 
 const char *Graphics::getName() const
@@ -328,7 +340,7 @@ bool Graphics::setMode(int width, int height, int pixelwidth, int pixelheight, b
 
 	if (capabilities.features[FEATURE_TEXEL_BUFFER] && defaultBuffers[BUFFERTYPE_TEXEL].get() == nullptr)
 	{
-		Buffer::Settings settings(Buffer::TYPEFLAG_TEXEL, 0, BUFFERUSAGE_STATIC);
+		Buffer::Settings settings(Buffer::TYPEFLAG_TEXEL, BUFFERUSAGE_STATIC);
 		std::vector<Buffer::DataDeclaration> format = {{"", DATAFORMAT_FLOAT_VEC4, 0}};
 
 		const float texel[] = {0.0f, 0.0f, 0.0f, 1.0f};
@@ -1436,6 +1448,21 @@ void Graphics::setWireframe(bool enable)
 	states.back().wireframe = enable;
 }
 
+void *Graphics::getBufferMapMemory(size_t size)
+{
+	// We don't need anything more complicated because get/release calls are
+	// never interleaved (as of when this comment was written.)
+	if (bufferMapMemory == nullptr || size > bufferMapMemorySize)
+		return malloc(size);
+	return bufferMapMemory;
+}
+
+void Graphics::releaseBufferMapMemory(void *mem)
+{
+	if (mem != bufferMapMemory)
+		free(mem);
+}
+
 Graphics::Renderer Graphics::getRenderer() const
 {
 	return GLAD_ES_VERSION_2_0 ? RENDERER_OPENGLES : RENDERER_OPENGL;

src/modules/graphics/opengl/Graphics.h

@@ -112,6 +112,9 @@ public:
 	// Internal use.
 	void cleanupRenderTexture(love::graphics::Texture *texture);
 
+	void *getBufferMapMemory(size_t size);
+	void releaseBufferMapMemory(void *mem);
+
 private:
 
 	struct CachedFBOHasher
@@ -159,6 +162,9 @@ private:
 	GLuint internalBackbufferFBO;
 	int requestedBackbufferMSAA;
 
+	char *bufferMapMemory;
+	size_t bufferMapMemorySize;
+
 	// Only needed for buffer types that can be bound to shaders.
 	StrongRef<love::graphics::Buffer> defaultBuffers[BUFFERTYPE_MAX_ENUM];
 

src/modules/graphics/opengl/Shader.cpp

@@ -761,30 +761,8 @@ void Shader::sendBuffers(const UniformInfo *info, love::graphics::Buffer **buffe
 			buffer->retain();
 		}
 
-		bool addbuffertoarray = true;
-
 		if (info->buffers[i] != nullptr)
-		{
-			Buffer *oldbuffer = info->buffers[i];
-			auto it = std::find(buffersToUnmap.begin(), buffersToUnmap.end(), oldbuffer);
-			if (it != buffersToUnmap.end())
-			{
-				addbuffertoarray = false;
-				if (buffer != nullptr)
-					*it = buffer;
-				else
-				{
-					auto last = buffersToUnmap.end() - 1;
-					*it = *last;
-					buffersToUnmap.erase(last);
-				}
-			}
-
-			oldbuffer->release();
-		}
-
-		if (addbuffertoarray && buffer != nullptr)
-			buffersToUnmap.push_back(buffer);
+			info->buffers[i]->release();
 
 		info->buffers[i] = buffer;
 
@@ -916,13 +894,6 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
 	GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW];
 	if (location >= 0)
 		glUniform4fv(location, 13, (const GLfloat *) &data);
-
-	// TODO: Find a better place to put this.
-	// Buffers used in this shader can be mapped by external code without
-	// unmapping. We need to make sure the data on the GPU is up to date,
-	// otherwise the shader can read from old data.
-	for (Buffer *buffer : buffersToUnmap)
-		buffer->unmap();
 }
 
 int Shader::getUniformTypeComponents(GLenum type) const

src/modules/graphics/opengl/Shader.h

@@ -119,8 +119,6 @@ private:
 
 	std::vector<std::pair<const UniformInfo *, int>> pendingUniformUpdates;
 
-	std::vector<Buffer *> buffersToUnmap;
-
 	float lastPointSize;
 
 }; // Shader