Always use a 16 bit index array for quad indices, with multiple draws when necessary. See issue #1356.

src/modules/graphics/Buffer.cpp

@@ -42,82 +42,35 @@ Buffer::~Buffer()
 
 // QuadIndices
 
-size_t QuadIndices::maxSize = 0;
-size_t QuadIndices::elementSize = 0;
+static const int MAX_VERTICES_PER_DRAW = LOVE_UINT16_MAX;
+static const int MAX_QUADS_PER_DRAW    = MAX_VERTICES_PER_DRAW / 4;
+
 size_t QuadIndices::objectCount = 0;
 
 Buffer *QuadIndices::indexBuffer = nullptr;
-char *QuadIndices::indices = nullptr;
 
-QuadIndices::QuadIndices(Graphics *gfx, size_t size)
-	: size(size)
+QuadIndices::QuadIndices(Graphics *gfx)
 {
-	// The upper limit is the maximum of uint32 divided by six (the number
-	// of indices per size) and divided by the size of uint32. This guarantees
-	// no overflows when calculating the array size in bytes.
-	if (size == 0 || size > ((uint32) -1) / 6 / sizeof(uint32))
-		throw love::Exception("Invalid number of quads.");
-
-	// Create a new / larger buffer if needed.
-	if (indexBuffer == nullptr || size > maxSize)
+	if (indexBuffer == nullptr)
 	{
-		Buffer *newbuffer = nullptr;
-		char *newindices = nullptr;
+		size_t buffersize = sizeof(uint16) * MAX_QUADS_PER_DRAW * 6;
 
-		// Depending on the size, a switch to int and more memory is needed.
-		IndexDataType targettype = getType(size);
-		size_t elemsize = vertex::getIndexDataSize(targettype);
+		indexBuffer = gfx->newBuffer(buffersize, nullptr, BUFFER_INDEX, vertex::USAGE_STATIC, 0);
 
-		size_t buffersize = elemsize * 6 * size;
-
-		try
-		{
-			newbuffer = gfx->newBuffer(buffersize, nullptr, BUFFER_INDEX, vertex::USAGE_STATIC, 0);
-			newindices = new char[buffersize];
-		}
-		catch (std::bad_alloc &)
-		{
-			delete newbuffer;
-			delete[] newindices;
-			throw love::Exception("Out of memory.");
-		}
-
-		// Allocation of the new Buffer succeeded.
-		// The old Buffer can now be deleted.
-		delete indexBuffer;
-		indexBuffer = newbuffer;
-
-		delete[] indices;
-		indices = newindices;
-
-		maxSize = size;
-		elementSize = elemsize;
-
-		switch (targettype)
-		{
-		case INDEX_UINT16:
-			fill<uint16>();
-			break;
-		case INDEX_UINT32:
-			fill<uint32>();
-			break;
-		case INDEX_MAX_ENUM:
-			break;
-		}
+		Buffer::Mapper map(*indexBuffer);
+		vertex::fillIndices(vertex::TriangleIndexMode::QUADS, 0, MAX_VERTICES_PER_DRAW, (uint16 *) map.get());
 	}
 
 	objectCount++;
 }
 
-QuadIndices::QuadIndices(const QuadIndices &other)
-: size(other.size)
+QuadIndices::QuadIndices(const QuadIndices &)
 {
 	objectCount++;
 }
 
-QuadIndices &QuadIndices::operator = (const QuadIndices &other)
+QuadIndices &QuadIndices::operator = (const QuadIndices &)
 {
-	size = other.size;
 	return *this;
 }
 
@@ -130,30 +83,54 @@ QuadIndices::~QuadIndices()
 	{
 		delete indexBuffer;
 		indexBuffer = nullptr;
-
-		delete[] indices;
-		indices = nullptr;
 	}
 }
 
-size_t QuadIndices::getSize() const
+static inline void advanceVertexOffsets(const vertex::Attributes &attributes, vertex::Buffers &buffers, int vertexcount)
 {
-	return size;
+	// TODO: Figure out a better way to avoid touching the same buffer multiple
+	// times, if multiple attributes share the buffer.
+	uint32 touchedbuffers = 0;
+
+	for (unsigned int i = 0; i < vertex::Attributes::MAX; i++)
+	{
+		if (!attributes.isEnabled(i))
+			continue;
+
+		auto &attrib = attributes.attribs[i];
+
+		uint32 bufferbit = 1u << attrib.bufferindex;
+		if ((touchedbuffers & bufferbit) == 0)
+		{
+			touchedbuffers |= bufferbit;
+			buffers.info[attrib.bufferindex].offset += attrib.stride * vertexcount;
+		}
+	}
 }
 
