Hello there,

I have tried following various OpenGL tutorials and I'm now at a point where I can render multiple 2d sprites with textures.

For that I have a sprite class:

Header:

#ifndef SPRITE_H
#define SPRITE_H

#include <GL/glew.h>
#include "Shader.h"
#include <glm/glm.hpp>
#include <glm/gtc/matrix_transform.hpp>
#include <glm/gtc/type_ptr.hpp>
#include "Texture.h"
#include <stb_image.h>
#include "Camera.h"





class Sprite
{
public:
	Sprite(GLfloat x, GLfloat y, GLfloat width, GLfloat height, Shader& shader, Texture *texture);

	~Sprite();
	void draw(Camera &camera);
	void setPosition(float x, float y, float z);
	void move(float x, float y, float z);

	void setTexture(Texture *texture);
	Texture getTexture();
	float x, y, width, height;


private:
	void init(GLfloat x, GLfloat y, GLfloat width, GLfloat height, Shader& shader, Texture *texture);
	GLuint VBO = 0, VAO = 0, EBO = 0;
	GLint transformShaderLocation, viewShaderLocation, projectionShaderLocation;
	Shader* shader;
	glm::mat4 transform, projection, view;
	Texture *texture;


};
#endif

Code:

#include "Sprite.h"


Sprite::Sprite(GLfloat x, GLfloat y, GLfloat width, GLfloat height, Shader& shader, Texture *texture)
{
	init(x, y, width, height, shader, texture);
}


void Sprite::init(GLfloat x, GLfloat y, GLfloat width, GLfloat height, Shader& shader, Texture *texture)
{
	this->shader = &shader;
	this->x = x;
	this->y = y;
	this->width = width;
	this->height = height;

	GLfloat vertices[] = {
	width / 2 ,  height / 2,  0.0f,  /* Top Right */	1.0f, 1.0f,
    width / 2 , -height / 2 , 0.0f,  /* Bottom Right*/	1.0f, 0.0f,
	-width / 2 ,-height / 2 , 0.0f,  /* Bottom Left	*/	0.0f, 0.0f,
	-width / 2 , height / 2 , 0.0f,   /* Top Left */	0.0f, 1.0f
	};
	GLuint indices[] = {
		0, 1, 3,  // 1
		1, 2, 3   // 2
	};
	glGenVertexArrays(1, &VAO);
	glGenBuffers(1, &VBO);
	glGenBuffers(1, &EBO);

	glBindVertexArray(VAO);

	glBindBuffer(GL_ARRAY_BUFFER, VBO);
	glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
	glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(indices), indices, GL_STATIC_DRAW);

	//Position
	glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 5 * sizeof(GLfloat), (GLvoid*)0);
	glEnableVertexAttribArray(0);
	// TexCoord
	glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 5 * sizeof(GLfloat), (GLvoid*)(3 * sizeof(GLfloat)));
	glEnableVertexAttribArray(1);

	glBindBuffer(GL_ARRAY_BUFFER, 0);
	glBindVertexArray(0);
	transformShaderLocation = glGetUniformLocation(shader.program, "transform");
	viewShaderLocation = glGetUniformLocation(shader.program, "view");
	projectionShaderLocation = glGetUniformLocation(shader.program, "projection");
	transform = glm::translate(transform, glm::vec3(x , y , 0));
	this->texture = texture;


}


Sprite::~Sprite()
{
	//DELETE BUFFERS
	glDeleteBuffers(1, &VBO);
	glDeleteBuffers(1, &EBO);
	glDeleteBuffers(1, &VAO);
	delete texture;
}


void Sprite::draw(Camera &camera) {



	shader->Use();

	glBindTexture(GL_TEXTURE_2D, texture->texture);



	view = camera.getView();
	projection = camera.getProjection();

	// Pass to shaders
	glUniformMatrix4fv(transformShaderLocation, 1, GL_FALSE, glm::value_ptr(transform));
	glUniformMatrix4fv(viewShaderLocation, 1, GL_FALSE, glm::value_ptr(view));
	// Note: currently we set the projection matrix each frame, but since the projection matrix rarely changes it's often best practice to set it outside the main loop only once.
	glUniformMatrix4fv(projectionShaderLocation, 1, GL_FALSE, glm::value_ptr(projection));




	glBindVertexArray(VAO);
	glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, 0);
	glBindVertexArray(0);
}


