PDA

View Full Version : I'm trying to render a triangle



Tomyfr
11-09-2017, 06:07 AM
Hello all,

I'm very new on OpenGL and at this beginning I've found it very complex. I would think C++ is the most complex language but it's better.

Anyway, the code below is for rendering my first triangle. Please take a look:


#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <C:\Users\Abbasi\Desktop\std_lib_facilities_4.h>
using namespace std;


//*********************************

int main()
{
glfwInit();
glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 3);
glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 3);
glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);

GLFWwindow* window = glfwCreateWindow(800, 600, "The First Triangle", NULL, NULL);

if (window == NULL)
{
cout << "Failed to create GLFW window" << endl;
glfwTerminate();
return -1;
}

glfwMakeContextCurrent(window);


if (!gladLoadGLLoader((GLADloadproc)glfwGetProcAddres s))
{
cout << "Failed to initialize GLAD" << endl;
return -1;
}

glViewport(0, 0, 700, 500);

float vertices[] = {
-0.5f, -0.5f, 0.5f,
0.5f, -0.5f, 0.5f,
0.0f, 0.5f, 0.0f
};

unsigned int VBO; // Creating a vertex buffer object
glGenBuffers(1, &VBO);
glBindBuffer(GL_ARRAY_BUFFER, VBO);
glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

// Creating the Vertex Shader
const char* vertexShaderSource = "#version 330 core\nlayout (location = 0)"
"in vec3 aPos;\n\nvoid main()\n{\ngl_Position ="
"vec4(aPos.x, aPos.y, aPos.z, 1.0);\n}\n\0";

unsigned int vertexShader = glCreateShader(GL_VERTEX_SHADER);
glShaderSource(vertexShader, 1, &vertexShaderSource, nullptr);
glCompileShader(vertexShader);

//check the vertex shader compilation error(s)
int success;
char infoLog[512];
glGetShaderiv(vertexShader, GL_COMPILE_STATUS, &success);
if (!success)
{
glGetShaderInfoLog(vertexShader, 512, nullptr, infoLog);
cout << "ERROR::SHADER::VERTEX::COMPILATION_FAILED\n" << infoLog << endl;
}

// Creating the Fragment Shader
const char* fragmentShaderSource = "#version 330 core\n"
"out vec4 FragColor;\n\nvoid main()\n{\n"
"FragColor = vec4(1.0f, 0.5f, 0.2f, 1.0f);\n}\n\0";

unsigned int fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
glShaderSource(fragmentShader, 1, &fragmentShaderSource, nullptr);
glCompileShader(fragmentShader);

//check the fragment shader compilation error(s)
glGetShaderiv(fragmentShader, GL_COMPILE_STATUS, &success);
if (!success)
{
glGetShaderInfoLog(fragmentShader, 512, nullptr, infoLog);
cout << "ERROR::SHADER::FRAGMENT::COMPILATION_FAILED\n" << infoLog << endl;
}

// Linking both shaders into a shader program for rendering
unsigned int shaderProgram = glCreateProgram();
glAttachShader(shaderProgram, vertexShader);
glAttachShader(shaderProgram, fragmentShader);
glLinkProgram(shaderProgram);

//check the shader program linking error(s)
glGetProgramiv(shaderProgram, GL_LINK_STATUS, &success);
if (!success)
{
glGetProgramInfoLog(shaderProgram, 512, nullptr, infoLog);
cout << "ERROR::PROGRAM::SHADER::LINKING_FAILED\n" << infoLog << endl;
}

glUseProgram(shaderProgram);

// We no longer need the prior shaders after the linking
glDeleteShader(vertexShader);
glDeleteShader(fragmentShader);

glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void*)0);
glEnableVertexAttribArray(0);

unsigned int VAO;
glGenVertexArrays(1, &VAO);
glBindVertexArray(VAO);
glDrawArrays(GL_TRIANGLES, 0, 3);

system("pause");
return 0;
}

The output is:

2543

What is the problem of this code that doesn't show the triangle, please?