-size_t QuadIndices::getIndexCount(size_t elements) const
+void QuadIndices::draw(Graphics *gfx, int quadstart, int quadcount, const vertex::Attributes &attributes, vertex::Buffers buffers, Texture *texture)
 {
-	return elements * 6;
-}
+	Graphics::DrawIndexedCommand cmd(&attributes, &buffers, indexBuffer);
+	cmd.primitiveType = PRIMITIVE_TRIANGLES;
+	cmd.indexBufferOffset = 0;
+	cmd.indexType = INDEX_UINT16;
+	cmd.texture = texture;
 
-IndexDataType QuadIndices::getType(size_t s) const
-{
-	return vertex::getIndexDataTypeFromMax(getIndexCount(s));
-}
+	// TODO: We can use glDrawElementsBaseVertex when supported, instead of
+	// advancing the vertex offset.
+	if (quadstart > 0)
+		advanceVertexOffsets(attributes, buffers, quadstart * 4);
 
-size_t QuadIndices::getElementSize() const
-{
-	return elementSize;
+	for (int quadindex = 0; quadindex < quadcount; quadindex += MAX_QUADS_PER_DRAW)
+	{
+		int quaddrawcount = std::min(MAX_QUADS_PER_DRAW, quadcount - quadindex);
+
+		if (quadindex > 0)
+			advanceVertexOffsets(attributes, buffers, quaddrawcount * 4);
+
+		cmd.indexCount = quaddrawcount * 6;
+		gfx->draw(cmd);
+	}
 }
 
 Buffer *QuadIndices::getBuffer() const
@@ -161,17 +138,5 @@ Buffer *QuadIndices::getBuffer() const
 	return indexBuffer;
 }
 
-const void *QuadIndices::getIndices(size_t offset) const
-{
-	return indices + offset;
-}
-
-template <typename T>
-void QuadIndices::fill()
-{
-	vertex::fillIndices(vertex::TriangleIndexMode::QUADS, 0, maxSize * 4, (T *) indices);
-	indexBuffer->fill(0, indexBuffer->getSize(), indices);
-}
-
 } // graphics
 } // love

src/modules/graphics/Buffer.h