void Sprite::setPosition(float x, float y, float z) { //Z?


	transform = glm::translate(transform, glm::vec3(x - this->x , y - this->y , z));
	this->x = x;
	this->y = y;
}

void Sprite::move(float x, float y, float z) {
	transform = glm::translate(transform, glm::vec3(x, y , z));
	this->x += x;
	this->y += y;

}

void Sprite::setTexture(Texture *texture) {
	delete this->texture;
	this->texture = texture;
}
Texture Sprite::getTexture()
{
	return *texture;
}

When I want to draw something, I create an instance of the sprite class with it's own Texture and use sprite->draw(); in the draw loop for each sprite to draw it. This works perfectly fine.

To improve the performance, I now want to create a spritebatch.

As far as I understood it puts all the sprites together so it can send them all at once to the gpu.

I had no clue how to get started, so I just created a spritebatch class which put all the vertices and indices into one object every time draw() is called, and actually only draws when flush() is called. Here's the header file:

#ifndef SPRITEBATCH_H
#define SPRITEBATCH_H

#include <glm/glm.hpp>
#include "Texture.h"
#include <GL/glew.h>
#include "Camera.h"
#include "Shader.h"
#include <vector>

class SpriteBatch
{
public:
	SpriteBatch(Shader& shader, Camera &camera);
	~SpriteBatch();

	void draw(Texture *texture, GLfloat x, GLfloat y, GLfloat width, GLfloat height);
	void flush();
private:
	GLfloat vertices[800];
	GLuint indices[800];
	int index{ 0 };
	int indicesIndex{ 0 };
	GLuint VBO = 0, VAO = 0, EBO = 0;
	GLint transformShaderLocation, viewShaderLocation, projectionShaderLocation;
	Shader *shader;
	Camera *camera;
	std::vector<Texture*>* textures;
	glm::mat4 transform, projection, view;
};
#endif

And the class. I added some comments here:

#include "SpriteBatch.h"



SpriteBatch::SpriteBatch(Shader& shader, Camera &camera)
{
	this->shader = &shader;
	this->camera = &camera;
	textures = new std::vector<Texture*>();
}


SpriteBatch::~SpriteBatch()
{
	glDeleteBuffers(1, &VBO);
	glDeleteBuffers(1, &EBO);
	glDeleteBuffers(1, &VAO);
	//delete texture;
}

void SpriteBatch::draw(Texture *texture, GLfloat x, GLfloat y, GLfloat width, GLfloat height)
{

	textures->push_back(texture);

	vertices[index] = width/2 ;
	vertices[index + 1] = height/2;
	vertices[index + 2] = 0.0f;
	vertices[index + 3] = 1.0f;
	vertices[index + 4] = 1.0f;

	vertices[index + 5] = width / 2;
	vertices[index + 6] = -height / 2;
	vertices[index + 7] = 0.0f;
	vertices[index + 8] = 1.0f;
	vertices[index + 9] = 0.0f;

	vertices[index + 10] = -width / 2;
	vertices[index + 11] = -height / 2;
	vertices[index + 12] = 0.0f;
	vertices[index + 13] = 0.0f;
	vertices[index + 14] = 0.0f;

	vertices[index + 15] = -width / 2;
	vertices[index + 16] = height / 2;
	vertices[index + 17] = 0.0f;
	vertices[index + 18] = 0.0f;
	vertices[index + 19] = 1.0f;
	index += 20;


	indices[indicesIndex] = 0;
	indices[indicesIndex + 1] = 1;
	indices[indicesIndex + 2] = 3;
	indices[indicesIndex + 3] = 1;
	indices[indicesIndex + 4] = 2;
	indices[indicesIndex + 5] = 3;
	indicesIndex += 6;

}

