Jump to content
  • Advertisement
Sign in to follow this  
MaimaVanPersie

OpenGL Quad not drawing correctly (Beginner)

This topic is 568 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello guys, I only started learning OpenGL recently and I have been able to draw several primitives. I wanted to organize my code by placing it in classes and stuff as opposed to putting it all in main. I have been able to draw a quad with texture on it, now when I call render all I see is this odd result: http://imgur.com/auKpRGH

 

Here's what I have, please point out my mistake(s) I'd greatly appreciate that :)

 

Global Initializations in main.cpp: 

Vertex Vertices[] =
{ 
Vertex(glm::vec3(1.0f, 1.0f, 0.0f), glm::vec2(0.0f, 0.0f), glm::vec3(0.0f,0.0f,0.0f)),
Vertex(glm::vec3(1.0f, -1.0f, 0.0f), glm::vec2(0.0f, 1.0f), glm::vec3(0.0f,0.0f,0.0f)),
Vertex(glm::vec3(-1.0f, -1.0f, 0.0f), glm::vec2(1.0f, 1.0f), glm::vec3(0.0f,0.0f,0.0f)),
Vertex(glm::vec3(-1.0f, 1.0f, 0.0f), glm::vec2(1.0f, 0.0f), glm::vec3(0.0f,0.0f,0.0f)),
};

Render function in main.cpp: 

void Render()
{
Sprite Background(Vertices, sizeof(Vertices) / sizeof(Vertices[0]));
Background.Render();
glutSwapBuffers();}

My Sprite.h header:

class Vertex
{
public:
Vertex(const glm::vec3& pos, const glm::vec2& texCoordinates, const glm::vec3& colors) 
{
this->m_Pos = pos;
this->m_Tex = texCoordinates;
this->m_Col = colors;
}


private:
glm::vec3 m_Pos;
glm::vec2 m_Tex;
glm::vec3 m_Col;
};


class Sprite
{
public:
Sprite();
Sprite(Vertex* vertices, unsigned int numOfVertices);
~Sprite();


void Render();


private:


enum { POSITION_BUFFER, TEXTURE_BUFFER, COLOR_BUFFER, NUM_OF_BUFFERS };


GLuint m_VertexArrayObject;   
GLuint m_VertexBufferObject[NUM_OF_BUFFERS];
GLuint m_ElementBufferObject;
GLuint m_Program;
ShaderLoader m_Shader;
unsigned int m_DrawCount; 
};

My Sprite.cpp file:

#include "Sprite.h"


Sprite::Sprite()
{
m_Program = m_Shader.CreateProgram("VShader.vs", "FShader.fs");
}


Sprite::~Sprite()
{

}


Sprite::Sprite(Vertex* vertices, unsigned int numOfVertices)
{
m_DrawCount = numOfVertices;
glGenVertexArrays(1, &m_VertexArrayObject);
glBindVertexArray(m_VertexArrayObject);


glGenBuffers(1, m_VertexBufferObject);
glBindBuffer(GL_ARRAY_BUFFER, m_VertexBufferObject[POSITION_BUFFER]);
glBufferData(GL_ARRAY_BUFFER, numOfVertices * sizeof(vertices[0]), vertices, GL_STATIC_DRAW);


glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, (GLvoid*)0);

glEnableVertexAttribArray(0);
glBindVertexArray(0);
}


void Sprite::Render()
{
glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);


glUseProgram(m_Program);
glBindVertexArray(m_VertexArrayObject);


glDrawArrays(GL_QUADS, 0, m_DrawCount);


glBindVertexArray(0);
}

Basically, I just want to draw the quad for now. And then, I want to be able to pass in the texture coordinates to display a nice picture. In the future, I want to be able to create multiple sprites like this and stuff like that. Am I doing it right? And what seems to be the problem with my code above? Why is it not producing the expected result? 

 

Thank you very much for reading my thread and I appreciate any help/advice you could give me. 

 
Edited by MaimaVanPersie

Share this post


Link to post
Share on other sites
Advertisement

Hello,

The problem you are experiencing has to do with the layout of your vertex data in C++, relative to the layout of your OpenGL buffer. You have already come a long way though and what you are doing is mostly correct.

First off, on the intended layout of your OpenGL buffers, it is common practice to create only one buffer for all your vertex properties whenever possible. In this case, that means that the vertex position, texture coordinate and color are combined in an interleaved form into one buffer. This means you will only need one Vertex Buffer Object. Think of it as (v1 x, v1 y, v1 z, v1 u, v1 v, V1 r, v1 g, v1 b, v2 x, v2 y, ...). This also happens to be an exact match to the vertices in your C++ Vertex array. When you call:

