(Beginner) Texture problem

Hello, I am a beginner OpenGL learner and I am trying to stick a texture onto my mesh. For some reason, every time my program gets to line “glTextureParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);” it crashes due to memory access violation. What is wrong with my code? Am I reading the file incorrectly? Thank you for any help you could give me.

texture.cpp

Texture::Texture(const std::string& filename)
{
	unsigned char* image = SOIL_load_image(filename.c_str(), &m_iWidth, &m_iHeight, 0, SOIL_LOAD_RGBA);

	if (image == nullptr)
		std::cerr << "Texture loading failed. 
";

	glGenTextures(1, &m_texture);
	glBindTexture(GL_TEXTURE_2D, m_texture);

	// Texture Wrapping 
	glTextureParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
	glTextureParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);

	// Texture Filtering 
	glTextureParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
	glTextureParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

	glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, m_iWidth, m_iHeight, 0, GL_RGBA, GL_UNSIGNED_BYTE, image); 

	SOIL_free_image_data(image);
}

source.cpp

void render()
{
	glUseProgram(program);
	glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

	Mesh mesh(vertices, sizeof(vertices) / sizeof(vertices[0]));
	Texture texture("Files/Sprites/Fireball.png"); // If I remove .png it then display "Texture loading failed"
	
        texture.bind();
	mesh.Draw();
 

	glutSwapBuffers();
}

thats 1 reason to remove opengl code out of the constructor and put it in a “initialize()” function. its very likely that at the time the “Texture” class constructor is invoked, you didnt initialize the opengl loader library (for example GLEW)

the opengl functions like “glTextureParameteri()” are actually function pointers, and are initially null / undefined, you have to query the address of that function before calling it (by initializing GLEW / other loader libraries)

https://www.khronos.org/opengl/wiki/Getting_Started#Getting_Functions
https://www.khronos.org/opengl/wiki/OpenGL_Loading_Library

Every time or from a certain time ?

You create the same texture every frames, which you should not. All OpenGL objects don’t have to be created each time. They are persistent. So create them once you have initialized OpenGL fully (once you have a valid context, this context is current and all your potential function pointers have been affected their addresses - threw glew for example).

You’re getting confused between the DSA (Direct State Access) functions added in 4.5 and the non-DSA functions.

The above call would be correct for glTexParameteri(). Note: Tex, not Texture; the non-DSA functions use Tex, the DSA functions use Texture. Non-DSA functions identify the texture via a “target” parameter such as GL_TEXTURE_2D, which refers to the specific target in the active texture unit. The DSA functions identify a texture via its “name”; the GLuint generated by glGenTextures() or glCreateTextures().

[duplicated post, sorry]

Hello again, thank you for your help. I think I am finally getting somewhere with this, thanks to you. As GClements mentioned, I should have been using the glTexParameteri function instead as I am identifying the textures by name. I removed all OpenGL code from constructors as John_Connor suggested and that way I am not longer re-creating meshes per frame as Silence mentioned.

Here is the result (so far): http://imgur.com/a/Ix32f (the forum wouldn’t let me use a full link, simply substitute the (dot) with an actual dot)

Pretty isn’t it? Well, as pretty as it is, that is not the result I was hoping to obtain. The mesh itself should have been a triangle, and this doesn’t look like a triangle, more like a tunnel. A fiery one, rather.

I’d like to ask for your assistance one last time, and help me identify the issue that’s causing this distortion.

Mesh.h

class Mesh
{
public:
    Mesh();
    ~Mesh();

    void InitGeometry(CVertex*, unsigned int);
    void Draw();

private:
    Mesh(const Mesh&) = delete;
    void operator=(const Mesh&) = delete;
    
    enum { POSITION_VB, TEXTURE_VB, NUM_BUFFERS };

    GLuint m_VAO;
    GLuint m_VBO[NUM_BUFFERS];
    unsigned int m_DrawCount;
};

Mesh.cpp