@@ -35,6 +35,7 @@ namespace graphics
 {
 
 class Graphics;
+class Texture;
 
 /**
  * A block of GPU-owned memory. Currently meant for internal use.
@@ -52,25 +53,13 @@ public:
 	Buffer(size_t size, BufferType type, vertex::Usage usage, uint32 mapflags);
 	virtual ~Buffer();
 
-	size_t getSize() const
-	{
-		return size;
-	}
+	size_t getSize() const { return size; }
 
-	BufferType getType() const
-	{
-		return type;
-	}
+	BufferType getType() const { return type; }
 
-	vertex::Usage getUsage() const
-	{
-		return usage;
-	}
+	vertex::Usage getUsage() const { return usage; }
 
-	bool isMapped() const
-	{
-		return is_mapped;
-	}
+	bool isMapped() const { return is_mapped; }
 
 	/**
 	 * Map the Buffer to client memory.
@@ -106,10 +95,7 @@ public:
 	 **/
 	virtual void copyTo(size_t offset, size_t size, Buffer *other, size_t otheroffset) = 0;
 
-	uint32 getMapFlags() const
-	{
-		return map_flags;
-	}
+	uint32 getMapFlags() const { return map_flags; }
 
 	class Mapper
 	{
@@ -119,7 +105,7 @@ public:
 		 * Memory-maps a Buffer.
 		 */
 		Mapper(Buffer &buffer)
-		: buf(buffer)
+			: buf(buffer)
 		{
 			elems = buf.map();
 		}
@@ -189,12 +175,13 @@ protected:
 class QuadIndices
 {
 public:
+
 	/**
 	 * Adds an entry to the list of sizes and resizes the Buffer
 	 * if needed. A size of 1 allocates a group of 6 indices for 4 vertices
 	 * creating 1 face.
 	 */
-	QuadIndices(Graphics *gfx, size_t size);
+	QuadIndices(Graphics *gfx);
 
 	QuadIndices(const QuadIndices &other);
 	QuadIndices &operator = (const QuadIndices &other);
@@ -205,81 +192,13 @@ public:
 	 */
 	~QuadIndices();
 
-	/**
-	 * Returns the number of index groups.
-	 * This can be used for getIndexCount to get the full count of indices.
-	 */
-	size_t getSize() const;
-
-	/**
-	 * Returns the number of indices that the passed element count will have.
-	 * Use QuadIndices::getSize to get the full index count for that
-	 * QuadIndices instance.
-	 */
-	size_t getIndexCount(size_t elements) const;
-
-	/**
-	 * Returns the integer type of the element array.
-	 * If an optional nonzero size argument is passed, the function returns
-	 * the integer type of the element array of that size.
-	 */
-	IndexDataType getType(size_t s) const;
-	inline IndexDataType getType() const
-	{
-		return getType(maxSize);
-	}
-
-	/**
-	 * Returns the size in bytes of an element in the element array.
-	 * Can be used with getPointer to calculate an offset into the array based
-	 * on a number of elements.
-	 **/
-	size_t getElementSize() const;
-
-	/**
-	 * Returns the pointer to the Buffer.
-	 * The pointer will change if a new size request or removal causes a Buffer
-	 * resize. It is recommended to retrieve the pointer value directly before
-	 * the drawing call.
-	 */
+	void draw(Graphics *gfx, int quadstart, int quadcount, const vertex::Attributes &attributes, vertex::Buffers buffers, Texture *texture);
 	Buffer *getBuffer() const;
 
-	/**
-	 * Returns a direct pointer to the index data.
-	 *
-	 * At least one graphics driver (the one for Kepler nvidia GPUs in OS X)
-	 * fails to render geometry if the vertex data was a direct CPU pointer but
-	 * the index data came from an Index Buffer.
-	 * So the direct pointer to the index buffer should be used instead of the
-	 * index buffer when rendering using client-side vertex arrays.
-	 **/
-	const void *getIndices(size_t offset) const;
-
 private:
 
-	/**
-	 * Adds all indices to the array with the type T.
-	 * There are no checks for the correct types or overflows. The calling
-	 * function should check for that.
-	 */
-	template <typename T> void fill();
-
-	// The size of the array requested by this instance.
-	size_t size;
-
-	// The size in bytes of an element in the element array.
-	static size_t elementSize;
-
-	// The current GLBuffer size. 0 means no Buffer.
-	static size_t maxSize;
-
 	static size_t objectCount;
-
-	// The Buffer for the element array. Can be null.
 	static Buffer *indexBuffer;
-	
-	// The array of indices that will also be stored in the index buffer.
-	static char *indices;
 
 }; // QuadIndices
 

src/modules/graphics/Graphics.cpp

@@ -440,21 +440,15 @@ void Graphics::restoreStateChecked(const DisplayState &s)
 	{
 		for (size_t i = 0; i < sRTs.colors.size() && i < curRTs.colors.size(); i++)
 		{
-			const auto &rt1 = sRTs.colors[i];
-			const auto &rt2 = curRTs.colors[i];
-			if (rt1.canvas.get() != rt2.canvas.get() || rt1.slice != rt2.slice || rt1.mipmap != rt2.mipmap)
+			if (sRTs.colors[i] != curRTs.colors[i])
 			{
 				canvaseschanged = true;
 				break;
 			}
 		}
 
-		if (!canvaseschanged && (sRTs.depthStencil.canvas.get() != curRTs.depthStencil.canvas.get()
-			|| sRTs.depthStencil.slice != curRTs.depthStencil.slice
-			|| sRTs.depthStencil.mipmap != curRTs.depthStencil.mipmap))
-		{
+		if (!canvaseschanged && sRTs.depthStencil != curRTs.depthStencil)
 			canvaseschanged = true;
-		}
 
 		if (sRTs.temporaryRTFlags != curRTs.temporaryRTFlags)
 			canvaseschanged = true;
@@ -577,21 +571,15 @@ void Graphics::setCanvas(const RenderTargets &rts)
 
 		for (int i = 0; i < ncanvases; i++)
 		{
-			if (rts.colors[i].canvas != prevRTs.colors[i].canvas.get()
-				|| rts.colors[i].slice != prevRTs.colors[i].slice
-				|| rts.colors[i].mipmap != prevRTs.colors[i].mipmap)
+			if (rts.colors[i] != prevRTs.colors[i])
 			{
 				modified = true;
 				break;
 			}
 		}
 
-		if (!modified && (rts.depthStencil.canvas != prevRTs.depthStencil.canvas
-						  || rts.depthStencil.slice != prevRTs.depthStencil.slice
-						  || rts.depthStencil.mipmap != prevRTs.depthStencil.mipmap))
-		{
+		if (!modified && rts.depthStencil != prevRTs.depthStencil)
 			modified = true;
-		}
 
 		if (rts.temporaryRTFlags != prevRTs.temporaryRTFlags)
 			modified = true;

src/modules/graphics/Graphics.h

@@ -354,6 +354,8 @@ public:
 		void *data = nullptr;
 	};
 
+	struct RenderTargetStrongRef;
+
 	struct RenderTarget
 	{
 		Canvas *canvas;
@@ -370,6 +372,16 @@ public:
 			: canvas(nullptr)
 			, slice(0)
 		{}
+
+		bool operator != (const RenderTarget &other) const
+		{
+			return canvas != other.canvas || slice != other.slice || mipmap != other.mipmap;
+		}
+
+		bool operator != (const RenderTargetStrongRef &other) const
+		{
+			return canvas != other.canvas.get() || slice != other.slice || mipmap != other.mipmap;
+		}
 	};
 
 	struct RenderTargetStrongRef
@@ -383,6 +395,16 @@ public:
 			, slice(slice)
 			, mipmap(mipmap)
 		{}
+
+		bool operator != (const RenderTargetStrongRef &other) const
+		{
+			return canvas.get() != other.canvas.get() || slice != other.slice || mipmap != other.mipmap;
+		}
+
+		bool operator != (const RenderTarget &other) const
+		{
+			return canvas.get() != other.canvas || slice != other.slice || mipmap != other.mipmap;
+		}
 	};
 
 	struct RenderTargets

src/modules/graphics/ParticleSystem.cpp

@@ -95,7 +95,7 @@ ParticleSystem::ParticleSystem(Graphics *gfx, Texture *texture, uint32 size)
 	, relativeRotation(false)
 	, vertexAttributes(vertex::CommonFormat::XYf_STf_RGBAub, 0)
 	, buffer(nullptr)
-	, quadIndices(gfx, size)
+	, quadIndices(gfx)
 {
 	if (size == 0 || size > MAX_PARTICLES)
 		throw love::Exception("Invalid ParticleSystem size.");
@@ -195,7 +195,7 @@ void ParticleSystem::createBuffers(size_t size)
 		size_t bytes = sizeof(Vertex) * size * 4;
 		buffer = gfx->newBuffer(bytes, nullptr, BUFFER_VERTEX, vertex::USAGE_STREAM, 0);
 
-		quadIndices = QuadIndices(gfx, size);
+		quadIndices = QuadIndices(gfx);
 	}
 	catch (std::bad_alloc &)
 	{
@@ -1094,20 +1094,14 @@ void ParticleSystem::draw(Graphics *gfx, const Matrix4 &m)
 		p = p->next;
 	}
 
+	Graphics::TempTransform transform(gfx, m);
+
 	buffer->unmap();
 
 	vertex::Buffers vertexbuffers;
 	vertexbuffers.set(0, buffer, 0);
 
-	Graphics::TempTransform transform(gfx, m);
-
-	Graphics::DrawIndexedCommand cmd(&vertexAttributes, &vertexbuffers, quadIndices.getBuffer());
-	cmd.primitiveType = PRIMITIVE_TRIANGLES;
-	cmd.indexBufferOffset = 0;
-	cmd.indexCount = (int) quadIndices.getIndexCount(pCount);
-	cmd.indexType = quadIndices.getType();
-	cmd.texture = texture;
-	gfx->draw(cmd);
+	quadIndices.draw(gfx, 0, pCount, vertexAttributes, vertexbuffers, texture);
 }
 
 bool ParticleSystem::getConstant(const char *in, AreaSpreadDistribution &out)

