Fix issue #495: Large spritebatches doesn't render properly

src/modules/graphics/opengl/SpriteBatch.cpp

@@ -43,8 +43,6 @@ SpriteBatch::SpriteBatch(Image *image, int size, int usage)
 	, array_buf(0)
 	, element_buf(0)
 {
-	image->retain();
-
 	GLenum gl_usage;
 
 	switch (usage)
@@ -61,33 +59,27 @@ SpriteBatch::SpriteBatch(Image *image, int size, int usage)
 		break;
 	}
 
-	int vertex_size = sizeof(vertex) * 4 * size;
-	int element_size = sizeof(GLushort) * 6 * size;
+	const size_t vertex_size = sizeof(vertex) * 4 * size;
 
-	array_buf = VertexBuffer::Create(vertex_size, GL_ARRAY_BUFFER, gl_usage);
-	element_buf = VertexBuffer::Create(element_size, GL_ELEMENT_ARRAY_BUFFER, GL_STATIC_DRAW);
-
-	// Fill element buffer.
+	try
 	{
-		VertexBuffer::Bind bind(*element_buf);
-		VertexBuffer::Mapper mapper(*element_buf);
-
-		GLushort *indices = static_cast<GLushort *>(mapper.get());
-
-		if (indices)
-		{
-			for (int i = 0; i < size; ++i)
-			{
-				indices[i*6+0] = 0+(i*4);
-				indices[i*6+1] = 1+(i*4);
-				indices[i*6+2] = 2+(i*4);
-
-				indices[i*6+3] = 0+(i*4);
-				indices[i*6+4] = 2+(i*4);
-				indices[i*6+5] = 3+(i*4);
-			}
-		}
+		array_buf = VertexBuffer::Create(vertex_size, GL_ARRAY_BUFFER, gl_usage);
+		element_buf = new VertexIndex(size);
 	}
+	catch (love::Exception &)
+	{
+		delete array_buf;
+		delete element_buf;
+		throw;
+	}
+	catch (std::bad_alloc &)
+	{
+		delete array_buf;
+		delete element_buf;
+		throw love::Exception("Out of memory.");
+	}
+
+	image->retain();
 }
 
 SpriteBatch::~SpriteBatch()
@@ -186,7 +178,7 @@ void SpriteBatch::setColor(const Color &color)
 	if (!this->color)
 		this->color = new Color(color);
 	else
-		 *(this->color) = color;
+		*(this->color) = color;
 }
 
 void SpriteBatch::setColor()
@@ -211,7 +203,7 @@ void SpriteBatch::draw(float x, float y, float angle, float sx, float sy, float
 	image->bind();
 
 	VertexBuffer::Bind array_bind(*array_buf);
-	VertexBuffer::Bind element_bind(*element_buf);
+	VertexBuffer::Bind element_bind(*element_buf->getVertexBuffer());
 
 	// Apply per-sprite color, if a color is set.
 	if (color)
@@ -226,7 +218,7 @@ void SpriteBatch::draw(float x, float y, float angle, float sx, float sy, float
 	glEnableClientState(GL_TEXTURE_COORD_ARRAY);
 	glTexCoordPointer(2, GL_FLOAT, sizeof(vertex), array_buf->getPointer(texel_offset));
 
-	glDrawElements(GL_TRIANGLES, next*6, GL_UNSIGNED_SHORT, element_buf->getPointer(0));
+	glDrawElements(GL_TRIANGLES, element_buf->getIndexCount(next), element_buf->getType(), element_buf->getPointer(0));
 
 	glDisableClientState(GL_VERTEX_ARRAY);
 	glDisableClientState(GL_TEXTURE_COORD_ARRAY);
@@ -269,7 +261,7 @@ bool SpriteBatch::getConstant(const char *in, UsageHint &out)
 	return usageHints.find(in, out);
 }
 
-bool SpriteBatch::getConstant(UsageHint in, const char  *&out)
+bool SpriteBatch::getConstant(UsageHint in, const char *&out)
 {
 	return usageHints.find(in, out);
 }

src/modules/graphics/opengl/SpriteBatch.h

@@ -48,6 +48,7 @@ namespace opengl
 class Image;
 class Quad;
 class VertexBuffer;
+class VertexIndex;
 
 class SpriteBatch : public Drawable
 {
@@ -93,7 +94,7 @@ public:
 	void draw(float x, float y, float angle, float sx, float sy, float ox, float oy, float kx, float ky) const;
 
 	static bool getConstant(const char *in, UsageHint &out);
-	static bool getConstant(UsageHint in, const char  *&out);
+	static bool getConstant(UsageHint in, const char *&out);
 
 private:
 
@@ -126,7 +127,8 @@ private:
 	Color *color;
 
 	VertexBuffer *array_buf;
-	VertexBuffer *element_buf;
+	VertexIndex *element_buf;
+
 }; // SpriteBatch
 
 } // opengl