void SpriteBatch::flush()
{
	if (index == 0) return; //Ensures that there are sprites added
	//Debug information. This works perfectly
	int spritesInBatch = index / 20;
	std::cout << spritesInBatch << " I : " << index << std::endl;
	
	int drawn = 0;


	//Create Buffers
	glGenVertexArrays(1, &VAO);
	glGenBuffers(1, &VBO);
	glGenBuffers(1, &EBO);

	glBindVertexArray(VAO);

	//Bind vertices
	glBindBuffer(GL_ARRAY_BUFFER, VBO);
	glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

	//Bind indices
	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
	glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(indices), indices, GL_STATIC_DRAW);

	//Position
	glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 5 * sizeof(GLfloat), (GLvoid*)0);
	glEnableVertexAttribArray(0);
	// TexCoord
	glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 5 * sizeof(GLfloat), (GLvoid*)(3 * sizeof(GLfloat)));
	glEnableVertexAttribArray(1);
	//VAO
	glBindBuffer(GL_ARRAY_BUFFER, 0);
	glBindVertexArray(0);
	//Shader locations
	transformShaderLocation = glGetUniformLocation(shader->program, "transform");
	viewShaderLocation = glGetUniformLocation(shader->program, "view");
	projectionShaderLocation = glGetUniformLocation(shader->program, "projection");


	//Draw

	
	//So this sets the texture for each sprites and draws it afterwards with the right texture. At least that's how it should work.
	for (int i = 0; i < spritesInBatch; i++) {
		Texture *tex = textures->at(i);
		shader->Use();

		glBindTexture(GL_TEXTURE_2D, tex->texture); //?
		view = camera->getView();

		projection = camera->getProjection();

		// Pass them to the shaders
		glUniformMatrix4fv(transformShaderLocation, 1, GL_FALSE, glm::value_ptr(transform));
		glUniformMatrix4fv(viewShaderLocation, 1, GL_FALSE, glm::value_ptr(view));
		glUniformMatrix4fv(projectionShaderLocation, 1, GL_FALSE, glm::value_ptr(projection));



		//Draw VAO
		glBindVertexArray(VAO);
		glDrawElements(GL_TRIANGLES, indicesIndex, GL_UNSIGNED_INT, 0);
		glBindVertexArray(0);
	}
	//Sets index 0 to welcome new sprites
	index = 0;
}

It also puts the textures into a list.

The code to draw two sprites is this:

	spriteBatch->draw(&_sprite1->getTexture(), _sprite1->x, _sprite1->y, _sprite1->width, _sprite1->height);
	spriteBatch->draw(&_sprite1->getTexture(), _sprite1->x+10, _sprite1->y+10, _sprite1->width*2, _sprite1->height);


	spriteBatch->flush();

but I only get one small black rectangle in the bottom left corner.

It works perfectly when I draw the sprites without the spritebatch; 

_sprite1->draw(*camera);
_sprite2->draw(*camera);

I think I messed up in the flush() method, but I have no clue how to implement this. I'd be grateful if someone can help me with it.

Thank you!

I see a number of issues and potential issues here. I won't try to address all of them, but I'll try to offer a few ideas to perhaps help move things in the right direction.

It does look like you have the wrong idea conceptually about batching, so that's probably the first thing to look at.

It seems like this:

textures->push_back(texture);

Will cause the textures array to grow continually, which I doubt is what you intend. It also looks like you're leaking OpenGL resources in your flush function. Those are two possible technical errors that I noticed.

The main conceptual error here is that in your flush function you're still issuing one draw call per quad, whereas the purpose of batching is to combine multiple objects (quads, meshes, etc.) and draw them in a single draw call.

Another aspect of this is that typically all objects in a batch will share the same render state. This means, among other things, sharing the same texture. If your quads all use different textures, then barring a more sophisticated system, you won't be able to batch them. Common solutions are to group quads that use the same render state together in the render order so they can be batched, and also to use texture atlases to increase the number of quads (or other objects) that can be batched together.

A simple batching system might looking something like this:

- Create a single set of OpenGL resources (VAO, VBOs) in advance. These resources will be reused for every batch. (This is in contrast to creating them for every flush, as you're doing now.)

- Create a single index array to use for all batches, large enough to accommodate the maximum number of quads per batch. Using your convention, this would look something like this: 0, 1, 3, 1, 2, 3, 4, 5, 7, 5, 6, 7, etc.

- As quads are added, add the vertex data to a container. Typically vertex positions would have the model transform pre-applied, since you can no longer change the corresponding matrix in the shader per-quad.

- When it's time to flush, upload the vertex data to the buffer(s), and make a single draw call that draws all the batched quads at once.

That's a very terse overview, and there are many variations on this approach (some more performant than others).

I see some other potential code quality issues and things that look a bit suspicious, but they may be tangential, so I won't go into detail here. I'll just point out a couple things though. I'm a little suspicious of your deleting 'texture' in the Sprite destructor, as that implies exclusive ownership, which would be somewhat unusual. Also, the std::vector instance 'textures' doesn't need to be a pointer or to be created with 'new' (and it looks like it's being leaked). More generally, the use of pointers and references here looks a bit haphazard and could potentially lead to lifetime-related issues. A typical approach in C++ is to use smart pointers for this sort of thing, which helps clarify ownership, avoid object lifetime errors, and avoid leaks.

Thanks a lot for the explanations.

Quote

and also to use texture atlases to increase the number of quads (or other objects) that can be batched together.

So if the quads share the same texture atlas, they can still be batched together even though they use different parts of the texture atlas, did I understand that right?

As for 

textures->push_back(texture);

I wanted to have all textures stored in a list, so I can bind them. I mean I must save them somewhere and it will be multiple textures until I flush. Also, they are just pointers to the objects, no real objects, so it doesn't matter if I have many of them here, right? I'll delete them elsewhere. For the vector, I have added a simple delete textures; in the destructor for now.

I've moved     

    glGenVertexArrays(1, &VAO);
    glGenBuffers(1, &VBO);
    glGenBuffers(1, &EBO);

into the constructor. Is that enough or do I have to "empty" the objects every flush call?

And where should I delete the texture? Visual studio throws an error when I close it at delete texture in the destructor, but I don't understand why. It's a pointer, so I have to delete it. Is it really necessary to delete the texture from where I created it? I'm thinking of looking into smart pointers but my main goal now is to get this to work, smart pointers look pretty time consuming.

Well, the first thing I have to do is to sort the quads first, so that the batch only renders quads with the same textures at once, right? For this I have written this piece of code:

	std::vector<std::vector<textureandindex>> quads;
	for (int i = 0; i < spritesInBatch; i++) {
		std::vector<textureandindex> sameTextureObjects;
		Texture *tex = textures->at(i);
		textures->erase(textures->begin() + i);
		textureandindex tocompare = { i,tex };
		sameTextureObjects.push_back(tocompare);

		for (int a = 0; a < spritesInBatch; a++) {
			if (textures->at(a) == tex) {
				textures->erase(textures->begin() + a);
				textureandindex compared = { a,textures->at(a) };
				sameTextureObjects.push_back(compared);
			}
		}
		quads.push_back(sameTextureObjects);
	}

It should sort all quads which have the same textures into the vector sameTextureObjects. And all those sameTextureObjects vectors are added to the quads vector. Now the batch should be able to render all quads in every vector in the quads vector at once, and the number of items in the quads vector are basically the amount of draw calls.

The quads in there are also in a struct called textureandindex which is basically the index + the texture, so when drawing, I know what indexes and vertices each quad has.

And you said this

1 hour ago, Zakwayda said:

- Create a single index array to use for all batches, large enough to accommodate the maximum number of quads per batch. Using your convention, this would look something like this: 0, 1, 3, 1, 2, 3, 4, 5, 7, 5, 6, 7, etc.

- As quads are added, add the vertex data to a container. Typically vertex positions would have the model transform pre-applied, since you can no longer change the corresponding matrix in the shader per-quad.

isn't that what my draw() function already does?

How do I pre-apply the model transform? Just multiply every vertex in the draw function with it?

Personally I wouldn't necessarily worry about batching until you've solidified some other aspects of your code. Batching is an optimization, and not always a necessity. I don't know what platforms you're targeting or how many quads you're rendering, but you may very well be able to get away without batching (for the time being at least).

Now I'll attempt to answer some of your queries

Quote

So if the quads share the same texture atlas, they can still be batched together even though they use different parts of the texture atlas, did I understand that right?

Yes. Using different parts of the texture only requires using different texture coordinates for each quad, which is not a problem (just as it's not a problem to use different vertex positions for each quad).

Quote

I wanted to have all textures stored in a list, so I can bind them. I mean I must save them somewhere and it will be multiple textures until I flush.

As noted, this is conceptually wrong. A simple batching system requires all batched objects to share the same render state, which means sharing the same texture. So, there's no need to save textures as you go. There will only be one texture used for each batch. (Note that I'm keeping things simple here and not worrying about things like multitexturing and so on.)

Quote

Also, they are just pointers to the objects, no real objects, so it doesn't matter if I have many of them here, right?

In your original code I don't see you clearing the textures vector anywhere, which means it's growing unbounded with each call to draw() (this is true regardless of what you're storing in the vector). You could fix this by clearing it in the appropriate place, but as discussed elsewhere the texture container is conceptually wrong to begin with, so it's not needed anyway.

Quote

For the vector, I have added a simple delete textures; in the destructor for now.

That's really just a bandaid. The correct solution is not to create the vector using 'new'.

Quote

Is that enough or do I have to "empty" the objects every flush call?

I'm not sure what you mean by empty, but typically you would upload the needed data (e.g. vertex data) to the appropriate buffers each flush before rendering. (There are variations on that approach, but that's the simplest approach.)

Quote

And where should I delete the texture? Visual studio throws an error when I close it at delete texture in the destructor, but I don't understand why. It's a pointer, so I have to delete it. Is it really necessary to delete the texture from where I created it? I'm thinking of looking into smart pointers but my main goal now is to get this to work, smart pointers look pretty time consuming.

Smart pointers aren't time-consuming. Or at least they're less time-consuming than the types of errors they're intended to prevent

That said, I think I agree that you shouldn't switch to smart pointers yet, but for different reasons than you've given. It's important to understand the underlying concepts (pointers, memory management, object lifetime), and implementing it 'manually' first before using smart pointers will help with that.

Something being a pointer doesn't mean you have to delete it. In the simplest terms, you delete things that you create with 'new', and there should be exactly one invocation of 'delete' for each invocation of 'new'. This gets into issues of object lifetime and ownership, which are conceptually important (and particularly difficult in C++).

Quote

Well, the first thing I have to do is to sort the quads first, so that the batch only renders quads with the same textures at once, right?

Conceptually, yes (although I'm not sure the code you posted for that is correct).

Quote

isn't that what my draw() function already does?

Your draw function does add vertex data progressively to a container. That part looks more or less correct. It's the rest of the implementation (the index handling, and using a draw call per quad) that's incorrect.

Quote

How do I pre-apply the model transform? Just multiply every vertex in the draw function with it?

Yes, that's one way to do it. Basically, you apply the model transform to the vertex positions in your own code before storing them in the container.

After all that though, I'd still suggest setting batching aside for the moment and getting some other things squared away first. If you feel like it, you might post your texture class, as we might be able to offer some suggestions regarding lifetime management and so on.

My goal with the batching is to do it like libgdx does it: You can draw every single texture in the batch between beginning and flushing. It doesn't have to be the same one. 

That's what I wanted to do because it's much simpler to work with in the end rather than starting a new batch for each texture. 

That's why I wanted to save the textures. The batch itself would start a new draw for every item in the quads vector automatically so I won't have to do it myself. That's why I sorted it, else wouldn't sorting be pretty unnecessary as everything has the same texture anyways?

I think the batching will be pretty important for me because I want to do a 2d orthogonal tile game, like pokemon. There will be a lots of duplicate textures, especially for the ground, so it will probably give me a huge performance boost.

The texture actually isn't created with new, I just create it in the main function and pass it as reference.

28 minutes ago, Zakwayda said:

I'm not sure what you mean by empty, but typically you would upload the needed data (e.g. vertex data) to the appropriate buffers each flush before rendering. (There are variations on that approach, but that's the simplest approach.)

Quote

Well if I used the VAO in the first batch it will be filled with data so I thought I have to clear it first if I want to use it again, but if it just gets overwritten then that's fine.

30 minutes ago, Zakwayda said:

That's really just a bandaid. The correct solution is not to create the vector using 'new'.

Alright, that's just a java habit creating everything with new ?

33 minutes ago, Zakwayda said:

It's the rest of the implementation (the index handling, and using a draw call per quad) that's incorrect.

What exactly is incorrect with the index handling? Every quad has 2 triangles with exactly those 2 duplicate points, so shouldn't the index buffer be always the same?

Sorry for asking so many questions and thanks for the answers.

As for my Texture class, I think it looks okay. Apart from that that I have to set this reg_comp field (STBI_rgb_alpha or STBI_rgb) for each texture manually, I thinks everything is deleted and freed correctly.

#include "Texture.h"


Texture::Texture(char *texturePath, int reg_comp)
{
	glGenTextures(1, &texture);

	int _width, _height, _ImageComponents;

	unsigned char *image = stbi_load(texturePath, &_width, &_height, &_ImageComponents, reg_comp);
	if (image == nullptr) {
		std::cout << "error loading image" << std::endl;
	}


	glBindTexture(GL_TEXTURE_2D, texture);


	// Set our texture parameters
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);	
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
	// Set texture filtering
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

	//glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, _width, _height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);

	if (_ImageComponents == 3)
		glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, _width, _height, 0, GL_RGB, GL_UNSIGNED_BYTE, image);
	else if (_ImageComponents == 4)
		glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, _width, _height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image); //RGBA8?


	glGenerateMipmap(GL_TEXTURE_2D);

	glBindTexture(GL_TEXTURE_2D, 0);
	stbi_image_free(image);
}


