Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MemoryStack api for OpenGL and document it for SimpleDrawElements #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use MemoryStack api for OpenGL and document it for SimpleDrawElements #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bca393
Use MemoryStack api for OpenGL and document it
Bawbby Oct 20, 2018
edd0859
Undo import order and update notes about Buffer allocation
Bawbby Oct 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions src/org/lwjgl/demo/opengl/SimpleDrawElements.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/
package org.lwjgl.demo.opengl;

import org.lwjgl.BufferUtils;
import org.lwjgl.glfw.*;
import org.lwjgl.opengl.*;
import org.lwjgl.system.*;
Expand All @@ -26,6 +25,8 @@ public class SimpleDrawElements {

// The window handle
private long window;

// Window size
private int width, height;

public void run() {
Expand All @@ -37,7 +38,7 @@ public void run() {
glfwDestroyWindow(window);
keyCallback.free();
wsCallback.free();
if (debugProc != null)
if ( debugProc != null )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change formatting?

Copy link
Author

@Bawbby Bawbby Oct 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses both with spaces and no spaces between parens, I just changed formatting to make it consistent. I can revert it if needed.

debugProc.free();
} finally {
// Terminate GLFW and release the GLFWerrorfun
Expand All @@ -49,7 +50,7 @@ public void run() {
private void init() {
// Setup an error callback. The default implementation
// will print the error message in System.err.
glfwSetErrorCallback(errorCallback = GLFWErrorCallback.createPrint(System.err));
errorCallback = GLFWErrorCallback.createPrint().set();

// Initialize GLFW. Most GLFW functions will not work before doing this.
if ( !glfwInit() )
Expand Down Expand Up @@ -81,7 +82,7 @@ public void invoke(long window, int key, int scancode, int action, int mods) {
glfwSetWindowSizeCallback(window, wsCallback = new GLFWWindowSizeCallback() {
@Override
public void invoke(long window, int w, int h) {
if (w > 0 && h > 0) {
if ( w > 0 && h > 0 ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting?

width = w;
height = h;
}
Expand All @@ -90,7 +91,7 @@ public void invoke(long window, int w, int h) {

GLFWVidMode vidmode = glfwGetVideoMode(glfwGetPrimaryMonitor());
glfwSetWindowPos(window, (vidmode.width() - width) / 2, (vidmode.height() - height) / 2);
try (MemoryStack frame = MemoryStack.stackPush()) {
try ( MemoryStack frame = MemoryStack.stackPush() ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting.

IntBuffer framebufferSize = frame.mallocInt(2);
nglfwGetFramebufferSize(window, memAddress(framebufferSize), memAddress(framebufferSize) + 4);
width = framebufferSize.get(0);
Expand All @@ -117,15 +118,30 @@ private void loop() {
// Set the clear color
glClearColor(1.0f, 0.0f, 0.0f, 0.0f);

// Generate vertex and index buffers
int vbo = glGenBuffers();
int ibo = glGenBuffers();

// Vertex and index data
float[] vertices = {-0.5f, -0.5f, 0.5f, -0.5f, 0.5f, 0.5f};
int[] indices = {0, 1, 2};
glBindBuffer(GL_ARRAY_BUFFER, vbo);
glBufferData(GL_ARRAY_BUFFER, (FloatBuffer) BufferUtils.createFloatBuffer(vertices.length).put(vertices).flip(), GL_STATIC_DRAW);
glEnableClientState(GL_VERTEX_ARRAY);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, (IntBuffer) BufferUtils.createIntBuffer(indices.length).put(indices).flip(), GL_STATIC_DRAW);

// Here we need to send the vertex and index data to OpenGL via Buffers.
// We will use the MemoryStack api as this is a small amount of data
// and is not short lived. But if larger data and/or data that is longer lived
// is needed, It is recommended to use the memAlloc/memFree methods in the MemoryUtil class
// Note: Before sending off data, call the flip() method to finish putting data in
try ( MemoryStack stack = MemoryStack.stackPush() ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using the MemoryStack for buffer data upload is stretching it a bit. In this example, since it is only 3 vertices it's okay, but it might give the wrong idea to people. The problem is that MemoryStack is not meant for potentially large buffer data, which one usually has when uploading models/vertices. A better alternative for this is manual Buffer memory management with memAlloc/memFree.

Copy link
Author

@Bawbby Bawbby Oct 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had forgotten to state why I used the MemoryStack vs other methods. I'll add a note about using different methods for allocating buffers and the reasons why.

glBindBuffer(GL_ARRAY_BUFFER, vbo);
FloatBuffer verticesBuffer = (FloatBuffer) stack.mallocFloat(vertices.length).put(vertices).flip();
glBufferData(GL_ARRAY_BUFFER, verticesBuffer, GL_STATIC_DRAW);

glEnableClientState(GL_VERTEX_ARRAY);
IntBuffer indicesBuffer = (IntBuffer) stack.mallocInt(indices.length).put(indices).flip();
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, indicesBuffer, GL_STATIC_DRAW);
}
// Specifies number of coordinates per vertex, in this case there are 2 and of type GL_FLOAT
glVertexPointer(2, GL_FLOAT, 0, 0L);

// Run the rendering loop until the user has attempted to close
Expand All @@ -135,7 +151,7 @@ private void loop() {

glViewport(0, 0, width, height);
glMatrixMode(GL_PROJECTION);
float aspect = (float)width/height;
float aspect = (float) width/height;
glLoadIdentity();
glOrtho(-aspect, aspect, -1, 1, -1, 1);
glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_INT, 0L);
Expand Down