void Mesh::InitGeometry(CVertex* vertices, unsigned int numVertices)
{
    m_DrawCount = numVertices;

    glGenVertexArrays(1, &m_VAO);
    glBindVertexArray(m_VAO);

    std::vector<glm::vec3> positions;
    std::vector<glm::vec2> texCoords;

    positions.reserve(numVertices);
    texCoords.reserve(numVertices);

    for (unsigned int i = 0; i < numVertices; ++i)
    {
        positions.push_back(vertices[i].GetPos());
        texCoords.push_back(vertices[i].GetTex());
    }

    glGenBuffers(NUM_BUFFERS, m_VBO);
    glBindBuffer(GL_ARRAY_BUFFER, m_VBO[POSITION_VB]);
    glBufferData(GL_ARRAY_BUFFER, numVertices * sizeof(positions[0]), &positions[0], GL_STATIC_DRAW);

    glVertexAttribPointer(0, 3, GL_FLOAT, false, 0, 0);

    glBindBuffer(GL_ARRAY_BUFFER, m_VBO[TEXTURE_VB]);
    glBufferData(GL_ARRAY_BUFFER, numVertices * sizeof(texCoords[0]), &texCoords[0], GL_STATIC_DRAW);

    glVertexAttribPointer(1, 2, GL_FLOAT, false, 0, 0);

    glEnableVertexAttribArray(0);
    glEnableVertexAttribArray(1);

    glBindVertexArray(0);
}

void Mesh::Draw()
{
    glBindVertexArray(m_VAO);
    glDrawArrays(GL_TRIANGLES, 0, m_DrawCount);
}

Texture.cpp

void Texture::InitTexture(const std::string& filename)
{
    unsigned char* image = SOIL_load_image(filename.c_str(), &m_iWidth, &m_iHeight, 0, SOIL_LOAD_RGBA);

    if (image == nullptr)
        std::cerr << "Texture Image Loading Operation Failed. 
";

    glGenTextures(1, &m_texture);
    glBindTexture(GL_TEXTURE_2D, m_texture);

    // Texture Wrapping 
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);

    // Texture Filtering 
    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
    glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, m_iWidth, m_iHeight, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);

    SOIL_free_image_data(image);
}
void Texture::Bind(unsigned int unit)
{
    glActiveTexture(GL_TEXTURE0 + unit);
    glBindTexture(GL_TEXTURE_2D, m_texture);
}

Vertices define as such:

CVertex vertices[] =
{
    CVertex(glm::vec3(-0.5f, -0.5f, 0.0f), glm::vec2(0.0f, 0.0f)),
    CVertex(glm::vec3(0.0f, 0.5f, 0.0f), glm::vec2(0.0f, 1.0f)),
    CVertex(glm::vec3(0.5f, -0.5f, 0.0f), glm::vec2(1.0f, 1.0f))
};

source.cpp

void render()
{
    glUseProgram(program);
    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

    mesh.InitGeometry(vertices, sizeof(vertices) / sizeof(vertices[0]));
    texture.InitTexture("Assets/Sprites/Fireball.png");
    
    texture.Bind(0);
    mesh.Draw();
    
    glutSwapBuffers();
}

Thank you for your help so far. I am open to more tips if you can offer me, I’m trying to learn OpenGL the right way.

Certainly not related to the issue you have, but you are still creating OpenGL objects each frame:

InitGeometry creates VAOs and is called within render(). You should not.

Also, I advise you to use plain float instead of classes to store/represent your graphical data (vertices, texture coordinates).
Finally, and as a starting point, avoid interleaved arrays (so one separate array for each attribute).

i’d suggest the opposite, but maybe its a matter of convenience:

– use structs for plain data, classes for “controlled” interaction
– “CVertex” wuld be plain data, so:

struct CVertex_P { /* P indicated the members it has: here just position (usually vec3) */
   vec3 Position;
};

struct CVertex_PT { /* PT = position + texcoord */
   vec3 Position;
   vec2 TexCoord;
};

struct CVertex_PN { /* PN = position + normal*/
   vec3 Position;
   vec3 Normal;
};

struct CVertex_PNT { /* PNT = position + normal + texcoord */
   vec3 Position;
   vec3 Normal;
   vec2 TexCoord;
};

etc...