glBufferData(GL_ARRAY_BUFFER, numOfVertices * sizeof(vertices[0]), vertices, GL_STATIC_DRAW);

your OpenGL buffer is actually filled with exactly that data.

Using that buffer, it is up to you to specify where in that buffer your vertex positions are located. As you already wrote, you can use glVertexAttribPointer for that. You wrote:

glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, (GLvoid*)0);

The first parameter here is an index to a generic vertex attribute. It is basically an index to the property in your vertex shader that will receive this bit of data. You can use glGetAttribLocation to get that index of your don't know what it is. If you have only one property, 0 is (probably?) a pretty safe bet.

The second parameter you provided is correct. 3 shows you have 3 float for this property, an xyz triplet, turned into a vec3 in glsl.

The third and fourth parameters are also correct. These denote the type and normalization of the data.

The fifth parameter is where things get interesting. This is the stride, the byte offset between consecutive generic vertex attributes. In other words, in your case, the size in bytes of your Vertex structure. This is the offset between your separate vertices. If you specify 0 here, it defaults to the size of the given property, which is the size of 3 floats in this case.

The sixth and final parameter is a 'pointer', a byte offset to the component you are referring to for this property. For each vertex, this should point to the first float (or just the vec2/vec3 itself) of the property you are declaring. C++ has a useful function here: offsetof. So you could write something like:

(void *)offsetof(Vertex, m_Pos)

I'm not entirely sure if having m_Pos private will cause problems for that call.

 

Now looking back at what you had written so far, what you actually see happening, is that your entire vertex array is interpreted as triplets of xyz. Noting that your displayed coordinates are in the [-1, 1] range for xy with an ignored z, what you see being drawn starts to make sense.

Just two more small things that I would like to point out as advice. You are using quads, which makes sense for sprites, but please note that quads are now deprecated and have even been removed in modern OpenGL versions. Instead, try to form your quads using two triangles.

Also, since you are programming in C++, try using containers for your data whenever you can. If you pass a pointer to an array and a length of that array, you could have probably used a vector instead. Those actually clean up after themselves, without you having to delete them, and you can query their size easily.

Edited by Ignifex

Share this post


Link to post
Share on other sites

Hello Ignifex, 

Thank you for your help. I have been able to fix the problem with the triangle. I also took your advice and drew the quad as two triangles. I have also used indices for efficiency as well. I guess the last thing I'm stuck on before moving on to doing other things is texturing. Here is how my code looks like right now:

 

Sprite::Sprite(Vertex* vertices, unsigned int numVertices)
{
m_NumVertices = numVertices;


GLubyte m_Background[] = { 0, 1, 3, 1, 2, 3 };


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


glGenBuffers(1, &m_VertexBufferObject);
glBindBuffer(GL_ARRAY_BUFFER, m_VertexBufferObject);
glBufferData(GL_ARRAY_BUFFER, m_NumVertices * sizeof(vertices[0]), vertices, GL_STATIC_DRAW);


glGenBuffers(1, &m_ElementBufferObject);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_ElementBufferObject);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(m_Background), m_Background, GL_STATIC_DRAW);


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


glEnableVertexAttribArray(0);
glEnableVertexAttribArray(1);


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


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


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


unsigned char* image = SOIL_load_image("adventure.png", &m_iWidth, &m_iHeight, 0, SOIL_LOAD_RGBA);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, m_iWidth, m_iHeight, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);


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


void Sprite::Draw()
{
glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);


glActiveTexture(GL_TEXTURE0);
glBindTexture(GL_TEXTURE_2D, m_Textures);
glUniform1i(glGetUniformLocation(m_Program, "Texture1"), 0);


glUseProgram(m_Program);
glBindVertexArray(m_VertexArrayObject);


glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_BYTE, 0);


glBindTexture(GL_TEXTURE_2D, 0);
glBindVertexArray(0);


glutSwapBuffers();
}