mhagain
11-09-2017, 07:07 AM
The most immediately and obviously wrong thing is this block of code:
glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void*)0);
glEnableVertexAttribArray(0);
unsigned int VAO;
glGenVertexArrays(1, &VAO);
What's wrong with this is that you actually don't have a VAO active when you make your glEnableVertexAttribArray and glVertexAttribPointer calls; this is illegal per core GL Specification; see https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glEnableVertexAttribArray.xhtml
GL_INVALID_OPERATION is generated by glEnableVertexAttribArray and glDisableVertexAttribArray if no vertex array object is bound.
GL_INVALID_OPERATION is generated by glEnableVertexArrayAttrib and glDisableVertexArrayAttrib if vaobj is not the name of an existing vertex array object.

Tomyfr
11-11-2017, 12:41 PM
Thanks for the answer.
By running the program I get no errors.
What's the simplest way to make the issue solved please?

DragonautX
11-12-2017, 12:51 PM
As mhagain mentioned, you need to have a currently bound vertex array object before you call commands like glEnableVertexAttribArray and glVertexAttribPointer. So you just need to generate a VAO and bind it before calling those commands.

You actually have a run-time error flagged by OpenGL (won't show in compilation). In your current code, after 'glEnableVertexAttribArray(0);', call glGetError() and check it's value. You'll get 1282 (that's what I see in my VS 2015 debugger in cases like these). In hex, this is 0x0502, and your glad.h labels this as GL_INVALID_OPERATION. As mhagain quoted, this GL_INVALID_OPERATION came from not binding a VAO before calling the two VertexAttrib commands.

Anyways, as for actually displaying the triangle, in this particular snippet, you'd also want to call glSwapBuffers() before your system("pause") call. Looking at your GLFW setup code, you didn't specify anything about whether you wanted a double-buffer or not, so GLFW gave a double-buffered OpenGL rendering context to you. The glDrawArrays command would then default to drawing to the back buffer, and you'd have to swap it out with the front to actually get it displayed.

So code-wise, after your last glDeleteShaders call, use this (I added comments where I thought would be helpful):



...
unsigned int VAO;
glGenVertexArrays(1, &VAO);
glBindVertexArray(VAO); //must have a VAO bound before callling VertexAttribPointer and EnableVertexAttribArray

glBindBuffer(GL_ARRAY_BUFFER, VBO);
glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void*)0);
glEnableVertexAttribArray(0);

glBindVertexArray(0); //I prefer to unbind OpenGL objects when I'm not using them, for consistency
glBindBuffer(GL_ARRAY_BUFFER, 0);

glBindVertexArray(VAO); //rebind VAO before calling glDrawArrays
glDrawArrays(GL_TRIANGLES, 0, 3);
glBindVertexArray(0);

glfwSwapBuffers(window); //
system("pause");

glfwTerminate(); //use to close window

//
//good practice to delete your OpenGL objects when you're sure you don't need them anymore
glDeleteBuffers(1, &VBO);
glDeleteVertexArrays(1, &VAO);
...


Hope that helps. That's an interesting approach to trying to display a triangle. I assume you're using the LearnOpenGL "Hello Triangle" tutorial. Would've thought you'd use a window while loop like the tutorial did, but guess you wanted to avoid the loop for now.

Tomyfr
11-13-2017, 12:17 PM
Hi, thanks for your answer.
The thing the is odd to me is binding something and unbinding it and then binding it again and unbinding. That seems rather repetitive.

I used this code: (although I don't think a professional programmer of OpenGL would also code a triangle that way)




#include <glad/glad.h>
#include <GLFW/glfw3.h>
#include <C:\Users\Abbasi\Desktop\std_lib_facilities_4.h>
using namespace std;

// Creating the Vertex Shader
const char* vertexShaderSource = "#version 330 core\nlayout (location = 0)"
"in vec3 aPos;\n\nvoid main()\n{\ngl_Position ="
"vec4(aPos.x, aPos.y, aPos.z, 1.0);\n}\n\0";

// Creating the Fragment Shader
const char* fragmentShaderSource = "#version 330 core\n"
"out vec4 FragColor;\n\nvoid main()\n{\n"
"FragColor = vec4(1.0f, 0.5f, 0.2f, 1.0f);\n}\n\0";

//*********************************

int main()
{
glfwInit();
glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 3);
glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 3);
glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);