Texture::~Texture()
{
	glDeleteTextures(1, &texture);
}

the texture (GLuint) will always be deleted at the destructor.

Quote

I think the batching will be pretty important for me because I want to do a 2d orthogonal tile game, like pokemon. There will be a lots of duplicate textures, especially for the ground, so it will probably give me a huge performance boost.

That does sound like a situation in which batching could be advantageous. If aspects of the environment are static, you might also consider storing parts of the environment in static buffers rather than streaming the data, which might be even more performant.

Quote

The texture actually isn't created with new, I just create it in the main function and pass it as reference.

If you're not creating it with new, then you don't need to delete it.

Quote

Well if I used the VAO in the first batch it will be filled with data so I thought I have to clear it first if I want to use it again, but if it just gets overwritten then that's fine.

Yes, that's right. Typically you just overwrite the data in an existing buffer. (There are various other approaches you can use for streaming vertex data, such as multibuffering, buffer orphaning, and buffer mapping, that may be more performant than simply reusing the same buffer repeatedly, but I'd recommend getting it working with a single buffer or set of buffers first.)

Quote

What exactly is incorrect with the index handling? Every quad has 2 triangles with exactly those 2 duplicate points, so shouldn't the index buffer be always the same?

It looks to me like your index buffer just contains 0, 1, 3, 1, 2, 3, repeatedly, but in a typical quad batching system it would contain something like I mentioned earlier (0, 1, 3, 1, 2, 3, 4, 5, 7, 5, 6, 7, etc.).