src/modules/graphics/opengl/VertexBuffer.cpp

@@ -25,6 +25,7 @@
 
 #include <cstdlib>
 #include <cstring>
+#include <algorithm>
 
 namespace love
 {
@@ -237,6 +238,155 @@ void VBO::unload(bool save)
 	vbo = 0;
 }
 
+
+// VertexIndex
+
+size_t VertexIndex::maxSize = 0;
+std::list<size_t> VertexIndex::sizeRefs;
+VertexBuffer *VertexIndex::element_array = NULL;
+
+VertexIndex::VertexIndex(size_t size)
+	: size(size)
+{
+	// The upper limit is the maximum of GLuint divided by six (the number
+	// of indices per size) and divided by the size of GLuint. This guarantees
+	// no overflows when calculating the array size in bytes.
+	// Memory issues will be handled by other exceptions.
+	if (size == 0 || size > ((GLuint) -1) / 6 / sizeof(GLuint))
+		throw love::Exception("Invalid size.");
+
+	addSize(size);
+}
+
+VertexIndex::~VertexIndex()
+{
+	removeSize(size);
+}
+
+size_t VertexIndex::getSize() const
+{
+	return size;
+}
+
+size_t VertexIndex::getIndexCount(size_t elements) const
+{
+	return elements * 6;
+}
+
+GLenum VertexIndex::getType(size_t s) const
+{
+	// Calculates if unsigned short is big enough to hold all the vertex indices.
+	static const GLint type_table[] = {GL_UNSIGNED_INT, GL_UNSIGNED_SHORT};
+	return type_table[int(GLushort(-1) < s * 4)];
+}
+
+VertexBuffer *VertexIndex::getVertexBuffer() const
+{
+	return element_array;
+}
+
+const void *VertexIndex::getPointer(size_t offset) const
+{
+	return element_array->getPointer(offset);
+}
+
+void VertexIndex::addSize(size_t newSize)
+{
+	if (newSize <= maxSize)
+	{
+		// Current size is bigger. Append the size to list and sort.
+		sizeRefs.push_back(newSize);
+		sizeRefs.sort();
+		return;
+	}
+
+	// Try to resize before adding it to the list because resize may throw.
+	resize(newSize);
+	sizeRefs.push_back(newSize);
+}
+
+void VertexIndex::removeSize(size_t oldSize)
+{
+	// TODO: For debugging purposes, this should check if the size was actually found.
+	sizeRefs.erase(std::find(sizeRefs.begin(), sizeRefs.end(), oldSize));
+	if (sizeRefs.size() == 0)
+	{
+		resize(0);
+		return;
+	}
+
+	if (oldSize == maxSize)
+	{
+		// Shrink if there's a smaller size.
+		size_t newSize = sizeRefs.back();
+		if (newSize < maxSize)
+			resize(newSize);
+	}
+}
+
+void VertexIndex::resize(size_t size)
+{
+	if (size == 0)
+	{
+		delete element_array;
+		element_array = NULL;
+		maxSize = 0;
+		return;
+	}
+
+	VertexBuffer *new_element_array;
+	// Depending on the size, a switch to int and more memory is needed.
+	GLenum target_type = getType(size);
+	size_t array_size = (target_type == GL_UNSIGNED_SHORT ? sizeof(GLushort) : sizeof(GLuint)) * 6 * size;
+
+	// Create may throw out-of-memory exceptions.
+	// VertexIndex will propagate the exception and keep the old VertexBuffer.
+	try
+	{
+		new_element_array = VertexBuffer::Create(array_size, GL_ELEMENT_ARRAY_BUFFER, GL_STATIC_DRAW);
+	}
+	catch (std::bad_alloc &)
+	{
+		throw love::Exception("Out of memory.");
+	}
+
+	// Allocation of the new VertexBuffer succeeded.
+	// The old VertexBuffer can now be deleted.
+	delete element_array;
+	element_array = new_element_array;
+	maxSize = size;
+
+	switch (target_type)
+	{
+	case GL_UNSIGNED_SHORT:
+		fill<GLushort>();
+		break;
+	case GL_UNSIGNED_INT:
+		fill<GLuint>();
+		break;
+	}
+}
+
+template <typename T>
+void VertexIndex::fill()
+{
+	VertexBuffer::Bind bind(*element_array);
+	VertexBuffer::Mapper mapper(*element_array);
+
+	T *indices = (T *) mapper.get();
+
+	for (size_t i = 0; i < maxSize; ++i)
+	{
+		indices[i*6+0] = i * 4 + 0;
+		indices[i*6+1] = i * 4 + 1;
+		indices[i*6+2] = i * 4 + 2;
+
+		indices[i*6+3] = i * 4 + 0;
+		indices[i*6+4] = i * 4 + 2;
+		indices[i*6+5] = i * 4 + 3;
+	}
+}
+
 } // opengl
 } // graphics
 } // love