GLFWwindow* window = glfwCreateWindow(800, 600, "The First Triangle", nullptr, nullptr);
glfwMakeContextCurrent(window);
gladLoadGLLoader((GLADloadproc)glfwGetProcAddress) ;

float vertices[] = {
-0.5f, -0.5f, 0.5f,
0.5f, -0.5f, 0.5f,
0.0f, 0.5f, 0.0f
};

unsigned int VBO;
glGenBuffers(1, &VBO);
glBindBuffer(GL_ARRAY_BUFFER, VBO);
glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

unsigned int vertexShader = glCreateShader(GL_VERTEX_SHADER);
glShaderSource(vertexShader, 1, &vertexShaderSource, nullptr);
glCompileShader(vertexShader);

unsigned int fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
glShaderSource(fragmentShader, 1, &fragmentShaderSource, nullptr);
glCompileShader(fragmentShader);

// Linking both shaders into a shader program for rendering
unsigned int shaderProgram = glCreateProgram();
glAttachShader(shaderProgram, vertexShader);
glAttachShader(shaderProgram, fragmentShader);
glLinkProgram(shaderProgram);

glUseProgram(shaderProgram);

unsigned int VAO;
glGenVertexArrays(1, &VAO);
glBindVertexArray(VAO); //must have a VAO bound before callling VertexAttribPointer
//and EnableVertexAttribArray

glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void*)0);
glEnableVertexAttribArray(0);
glDrawArrays(GL_TRIANGLES, 0, 3);

glfwSwapBuffers(window);
system("pause");
glfwTerminate();
return 0;
}

mhagain
11-13-2017, 12:29 PM
Unbinding is something that you only see in relatively recent tutorials, and is not actually necessary at all. I would personally be happier if tutorial writers didn't unbind, but instead taught how the API actually works, which calls interact with currently bound objects, and what the correct states and bindings you need for a given API call are. IMO, unbinding is bad practice that can hide programmer errors; you should be wanting to expose those errors instead so that you can correct them and learn from them.

DragonautX
11-13-2017, 08:53 PM
(to mhagain) That's interesting. Unbinding often felt natural to me. I also recently learned in my programming class about "avoiding side effects" when writing functions, like for example making sure functions avoid permanently modifying global variables. I'd consider OpenGL state to be something at the global scope, and I try to not modify it permanently in a function when I feel it's good to do so. Like maybe in a drawing function, I'd bind a VAO when I need to call DrawArrays, and before exiting the function, I'd unbind it.

Would you consider that good design, or unnecessary with OpenGL state? You mentioned that unbinding "can hide programmer errors". What is an example of that? Looking back at glGetError, I think it's keeps an error code regardless of object binding, so I'd say you'd still be able to catch an error with it even after you unbind.

DragonautX
11-13-2017, 09:51 PM
(to Tomyfr) Rebinding like that isn't required. It's more of my own design style. You can remove the extra bindings as you need.

mhagain
11-14-2017, 01:10 AM
OpenGL state is global, yes.

The cleaner design is to bind everything you need at the start of each drawing or creation function, but never unbind. Using that design, each function can be absolutely certain that it has it's correct state, that it doesn't inherit unwanted state from elsewhere, but yet you're also helping the driver's ability to batch or filter redundant state changes. You can even write your own redundant state change filtering system using this design.

This approach also mirrors the way newer APIs like Vulkan work, where you have monolithic state objects instead of fine-grained individual states. It works better with DSA OpenGL, it works better with D3D.