src/modules/graphics/SpriteBatch.cpp

@@ -47,7 +47,7 @@ SpriteBatch::SpriteBatch(Graphics *gfx, Texture *texture, int size, vertex::Usag
 	, color(255, 255, 255, 255)
 	, color_active(false)
 	, array_buf(nullptr)
-	, quad_indices(gfx, size)
+	, quad_indices(gfx)
 	, range_start(-1)
 	, range_count(-1)
 {
@@ -238,8 +238,6 @@ void SpriteBatch::setBufferSize(int newsize)
 		// Copy as much of the old data into the new GLBuffer as can fit.
 		size_t copy_size = vertex_stride * 4 * new_next;
 		array_buf->copyTo(0, copy_size, new_array_buf, 0);
-
-		quad_indices = QuadIndices(gfx, newsize);
 	}
 	catch (love::Exception &)
 	{
@@ -385,7 +383,7 @@ void SpriteBatch::draw(Graphics *gfx, const Matrix4 &m)
 		}
 	}
 
-	Graphics::DrawIndexedCommand cmd(&attributes, &buffers, quad_indices.getBuffer());
+	Graphics::TempTransform transform(gfx, m);
 
 	int start = std::min(std::max(0, range_start), next - 1);
 
@@ -395,15 +393,8 @@ void SpriteBatch::draw(Graphics *gfx, const Matrix4 &m)
 
 	count = std::min(count, next - start);
 