– for ALL the (static) vertices (of the same type, see above) you can use 1 (array) buffer + 1 vertexarray (+ 1 element buffer if needed)
– use std::vector<CVertex> instead of const sized CVertex myarray = …, because a vector can grow
– render your scene “centralized”, that means do not create classes with members like void RenderMeSomehow(); instead, create 1 function (or singleton class) that renders everything, that very function manages textures, buffers, programs, everything regarding the visual representation of your scene

example:

https://www.khronos.org/opengl/wiki/Tutorial:_OpenGL_3.1_The_First_Triangle_(C%2B%2B/Win)

other stuff to read:
https://www.khronos.org/opengl/wiki/Common_Mistakes
https://www.khronos.org/opengl/wiki/Vertex_Specification_Best_Practices
https://www.khronos.org/opengl/wiki/Vertex_Rendering

Thank you for the tips and examples. Here’s the changes I have made:

struct CVertex_PT
{
	glm::vec3 position;
	glm::vec2 texCoord;
};

Mesh.cpp

void Mesh::InitGeometry(CVertex_PT* vertices, unsigned int numVertices)
{
	m_DrawCount = numVertices;

	glGenVertexArrays(1, &m_VAO);
	glBindVertexArray(m_VAO);

	glGenBuffers(1, &m_VBO);
	glBindBuffer(GL_ARRAY_BUFFER, m_VBO);
	glBufferData(GL_ARRAY_BUFFER, numVertices * sizeof(CVertex_PT), vertices, GL_STATIC_DRAW);

	glEnableVertexAttribArray(0);
	glEnableVertexAttribArray(1);
	glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(CVertex_PT), 0);
	glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, sizeof(CVertex_PT), (void*)12);

	glBindVertexArray(0);
}

void Mesh::Draw()
{
	glBindVertexArray(m_VAO);
	glDrawArrays(GL_TRIANGLES, 0, m_DrawCount);
}

source.cpp

void startUp()
{
	CVertex_PT vertex[3];
	glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

	vertex[0].position = { -0.5f, 0.0f, 0.0f }; vertex[1].position = { 0.0f, 1.0f, 0.0f }; vertex[2].position = { 0.5f, 0.0f, 0.0f };
	vertex[0].texCoord = { 0.0f, 0.0f }; vertex[1].texCoord = { 0.5f, 1.0f }; vertex[2].texCoord = { 1.0f, 0.0f };

	mesh.InitGeometry(vertex, sizeof(vertex) / sizeof(vertex[0]));
	texture.InitTexture("Assets/Sprites/Fireball.png");

	Shader shader;
	program = shader.CreateProgram("VertexShader.vs", "FragmentShader.fs");

	glUseProgram(program);
}

void render()
{
	texture.Bind(0);
	mesh.Draw();
	
	glutSwapBuffers();
}

Problem is still persisting for some reason, but it’s a different shape now, it’s getting closer to a triangle though. Don’t understand why the coordinates are messing up.

[QUOTE=john_connor;1286956]i’d suggest the opposite, but maybe its a matter of convenience:
[/quote]
Yes, certainly a matter of convenience. I find it more easy to check data if you quickly know what that data is. If you go threw the vertex array, you know you have vertices. If it’s interleaved, then you have to do some (little) maths to understand if you are at a vertex position or a normal position…

This, to my opinion also, might be dangerous. You might end with objects which have a different size than what you thought. And debugging this, mostly when beginning, is very difficult.
Plus, depending on the language, the version of the language, potential inheritance, the compiler, options of the compiler, you can have different behaviors.
See this or thatfor example.

If all of the data is constant, then interleaving has a performance benefit. But if some of the attributes are dynamic, then using separate arrays allows you to replace a modified array in its entirety while leaving untouched the arrays for the unmodified attributes. It also allows you to set different usage flags for different attributes.

Also, you need to consider that structures may contain padding between fields. This will be more of an issue if compact types are used instead of floats.

Well, no. It might. But that’s less and less true.

Oh wow, I’m so stupid. It turned out the vertex coordinates were working just fine, it’s just the texture (because it is the shape of a ball) was giving it this weird distorted look. Anyway, I think my problem here is resolved all thanks to you guys. Many thanks for your tips (which I have noted btw), I will be posting again should I need any further assistance from great API programmers like yourselves.