Restructure some internal vertex buffer binding code to be closer to how graphics APIs actually behave.

src/modules/graphics/Graphics.cpp

@@ -1122,7 +1122,7 @@ void Graphics::flushStreamDraws()
 		return;
 
 	Attributes attributes;
-	Buffers buffers;
+	BufferBindings buffers;
 
 	size_t usedsizes[3] = {0, 0, 0};
 
@@ -1140,7 +1140,7 @@ void Graphics::flushStreamDraws()
 		sbstate.vbMap[i] = StreamBuffer::MapInfo();
 	}
 
-	if (attributes.enablebits == 0)
+	if (attributes.enableBits == 0)
 		return;
 
 	Colorf nc = getColor();

src/modules/graphics/Graphics.h

@@ -255,7 +255,7 @@ public:
 		PrimitiveType primitiveType = PRIMITIVE_TRIANGLES;
 
 		const vertex::Attributes *attributes;
-		const vertex::Buffers *buffers;
+		const vertex::BufferBindings *buffers;
 
 		int vertexStart = 0;
 		int vertexCount = 0;
@@ -266,7 +266,7 @@ public:
 		// TODO: This should be moved out to a state transition API?
 		CullMode cullMode = CULL_NONE;
 
-		DrawCommand(const vertex::Attributes *attribs, const vertex::Buffers *buffers)
+		DrawCommand(const vertex::Attributes *attribs, const vertex::BufferBindings *buffers)
 			: attributes(attribs)
 			, buffers(buffers)
 		{}
@@ -277,7 +277,7 @@ public:
 		PrimitiveType primitiveType = PRIMITIVE_TRIANGLES;
 
 		const vertex::Attributes *attributes;
-		const vertex::Buffers *buffers;
+		const vertex::BufferBindings *buffers;
 
 		int indexCount = 0;
 		int instanceCount = 1;
@@ -291,7 +291,7 @@ public:
 		// TODO: This should be moved out to a state transition API?
 		CullMode cullMode = CULL_NONE;
 
-		DrawIndexedCommand(const vertex::Attributes *attribs, const vertex::Buffers *buffers, Resource *indexbuffer)
+		DrawIndexedCommand(const vertex::Attributes *attribs, const vertex::BufferBindings *buffers, Resource *indexbuffer)
 			: attributes(attribs)
 			, buffers(buffers)
 			, indexBuffer(indexbuffer)
@@ -870,7 +870,7 @@ public:
 
 	virtual void draw(const DrawCommand &cmd) = 0;
 	virtual void draw(const DrawIndexedCommand &cmd) = 0;
-	virtual void drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::Buffers &buffers, Texture *texture) = 0;
+	virtual void drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::BufferBindings &buffers, Texture *texture) = 0;
 
 	void flushStreamDraws();
 	StreamVertexData requestStreamDraw(const StreamDrawCommand &command);

src/modules/graphics/Mesh.cpp

@@ -588,7 +588,7 @@ void Mesh::drawInstanced(Graphics *gfx, const Matrix4 &m, int instancecount)
 		Shader::current->checkMainTexture(texture);
 
 	vertex::Attributes attributes;
-	vertex::Buffers buffers;
+	vertex::BufferBindings buffers;
 
 	int activebuffers = 0;
 
@@ -619,7 +619,8 @@ void Mesh::drawInstanced(Graphics *gfx, const Matrix4 &m, int instancecount)
 			uint16 offset = (uint16) mesh->getAttributeOffset(attrib.second.index);
 			uint16 stride = (uint16) mesh->getVertexStride();
 
-			attributes.set(attributeindex, format.type, format.components, offset, stride, activebuffers, attrib.second.step);
+			attributes.set(attributeindex, format.type, (uint8) format.components, offset, activebuffers);
+			attributes.setBufferLayout(activebuffers, stride, attrib.second.step);
 
 			// TODO: Ideally we want to reuse buffers with the same stride+step.
 			buffers.set(activebuffers, mesh->vertexBuffer, 0);

src/modules/graphics/ParticleSystem.cpp

@@ -1080,7 +1080,7 @@ void ParticleSystem::draw(Graphics *gfx, const Matrix4 &m)
 
 	buffer->unmap();
 