src/modules/graphics/opengl/VertexBuffer.h

@@ -337,6 +337,140 @@ private:
 	GLenum mapped_access;
 };
 
+/**
+ * VertexIndex manages one shared VertexBuffer that stores the indices for an
+ * element array. Vertex arrays using the vertex structure (or anything else
+ * that can use the pattern below) can request a size and use it for the
+ * drawElements call.
+ *
+ *  indices[i*6 + 0] = i*4 + 0;
+ *  indices[i*6 + 1] = i*4 + 1;
+ *  indices[i*6 + 2] = i*4 + 2;
+ *
+ *  indices[i*6 + 3] = i*4 + 0;
+ *  indices[i*6 + 4] = i*4 + 2;
+ *  indices[i*6 + 5] = i*4 + 3;
+ *
+ * There will always be a large enough VertexBuffer around until all
+ * VertexIndex instances have been deleted.
+ *
+ * Q: Why have something like VertexIndex?
+ * A: The indices for the SpriteBatch do not change, only the array size
+ * varies. Using one VertexBuffer for all element arrays removes this
+ * duplicated data and saves some memory.
+ */
+class VertexIndex
+{
+public:
+	/**
+	 * Adds an entry to the list of sizes and resizes the VertexBuffer
+	 * if needed. A size of 1 allocates a group of 6 indices for 4 vertices
+	 * creating 1 face.
+	 *
+	 * @param size The requested size in groups of 6 indices.
+	 */
+	VertexIndex(size_t size);
+
+	/**
+	 * Removes an entry from the list of sizes and resizes the VertexBuffer
+	 * if needed.
+	 */
+	~VertexIndex();
+
+	/**
+	 * Returns the number of index groups.
+	 * This can be used for getIndexCount to get the full count of indices.
+	 *
+	 * @return The number of index groups.
+	 */
+	size_t getSize() const;
+
+	/**
+	 * Returns the number of indices that the passed element count will have.
+	 * Use VertexIndex::getSize to get the full index count for that
+	 * VertexIndex instance.
+	 *
+	 * @param elements The number of elements to calculate the index count for.
+	 * @return The index count.
+	 */
+	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.
+	 *
+	 * @param s The size of the array to calculated the integer type of.
+	 * @return The element array integer type.
+	 */
+	GLenum getType(size_t s) const;
+	inline GLenum getType() const
+	{
+		return getType(maxSize);
+	}
+
+	/**
+	 * Returns the pointer to the VertexBuffer.
+	 * The pointer will change if a new size request or removal causes
+	 * a VertexBuffer resize. It is recommended to retrieve the pointer
+	 * value directly before the drawing call.
+	 *
+	 * @return The pointer to the VertexBuffer.
+	 */
+	VertexBuffer *getVertexBuffer() const;
+
+	/**
+	 * Returns a pointer which represents the specified byte offset.
+	 *
+	 * @param offset The offset in bytes.
+	 * @return A pointer which represents the offset.
+	 */
+	const void *getPointer(size_t offset) const;
+
+private:
+
+	/**
+	 * Adds a new size to the size list, then sorts and resizes it if needed.
+	 *
+	 * @param newSize The new size to be added.
+	 */
+	void addSize(size_t newSize);
+
+	/**
+	 * Removes a size from the size list, then sorts and resizes it if needed.
+	 *
+	 * @param oldSize The old size to be removed.
+	 */
+	void removeSize(size_t oldSize);
+
+	/**
+	 * Resizes the VertexBuffer to the requested size.
+	 * This function takes care of choosing the correct integer type and
+	 * allocating and deleting the VertexBuffer instance. It also has some
+	 * fallback logic in case the memory ran out.
+	 *
+	 * @param size The requested VertexBuffer size. Passing 0 deletes the VertexBuffer without allocating a new one.
+	 */
+	void resize(size_t size);
+
+	/**
+	 * 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 current VertexBuffer size. 0 means no VertexBuffer.
+	static size_t maxSize;
+	// The list of sizes. Needs to be kept sorted in ascending order.
+	static std::list<size_t> sizeRefs;
+	// The VertexBuffer for the element array. Can be NULL.
+	static VertexBuffer *element_array;
+};
+
 } // opengl
 } // graphics
 } // love