Say there are two quads in a batch. The vertex buffer(s) contain something like this:

vertex 1 of quad 1
vertex 2 of quad 1
vertex 3 of quad 1
vertex 4 of quad 1
vertex 1 of quad 2
vertex 2 of quad 2
vertex 3 of quad 2
vertex 4 of quad 2

The indices to draw these two quads in one draw call would be (using your convention):

0, 1, 3, 1, 2, 3, 4, 5, 7, 5, 6, 7

Also, for a batching system dedicated entirely to quads, the indices will always be the same - the only thing that will change from batch to batch is how many of them are submitted when drawing. So, you can create a single index buffer of appropriate size up front rather than rebuilding it for each batch.

Quote

Sorry for asking so many questions and thanks for the answers.

No problem

For your texture class, you'll eventually want to look into the 'rule of three', as the way you currently have it implemented could lead to incorrect behavior. (These posts are long enough as is, so I won't elaborate here.)

As for LibGDX, I'm not familiar enough with its batching system or rendering interface to comment directly on that.

Alright thanks, now I have quite a bit of work to do

I have one other problem though:

struct glfloat {
	GLfloat item[800];
};

GLfloat myVertices[800];

glfloat structMyVertices = { myVertices }; //ERRORHERE

It says I can't use a value of type ""GLfloat *"" to initialize an entity of type ""GLfloat"". But there is no pointer anywhere.