DragonautX
11-14-2017, 10:50 PM
Alright, I can see the benefits of that. If I were to only bind in two different functions that I define, and I bind the same object, sounds reasonable for drivers to be able to avoid a duplicate bind. It'd also make me think more before writing my own GL function. If I'm already comfortable with the GL functions I plan to use, that could be a good idea. Never tried Vulkan, D3D, or OpenGL DSA (my card only supports GL 4.0 too, can't try it anyways), but I'll take your word at API consistency.

How would a redundant sate change filtering system work? Reading that, I'd think that maybe I could check for certain state, and from that, conclude that I don't need to modify other state. Is that what you meant?

---

Looking back at what you said about unbinding and hiding programming errors, I can see how keeping objects binded would make errors more apparent. Maybe if I forgot to bind a VAO for drawing a square in one function and called a previous function that bounded a VAO for a triangle, I'd probably see an unusual number of triangles, would be an easy error flag to see. Overall, I can see it plain binding as a useful idea. I'll give it a try, thanks.

john_connor
11-15-2017, 12:49 AM
How would a redundant state change filtering system work?

somehow like this:

/* describe the current gl state completely */
struct RenderState
{
GLuint program_used, vao_used, xfb_used, etcpp;
bool depth_testing, culling, wireframe_mode, blending, etc;
};


class RenderState_Manager
{
private:
RenderState current_state;

public:

void SetNewState(const RenderState& state)
{
if (state.depth_testing != current_state.depth_testing)
{
/* change state */
state.depth_testing ? glEnable(GL_DEPTH_TEST) : glDisable(GL_DEPTH_TEST);
}

/* ... do the same for each remaining GL state accordingly ... */

/* when everything is set, "cache" the current state */
current_state = state;
}

};


more advanced way would be to sort the states in a way that allows you to minimize the number of different states needed to draw everything.

DragonautX
11-15-2017, 10:09 AM
That's a nice idea, thanks. So basically, for any requested state, check if that state is already set, and if not, then set it. At first, I'd think it think it'd be less efficient since you may do 2 operations (conditional check, then possible state setting), but I can see how this can actually be more efficient if the client and server are connected via a slow network and a repetitive state setting function can take more time than just checking state in the client.

Sorting the states is also a nice idea. Maybe if I had two parts of my code that drew pure triangles (they'd just differ by what's passed to the DrawArrays command), and one part that draws a square, I could put the two triangle code snippets together and only bind a triangle VAO once, and put the square snippet afterwards. Seems less flexible than the filtering system, but it's still interesting. I'll think about trying the filtering system in the future. Anyways, thanks for the help.

john_connor
11-15-2017, 05:01 PM
you may want to check that out:
https://www.sfml-dev.org/documentation/2.4.2/classsf_1_1RenderStates.php
https://www.sfml-dev.org/documentation/2.4.2/classsf_1_1RenderTarget.php#a12417a3bcc245c41d957b 29583556f39

consider each renderstate as "key value" in a std::map<...> or just an array of these, each state has its "bunch of drawcalls" (batch), by sorting a new drawcall (mesh) to existing states, you can avoid setting a particular state twice (or multiple times). a new drawcall (that is the arguments of any glDraw*(...)) can also be "packed" into structures, take a look at that:
https://sites.google.com/site/john87connor/other/draw-commands

by the way:
in some of my examples i do explicitly "unbind" everything each frame, i do that to make it clear what changes have been made and how to reverse them, not for performace-reasons, but to make the code flow more "apparent" or easier to understand for beginners .. (i hope) maybe i'll comment them out soon ..

DragonautX
11-16-2017, 10:47 PM
I like the idea. Had to think about it for a bit. So you're saying like have some form of list of renderstates, and somehow associate these renderstates with certain draw calls. Then somehow write a function that says something like, "Given a renderstate, do all the draw calls associated with it." Which can be considered like a batch drawing command. Packing drawing arguments is creative too, I've seen that in Windows API a lot, esp. with creating windows.

I'll keep a note of SFML and your google site. Heard of SFML, but wanted to go very low-level, so went for OpenGL. And it'd be cool to look at your site to see how other people code OpenGL. Anyways, thanks again.