metal: fix uniform data corruption

The wrong buffer binding offset was being used in 2/3rds of frames.
slime73 · Mar 10, 2024 · 9e00ce0 · 9e00ce0
1 parent 75fd2fa
commit 9e00ce0
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 5 deletions.
diff --git a/src/modules/graphics/StreamBuffer.h b/src/modules/graphics/StreamBuffer.h
@@ -57,6 +57,8 @@ class StreamBuffer : public love::Object, public Resource
 	BufferUsage getMode() const { return mode; }
 	size_t getUsableSize() const { return bufferSize - frameGPUReadOffset; }
 
+	virtual size_t getGPUReadOffset() const = 0;
+
 	virtual MapInfo map(size_t minsize) = 0;
 	virtual size_t unmap(size_t usedsize) = 0;
 	virtual void markUsed(size_t usedsize) = 0;

diff --git a/src/modules/graphics/metal/Graphics.h b/src/modules/graphics/metal/Graphics.h
@@ -242,6 +242,7 @@ class Graphics final : public love::graphics::Graphics
 	StreamBuffer *uniformBuffer;
 	StreamBuffer::MapInfo uniformBufferData;
 	size_t uniformBufferOffset;
+	size_t uniformBufferGPUStart;
 
 	Buffer *defaultAttributesBuffer;
 

diff --git a/src/modules/graphics/metal/Graphics.mm b/src/modules/graphics/metal/Graphics.mm
@@ -280,6 +280,7 @@ static inline void setSampler(id<MTLComputeCommandEncoder> encoder, Graphics::Re
 	, attachmentStoreActions()
 	, renderBindings()
 	, uniformBufferOffset(0)
+	, uniformBufferGPUStart(0)
 	, defaultAttributesBuffer(nullptr)
 	, families()
 { @autoreleasepool {
@@ -1035,22 +1036,27 @@ static bool isClampOne(SamplerState::WrapMode w)
 	if (uniformBuffer->getSize() < uniformBufferOffset + size)
 	{
 		size_t newsize = uniformBuffer->getSize() * 2;
+		if (uniformBufferOffset > 0)
+			uniformBuffer->nextFrame();
 		uniformBuffer->release();
-		uniformBuffer = CreateStreamBuffer(device, BUFFERUSAGE_VERTEX, newsize);
+		uniformBuffer = CreateStreamBuffer(device, BUFFERUSAGE_UNIFORM, newsize);
 		uniformBufferData = {};
 		uniformBufferOffset = 0;
 	}
 
 	if (uniformBufferData.data == nullptr)
+	{
 		uniformBufferData = uniformBuffer->map(uniformBuffer->getSize());
+		uniformBufferGPUStart = uniformBuffer->getGPUReadOffset();
+	}
 
 	memcpy(uniformBufferData.data + uniformBufferOffset, bufferdata, size);
 
 	id<MTLBuffer> buffer = getMTLBuffer(uniformBuffer);
 	int uniformindex = Shader::getUniformBufferBinding();
 
 	auto &bindings = renderBindings;
-	setBuffer(encoder, bindings, uniformindex, buffer, uniformBufferOffset);
+	setBuffer(encoder, bindings, uniformindex, buffer, uniformBufferGPUStart + uniformBufferOffset);
 
 	uniformBufferOffset += alignUp(size, alignment);
 
@@ -1141,23 +1147,28 @@ static bool isClampOne(SamplerState::WrapMode w)
 	if (uniformBuffer->getSize() < uniformBufferOffset + size)
 	{
 		size_t newsize = uniformBuffer->getSize() * 2;
+		if (uniformBufferOffset > 0)
+			uniformBuffer->nextFrame();
 		uniformBuffer->release();
-		uniformBuffer = CreateStreamBuffer(device, BUFFERUSAGE_VERTEX, newsize);
+		uniformBuffer = CreateStreamBuffer(device, BUFFERUSAGE_UNIFORM, newsize);
 		uniformBufferData = {};
 		uniformBufferOffset = 0;
 	}
 
 	if (uniformBufferData.data == nullptr)
+	{
 		uniformBufferData = uniformBuffer->map(uniformBuffer->getSize());
+		uniformBufferGPUStart = uniformBuffer->getGPUReadOffset();
+	}
 
 	memcpy(uniformBufferData.data + uniformBufferOffset, bufferdata, size);
 
 	id<MTLBuffer> buffer = getMTLBuffer(uniformBuffer);
 	int uniformindex = Shader::getUniformBufferBinding();
 
 	auto &bindings = renderBindings;
-	setBuffer(renderEncoder, bindings, SHADERSTAGE_VERTEX, uniformindex, buffer, uniformBufferOffset);
-	setBuffer(renderEncoder, bindings, SHADERSTAGE_PIXEL, uniformindex, buffer, uniformBufferOffset);
+	setBuffer(renderEncoder, bindings, SHADERSTAGE_VERTEX, uniformindex, buffer, uniformBufferGPUStart + uniformBufferOffset);
+	setBuffer(renderEncoder, bindings, SHADERSTAGE_PIXEL, uniformindex, buffer, uniformBufferGPUStart + uniformBufferOffset);
 
 	uniformBufferOffset += alignUp(size, alignment);
 

diff --git a/src/modules/graphics/metal/StreamBuffer.mm b/src/modules/graphics/metal/StreamBuffer.mm
@@ -67,6 +67,11 @@
 		}
 	}}
 
+	size_t getGPUReadOffset() const override
+	{
+		return (frameIndex * bufferSize) + frameGPUReadOffset;
+	}
+
 	MapInfo map(size_t /*minsize*/) override
 	{
 		// Make sure this frame's section of the buffer is done being used.

diff --git a/src/modules/graphics/opengl/StreamBuffer.cpp b/src/modules/graphics/opengl/StreamBuffer.cpp
@@ -63,6 +63,11 @@ class StreamBufferClientMemory final : public love::graphics::StreamBuffer
 		delete[] data;
 	}
 
+	size_t getGPUReadOffset() const override
+	{
+		return (size_t) data;
+	}
+
 	MapInfo map(size_t /*minsize*/) override
 	{
 		return MapInfo(data, bufferSize);
@@ -111,6 +116,11 @@ class StreamBufferSubDataOrphan final : public love::graphics::StreamBuffer, pub
 		delete[] data;
 	}
 
+	size_t getGPUReadOffset() const override
+	{
+		return frameGPUReadOffset;
+	}
+
 	MapInfo map(size_t /*minsize*/) override
 	{
 		if (orphan)
@@ -192,6 +202,11 @@ class StreamBufferSync : public love::graphics::StreamBuffer
 
 	virtual ~StreamBufferSync() {}
 
+	size_t getGPUReadOffset() const override
+	{
+		return (frameIndex * bufferSize) + frameGPUReadOffset;
+	}
+
 	void nextFrame() override
 	{
 		// Insert a GPU fence for this frame's section of the data, we'll wait

diff --git a/src/modules/graphics/vulkan/StreamBuffer.cpp b/src/modules/graphics/vulkan/StreamBuffer.cpp
@@ -96,6 +96,11 @@ ptrdiff_t StreamBuffer::getHandle() const
 	return (ptrdiff_t) buffer;
 }
 
+size_t getGPUReadOffset() const override
+{
+	return (frameIndex * bufferSize) + frameGPUReadOffset;
+}
+
 love::graphics::StreamBuffer::MapInfo StreamBuffer::map(size_t /*minsize*/)
 {
 	// TODO: do we also need to wait until a fence is complete, here?