And if I do

	glfloat structMyVertices;
	structMyVertices/*ERRORHERE*/.item = myVertices;

it says expression must be a modifiable lvalue. Why?

That should be a workaround to store an [] array into a vector btw

14 minutes ago, Ookamix said:

struct glfloat { GLfloat item[800]; };

For the love of all that is important in your life, DO NOT USE THAT NAME.  You are going to make someone want to murder you.  No.  That does not, in any way what so ever, explain what the point of the struct is, at all.  Just, no.  I cannot emphasis that enough!  I'm not even sure what you're attempting here, as the variable item doesn't make any sense either (as a name).  But glfloat, argh!

Also a few observations about your code in general:

Why are you hard coding your vertex/index buffers but allowing unlimited textures?  You're already using std::vectors, let's make the class even more general.

struct QuadData
{
	GLfloat verts[4] = { 0.0f };
	GLuint	indicies[4] = { 0 };
};

std::vector<QuadData*> quadData;

I agree @Zakwayda you need to learn about pointers, object life-times, memory management, etc. before moving on to smart pointers.  Though smart pointers takes about 10 minutes to learn honestly, to use effectively (typically want to use unique_ptr, if you're using shared_ptr a lot you have a general flaw in most cases) takes longer though.

Also, you're prematurely optimizing without any data to show the need here.  I've had 2D tile based games that had 2000+ tiles on screen without batch rendering running at like 5ms/frame on an nVidia 970.  Make a good, easy to use interface and start making your game.  You have to decide, are you writing a game or an engine, because you'll likely only get one or the other done and you seem fairly new to C/C++ and learning while writing an engine will likely lead to multiple rewrites of your engine.

Food for thought

Alright alright

I know that my code is dirty, I will rewrite it. "item" was actually called "glfloat" too and it somehow compiled, but I'm not that dirty ^^

4 hours ago, CrazyCdn said:

Why are you hard coding your vertex/index buffers but allowing unlimited textures?  You're already using std::vectors, let's make the class even more general.

I'll do that, but that doesn't explain why I can't set  C-array in the struct without getting errors I'd rather not set the value for every index of the array seperately.

I plan to make a game for desktop only first. I have used the Godot engine first and it really lagged a lot with just 20 sprites, so I think it would really need optimizing. Also I can't stop thinking about my fellow gamers with integrated intel hd graphics, I don't want to give them a game which runs at 20fps.

The main reason why I want to code an engine though is because I want to improve my C++ skills (and because I have to kill time until I get an artistic friend who can actually provide me with assets for games.)

Well, I'll then continue to write a batch which can draw different textures. But I have noticed that the draw order will be incorrect. If I want to draw a quad with texture A, a quad with texture B and then another quad with texture A so texture B is rendered between them, it won't work because all texture A's are drawn first. How do I address this issue?

Quote

I'll do that, but that doesn't explain why I can't set  C-array in the struct without getting errors  I'd rather not set the value for every index of the array seperately.

The short answer is that you can't assign one raw array to another in C++ (the reasons for this are partly historical). As for the mention of a pointer in your error message, that's likely due to array decay.

Typically you'd use std::vector for large arrays like that. How best to copy the contents of one array to another depends on the context, but there are various convenient ways to do it - you don't have to do it manually.

Quote

But I have noticed that the draw order will be incorrect. If I want to draw a quad with texture A, a quad with texture B and then another quad with texture A so texture B is rendered between them, it won't work because all texture A's are drawn first. How do I address this issue?

This is one of the problems that atlasing and related techniques (e.g. array textures) are intended to address. You are correct that with a basic batching implementation, you'd have to start a new batch at each texture change. So in your A-B-A example, you'd have three batches of one quad each. With atlasing (for example), the two textures would be combined into a single texture, with each quad's texture coordinates specified so as to use the desired part of the atlas. Then all three quads could be rendered in the same batch.