For some reason the texture is not showing.  :(

Do you spot a mistake by any chance? 

 

Also I tried using vectors to store my vertices in but it required a bit of extra work including returning pointers from functions and things like that so for now I decided to move on with a simple pointer.   

Share this post


Link to post
Share on other sites

Nice to see you got it working already!

I found one thing that might be your problem. You are calling glUniform1i before glUseProgram. However, the uniform set by glUniform1i is actually set for the active program, set by a call to glUseProgram. This should however allow it to work in the next Draw call.

Could you post your shaders as well? If you want to test your texture coordinates, you could try rendering these out directly in your fragment shader.

Share this post


Link to post
Share on other sites

Here are my shaders: 

Vertex Shader:

#version 430 core

// Input
in layout (location = 0) vec3 VertexPos;
in layout (location = 1) vec2 TexCoordinate;
in layout (location = 2) vec4 VertexColors;


// Output 
out vec4 outputColors;
out vec2 TexCoord;

void main()
{
gl_Position = vec4(VertexPos, 1.0f);
outputColors = VertexColors;
TexCoord = TexCoordinate;
}

Fragment Shader:

#version 430 core

in vec4 outputColors;
in vec2 TexCoord;

out vec4 VertexColors;

uniform sampler2D Texture1;

void main()
{
VertexColors = texture(Texture1, TexCoord);
//VertexColors = outputColors;
}

And here is how I'm introducing vertex information and rendering the scene in main.cpp header file

Vertex Position[] =
{
Vertex(glm::vec3(1.0f, 1.0f, 0.0f), glm::vec2(0.0f, 0.0f)),
Vertex(glm::vec3(1.0f, -1.0f, 0.0f), glm::vec2(0.0f, 1.0f)),
Vertex(glm::vec3(-1.0f, -1.0f, 0.0f), glm::vec2(1.0f, 1.0f)),
Vertex(glm::vec3(-1.0f, 1.0f, 0.0f), glm::vec2(1.0f, 0.0f))
};

Render

void Render()
{
Sprite Background(Position, sizeof(Position) / sizeof(Position[0]));
Background.Draw();
}

I tried calling glUniform1i before glUseProgram but that did not fix the problem. Nevertheless I still appreciate you helping me out  :)

Edited by MaimaVanPersie

Share this post


Link to post
Share on other sites

Hmm, I can't really find any other problems just by looking over your code.

Could you try using VertexColors = vec4(TexCoord, 0.0, 1.0); in your fragment shader, to rule out texture coordinates as a problem? This should give you a fancy red/green gradient.

Do you have a debugger you could use to check whether the image data returned by SOIL_load_image is correct? If it has any variation in pixel values, it should be fine.

You could also try OpenGL's internal error logging functions. See here. If you want to test a part of your code, just call glGetError before it to clean up previous errors and call it again afterwards to get the error you are looking for.

Share this post


Link to post
Share on other sites

Could you try using VertexColors = vec4(TexCoord, 0.0, 1.0); in your fragment shader, to rule out texture coordinates as a problem? This should give you a fancy red/green gradient.

 

I still got a white screen (the two triangles). Looks like I'm having a texture coordinates problem?  

Share this post


Link to post
Share on other sites

If it is white, it indicates there is something wrong with your fragment shader or its output assignment. With that statement, vec4(TexCoord, 0.0, 1.0), the blue channel should be 0, resulting in any black/red/green/yellow tint, regardless of your texture coordinates.

Like the inputs of your vertex shader, the output of your fragment shader also needs to be mapped. In your case, you are writing directly to the backbuffer, which has 1 color output, index 0. For as far as i know, if you have one output variable in your fragment shader, that should automatically be bound to output 0, but this might be something worth checking. You can check here for more info on output assignments.

Once you have checked your output assignment, it probably helps to check if your shader program has any errors. See here and here.

Edited by Ignifex

Share this post


Link to post
Share on other sites

If it is white, it indicates there is something wrong with your fragment shader or its output assignment. With that statement, vec4(TexCoord, 0.0, 1.0), the blue channel should be 0, resulting in any black/red/green/yellow tint, regardless of your texture coordinates.

Like the inputs of your vertex shader, the output of your fragment shader also needs to be mapped. In your case, you are writing directly to the backbuffer, which has 1 color output, index 0. For as far as i know, if you have one output variable in your fragment shader, that should automatically be bound to output 0, but this might be something worth checking. You can check here for more info on output assignments.

Once you have checked your output assignment, it probably helps to check if your shader program has any errors. See here and here.

 

I did all that and it still did not report any errors. But I have been able to resolve the problem, I just needed to move the shader.create function from my Sprite constructor to my Sprite::Sprite(Vertex* vertices, unsigned int numVertices). *sigh* Thank you for your help, sorry if I exhausted you. 

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!