-	vertex::Buffers vertexbuffers;
+	vertex::BufferBindings vertexbuffers;
 	vertexbuffers.set(0, buffer, 0);
 
 	gfx->drawQuads(0, pCount, vertexAttributes, vertexbuffers, texture);

src/modules/graphics/SpriteBatch.cpp

@@ -332,7 +332,7 @@ void SpriteBatch::draw(Graphics *gfx, const Matrix4 &m)
 	array_buf->unmap();
 
 	Attributes attributes;
-	Buffers buffers;
+	BufferBindings buffers;
 
 	{
 		buffers.set(0, array_buf, 0);
@@ -374,9 +374,10 @@ void SpriteBatch::draw(Graphics *gfx, const Matrix4 &m)
 			uint16 offset = (uint16) mesh->getAttributeOffset(it.second.index);
 			uint16 stride = (uint16) mesh->getVertexStride();
 
-			attributes.set(attributeindex, format.type, format.components, offset, stride, activebuffers);
+			attributes.set(attributeindex, format.type, (uint8) format.components, offset, activebuffers);
+			attributes.setBufferLayout(activebuffers, stride);
 
-			// TODO: Ideally we want to reuse buffers with the same stride+step.
+			// TODO: We should reuse buffer bindings with the same buffer+stride+step.
 			buffers.set(activebuffers, mesh->vertexBuffer, 0);
 			activebuffers++;
 		}

src/modules/graphics/Text.h

@@ -86,7 +86,7 @@ private:
 	StrongRef<Font> font;
 
 	vertex::Attributes vertexAttributes;
-	vertex::Buffers vertexBuffers;
+	vertex::BufferBindings vertexBuffers;
 
 	Buffer *vertex_buffer;
 

src/modules/graphics/opengl/Graphics.cpp

@@ -343,7 +343,7 @@ void Graphics::draw(const DrawIndexedCommand &cmd)
 	++drawCalls;
 }
 
-static inline void advanceVertexOffsets(const vertex::Attributes &attributes, vertex::Buffers &buffers, int vertexcount)
+static inline void advanceVertexOffsets(const vertex::Attributes &attributes, vertex::BufferBindings &buffers, int vertexcount)
 {
 	// TODO: Figure out a better way to avoid touching the same buffer multiple
 	// times, if multiple attributes share the buffer.
@@ -356,16 +356,17 @@ static inline void advanceVertexOffsets(const vertex::Attributes &attributes, ve
 
 		auto &attrib = attributes.attribs[i];
 
-		uint32 bufferbit = 1u << attrib.bufferindex;
+		uint32 bufferbit = 1u << attrib.bufferIndex;
 		if ((touchedbuffers & bufferbit) == 0)
 		{
 			touchedbuffers |= bufferbit;
-			buffers.info[attrib.bufferindex].offset += attrib.stride * vertexcount;
+			const auto &layout = attributes.bufferLayouts[attrib.bufferIndex];
+			buffers.info[attrib.bufferIndex].offset += layout.stride * vertexcount;
 		}
 	}
 }
 
-void Graphics::drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::Buffers &buffers, love::graphics::Texture *texture)
+void Graphics::drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::BufferBindings &buffers, love::graphics::Texture *texture)
 {
 	const int MAX_VERTICES_PER_DRAW = LOVE_UINT16_MAX;
 	const int MAX_QUADS_PER_DRAW    = MAX_VERTICES_PER_DRAW / 4;
@@ -394,7 +395,7 @@ void Graphics::drawQuads(int start, int count, const vertex::Attributes &attribu
 	}
 	else
 	{
-		vertex::Buffers bufferscopy = buffers;
+		vertex::BufferBindings bufferscopy = buffers;
 		if (start > 0)
 			advanceVertexOffsets(attributes, bufferscopy, start * 4);
 

src/modules/graphics/opengl/Graphics.h

@@ -73,7 +73,7 @@ public:
 
 	void draw(const DrawCommand &cmd) override;
 	void draw(const DrawIndexedCommand &cmd) override;
-	void drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::Buffers &buffers, Texture *texture) override;
+	void drawQuads(int start, int count, const vertex::Attributes &attributes, const vertex::BufferBindings &buffers, Texture *texture) override;
 
 	void clear(OptionalColorf color, OptionalInt stencil, OptionalDouble depth) override;
 	void clear(const std::vector<OptionalColorf> &colors, OptionalInt stencil, OptionalDouble depth) override;

src/modules/graphics/opengl/OpenGL.cpp

@@ -195,7 +195,7 @@ void OpenGL::setupContext()
 	else
 		state.instancedAttribArrays = 0;
 
-	setVertexAttributes(vertex::Attributes(), vertex::Buffers());
+	setVertexAttributes(vertex::Attributes(), vertex::BufferBindings());
 
 	// Get the current viewport.
 	glGetIntegerv(GL_VIEWPORT, (GLint *) &state.viewport.x);
@@ -697,49 +697,60 @@ void OpenGL::deleteBuffer(GLuint buffer)
 	}
 }
 