-	cmd.indexBufferOffset = quad_indices.getIndexCount(start) * quad_indices.getElementSize();
-	cmd.indexCount = (int) quad_indices.getIndexCount(count);
-	cmd.indexType = quad_indices.getType();
-	cmd.texture = texture;
-
-	Graphics::TempTransform transform(gfx, m);
-
 	if (count > 0)
-		gfx->draw(cmd);
+		quad_indices.draw(gfx, start, count, attributes, buffers, texture);
 }
 
 } // graphics

src/modules/graphics/Text.cpp

@@ -34,7 +34,7 @@ Text::Text(Graphics *gfx, Font *font, const std::vector<Font::ColoredString> &te
 	: font(font)
 	, vertexAttributes(Font::vertexFormat, 0)
 	, vbo(nullptr)
-	, quadIndices(gfx, 20)
+	, quadIndices(gfx)
 	, vert_offset(0)
 	, texture_cache_id((uint32) -1)
 {
@@ -259,24 +259,12 @@ void Text::draw(Graphics *gfx, const Matrix4 &m)
 	for (const Font::DrawCommand &cmd : draw_commands)
 		totalverts = std::max(cmd.startvertex + cmd.vertexcount, totalverts);
 
-	if ((size_t) totalverts / 4 > quadIndices.getSize())
-		quadIndices = QuadIndices(gfx, (size_t) totalverts / 4);
-
 	vbo->unmap(); // Make sure all pending data is flushed to the GPU.
 
 	Graphics::TempTransform transform(gfx, m);
 
-	Graphics::DrawIndexedCommand drawcmd(&vertexAttributes, &vertexBuffers, quadIndices.getBuffer());
-	drawcmd.indexType = quadIndices.getType();
-	size_t elemsize = quadIndices.getElementSize();
-
 	for (const Font::DrawCommand &cmd : draw_commands)
-	{
-		drawcmd.indexCount = (cmd.vertexcount / 4) * 6;
-		drawcmd.indexBufferOffset = (cmd.startvertex / 4) * 6 * elemsize;
-		drawcmd.texture = cmd.texture;
-		gfx->draw(drawcmd);
-	}
+		quadIndices.draw(gfx, cmd.startvertex / 4, cmd.vertexcount / 4, vertexAttributes, vertexBuffers, cmd.texture);
 }
 
 } // graphics

src/modules/graphics/opengl/Graphics.cpp

@@ -232,7 +232,7 @@ bool Graphics::setMode(int width, int height, int pixelwidth, int pixelheight, b
 	// index buffer objects, since the shared index buffer used by QuadIndices
 	// objects is destroyed when the last object is destroyed.
 	if (quadIndices == nullptr)
-		quadIndices = new QuadIndices(this, 20);
+		quadIndices = new QuadIndices(this);
 
 	// Restore the graphics state.
 	restoreState(states.back());
@@ -543,7 +543,6 @@ void Graphics::endPass()
 			if (mask != 0)
 				glBlitFramebuffer(0, 0, w, h, 0, 0, w, h, mask, GL_NEAREST);
 		}
-
 	}
 
 	for (const auto &rt : rts.colors)