-void OpenGL::setVertexAttributes(const vertex::Attributes &attributes, const vertex::Buffers &buffers)
+void OpenGL::setVertexAttributes(const vertex::Attributes &attributes, const vertex::BufferBindings &buffers)
 {
-	uint32 enablediff = attributes.enablebits ^ state.enabledAttribArrays;
-	uint32 instancediff = attributes.instancebits ^ state.instancedAttribArrays;
+	uint32 enablediff = attributes.enableBits ^ state.enabledAttribArrays;
+	uint32 instanceattribbits = 0;
+	uint32 allbits = attributes.enableBits | state.enabledAttribArrays;
 
-	for (uint32 i = 0; i < vertex::Attributes::MAX; i++)
+	uint32 i = 0;
+	while (allbits)
 	{
 		uint32 bit = 1u << i;
 
 		if (enablediff & bit)
 		{
-			if (attributes.enablebits & bit)
+			if (attributes.enableBits & bit)
 				glEnableVertexAttribArray(i);
 			else
 				glDisableVertexAttribArray(i);
 		}
 
-		if (instancediff & bit)
-			glVertexAttribDivisor(i, (attributes.instancebits & bit) != 0 ? 1 : 0);
-
-		if (attributes.enablebits & bit)
+		if (attributes.enableBits & bit)
 		{
 			const auto &attrib = attributes.attribs[i];
-			const auto &bufferinfo = buffers.info[attrib.bufferindex];
+			const auto &layout = attributes.bufferLayouts[attrib.bufferIndex];
+			const auto &bufferinfo = buffers.info[attrib.bufferIndex];
+
+			uint32 bufferbit = 1u << attrib.bufferIndex;
+			uint32 divisor = (attributes.instanceBits & bufferbit) != 0 ? 1 : 0;
+			uint32 divisorbit = divisor << i;
+			instanceattribbits |= divisorbit;
+
+			if ((state.enabledAttribArrays & bit) ^ divisorbit)
+				glVertexAttribDivisor(i, divisor);
 
 			GLboolean normalized = GL_FALSE;
 			GLenum gltype = getGLVertexDataType(attrib.type, normalized);
 
-			const void *offsetpointer = reinterpret_cast<void*>(bufferinfo.offset + attrib.offsetfromvertex);
+			const void *offsetpointer = reinterpret_cast<void*>(bufferinfo.offset + attrib.offsetFromVertex);
 
 			bindBuffer(BUFFER_VERTEX, (GLuint) bufferinfo.buffer->getHandle());
-			glVertexAttribPointer(i, attrib.components, gltype, normalized, attrib.stride, offsetpointer);
+			glVertexAttribPointer(i, attrib.components, gltype, normalized, layout.stride, offsetpointer);
 		}
+
+		i++;
+		allbits >>= 1;
 	}
 
-	state.enabledAttribArrays = attributes.enablebits;
-	state.instancedAttribArrays = attributes.instancebits;
+	state.enabledAttribArrays = attributes.enableBits;
+	state.instancedAttribArrays = instanceattribbits | (state.instancedAttribArrays & (~attributes.enableBits));
 
 	// glDisableVertexAttribArray will make the constant value for a vertex
 	// attribute undefined. We rely on the per-vertex color attribute being
 	// white when no per-vertex color is used, so we set it here.
 	// FIXME: Is there a better place to do this?
-	if ((enablediff & ATTRIBFLAG_COLOR) && !(attributes.enablebits & ATTRIBFLAG_COLOR))
+	if ((enablediff & ATTRIBFLAG_COLOR) && !(attributes.enableBits & ATTRIBFLAG_COLOR))
 		glVertexAttrib4f(ATTRIB_COLOR, 1.0f, 1.0f, 1.0f, 1.0f);
 }
 

src/modules/graphics/opengl/OpenGL.h

@@ -237,7 +237,7 @@ public:
 	/**
 	 * Set all vertex attribute state.
 	 **/
-	void setVertexAttributes(const vertex::Attributes &attributes, const vertex::Buffers &buffers);
+	void setVertexAttributes(const vertex::Attributes &attributes, const vertex::BufferBindings &buffers);
 
 	/**
 	 * Wrapper for glCullFace which eliminates redundant state setting.

src/modules/graphics/vertex.cpp

@@ -228,51 +228,51 @@ void fillIndices(TriangleIndexMode mode, uint32 vertexStart, uint32 vertexCount,
 
 void Attributes::setCommonFormat(CommonFormat format, uint8 bufferindex)
 {
-	uint16 stride = (uint16) getFormatStride(format);
+	setBufferLayout(bufferindex, (uint16) getFormatStride(format));
 
 	switch (format)
 	{
 	case CommonFormat::NONE:
 		break;
 	case CommonFormat::XYf:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
 		break;
 	case CommonFormat::XYZf:
-		set(ATTRIB_POS, DATA_FLOAT, 3, 0, stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 3, 0, bufferindex);
 		break;
 	case CommonFormat::RGBAub:
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, 0, stride, bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, 0, bufferindex);
 		break;
 	case CommonFormat::STf_RGBAub:
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 2), stride, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 2), bufferindex);
 		break;
 	case CommonFormat::STPf_RGBAub:
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, 0, stride, bufferindex);
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 3), stride, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, 0, bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 3), bufferindex);
 		break;
 	case CommonFormat::XYf_STf:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, uint16(sizeof(float) * 2), stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, uint16(sizeof(float) * 2), bufferindex);
 		break;
 	case CommonFormat::XYf_STPf:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, uint16(sizeof(float) * 2), stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, uint16(sizeof(float) * 2), bufferindex);
 		break;
 	case CommonFormat::XYf_STf_RGBAub:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, uint16(sizeof(float) * 2), stride, bufferindex);
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 4), stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 2, uint16(sizeof(float) * 2), bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 4), bufferindex);
 		break;
 	case CommonFormat::XYf_STus_RGBAub:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_TEXCOORD, DATA_UNORM16, 2, uint16(sizeof(float) * 2), stride, bufferindex);
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 2 + sizeof(uint16) * 2), stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_UNORM16, 2, uint16(sizeof(float) * 2), bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 2 + sizeof(uint16) * 2), bufferindex);
 		break;
 	case CommonFormat::XYf_STPf_RGBAub:
-		set(ATTRIB_POS, DATA_FLOAT, 2, 0, stride, bufferindex);
-		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, uint16(sizeof(float) * 2), stride, bufferindex);
-		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 5), stride, bufferindex);
+		set(ATTRIB_POS, DATA_FLOAT, 2, 0, bufferindex);
+		set(ATTRIB_TEXCOORD, DATA_FLOAT, 3, uint16(sizeof(float) * 2), bufferindex);
+		set(ATTRIB_COLOR, DATA_UNORM8, 4, uint16(sizeof(float) * 5), bufferindex);
 		break;
 	}
 }

src/modules/graphics/vertex.h

@@ -36,7 +36,7 @@ namespace graphics
 
 class Resource;
 
-// Vertex attribute indices used in shaders by LOVE. The values map to OpenGL
+// Vertex attribute indices used in shaders by LOVE. The values map to GPU
 // generic vertex attribute indices.
 enum VertexAttribID
 {
@@ -189,11 +189,11 @@ struct XYf_STPf_RGBAub
 	Color color;
 };
 
-struct Buffers
+struct BufferBindings
 {
-	static const unsigned int MAX = 32;
+	static const uint32 MAX = 32;
 
-	uint32 usebits = 0;
+	uint32 useBits = 0;
 
 	struct
 	{
@@ -201,33 +201,40 @@ struct Buffers
 		size_t offset;
 	} info[MAX];
 
-	void set(unsigned int index, Resource *r, size_t offset)
+	void set(uint32 index, Resource *r, size_t offset)
 	{
-		usebits |= (1u << index);
+		useBits |= (1u << index);
 		info[index] = {r, offset};
 	}
 
-	void disable(unsigned int index) { usebits &= (1u << index); }
-	void clear() { usebits = 0; }
+	void disable(uint32 index) { useBits &= (1u << index); }
+	void clear() { useBits = 0; }
 };
 
 struct AttributeInfo
 {
-	uint8 bufferindex;
-	DataType type : 8;
-	uint16 components;
-	uint16 offsetfromvertex;
+	uint8 bufferIndex;
+	DataType type : 4;
+	uint8 components : 4;
+	uint16 offsetFromVertex;
+};
+
+static_assert(sizeof(AttributeInfo) == 4, "AttributeInfo struct size should be 4 bytes");
+
+struct BufferLayout
+{
 	uint16 stride;
 };
 
 struct Attributes
 {
-	static const unsigned int MAX = 32;
+	static const uint32 MAX = 32;
 
-	uint32 enablebits = 0;
-	uint32 instancebits = 0;
+	uint32 enableBits = 0; // indexed by attribute
+	uint32 instanceBits = 0; // indexed by buffer
 
 	AttributeInfo attribs[MAX];
+	BufferLayout bufferLayouts[BufferBindings::MAX];
 
 	Attributes() {}
 	Attributes(CommonFormat format, uint8 bufferindex)
@@ -235,42 +242,46 @@ struct Attributes
 		setCommonFormat(format, bufferindex);
 	}
 
-	void set(unsigned int index, DataType type, uint16 components, uint16 offsetfromvertex, uint16 stride, uint8 bufferindex, AttributeStep step = STEP_PER_VERTEX)
+	void set(uint32 index, DataType type, uint8 components, uint16 offsetfromvertex, uint8 bufferindex)
 	{
-		uint32 bit = (1u << index);
+		enableBits |= (1u << index);
 
-		enablebits |= bit;
+		attribs[index].bufferIndex = bufferindex;
+		attribs[index].type = type;
+		attribs[index].components = components;
+		attribs[index].offsetFromVertex = offsetfromvertex;
+	}
+
+	void setBufferLayout(uint32 bufferindex, uint16 stride, AttributeStep step = STEP_PER_VERTEX)
+	{
+		uint32 bufferbit = (1u << bufferindex);
 
 		if (step == STEP_PER_INSTANCE)
-			instancebits |= bit;
+			instanceBits |= bufferbit;
 		else
-			instancebits &= ~bit;
+			instanceBits &= ~bufferbit;
 
-		attribs[index].bufferindex = bufferindex;
-		attribs[index].type = type;
-		attribs[index].components = components;
-		attribs[index].offsetfromvertex = offsetfromvertex;
-		attribs[index].stride = stride;
+		bufferLayouts[bufferindex].stride = stride;
 	}
 
-	void disable(unsigned int index)
+	void disable(uint32 index)
 	{
-		enablebits &= ~(1u << index);
+		enableBits &= ~(1u << index);
 	}
 
 	void clear()
 	{
-		enablebits = 0;
+		enableBits = 0;
 	}
 
-	bool isEnabled(unsigned int index) const
+	bool isEnabled(uint32 index) const
 	{
-		return (enablebits & (1u << index)) != 0;
+		return (enableBits & (1u << index)) != 0;
 	}
 
-	AttributeStep getStep(unsigned int index) const
+	AttributeStep getBufferStep(uint32 index) const
 	{
-		return (instancebits & (1u << index)) != 0 ? STEP_PER_INSTANCE : STEP_PER_VERTEX;
+		return (instanceBits & (1u << index)) != 0 ? STEP_PER_INSTANCE : STEP_PER_VERTEX;
 	}
 
 	void setCommonFormat(CommonFormat format, uint8 bufferindex);