2D Lighting and Shadows

[quew8]

Firstly, really cool video. I'm really tempted to implement my own version of this it looks so cool. (But I'm not going to because I'm not making any excuses to avoid my current project).

Secondly, as per your request, some ideas for improving the code:

1) You are right about the glBegin() / glEnd(). It is called immediate mode (since the primitives get drawn immediately after you call glEnd() ). And it's the slowest way of drawing anything, so much so that it is deprecated in modern OpenGL and doesn't exist in GLES 2.0+. As a very (very very very) general rule of thumb, the more OpenGL methods you call, the slower your program is going to be. So I think you should move on up to either Vertex Arrays (not to be confused with Vertex Array Objects) or Vertex Buffer Objects. Here's a tutorial for VBOs http://www.lwjgl.org/wiki/index.php?title=Using_Vertex_Buffer_Objects_(VBO). The idea is that you fill a buffer with the vertex data and then send that to OpenGL in one go.

The tutorial if for VBOs but with Vertex Arrays you can just skip to the rendering part, don't bind the buffers (since there aren't any) and pass the glXXXPointer() function the FloatBuffer with the data in rather than a buffer offset. Then with glDrawElements() give it the IntBuffer containing the indices instead of a buffer offset. BTW these buffers are all java.nio. but create them with LWJGL BufferUtils class (because they have to be direct)

Some example code for Vertex Arrays since I made no sense above. It draws a funky looking cross:

            FloatBuffer vertexBuffer = BufferUtils.createFloatBuffer(12 * 2); //Contains the position data. You should be reusing these really
            vertexBuffer.put(new float[] {
                25, 45, //0
                45, 65, //1
                65, 45, //2
                45, 25, //3
                25, 85, //4
                45, 105, //5
                65, 85, //6
                85, 105, //7
                105, 85, //8
                85, 65, //9
                105, 45, //10
                85, 25, //11
            });
            vertexBuffer.flip();
            
            FloatBuffer colourBuffer = BufferUtils.createFloatBuffer(12 * 3); //Contains the colour data
            colourBuffer.put(new float[] {
                0, 1, 0, //0
                0, 0, 1, //1
                1, 0, 1, //2
                0, 1, 0, //3
                0, 1, 0, //4
                0, 1, 0, //5
                1, 0, 1, //6
                0, 1, 0, //7
                0, 1, 0, //8
                0, 0, 1, //9
                0, 1, 0, //10
                0, 1, 0, //11
            });
            colourBuffer.flip();
            
            IntBuffer indexBuffer = BufferUtils.createIntBuffer(20); //The order to draw each index
            indexBuffer.put(new int[] {
                0, 1, 2, 3, // Lower Left
                4, 5, 6, 1, // Upper Left
                7, 8, 9, 6, // Upper Right
                10, 11, 2, 9, // Lower Right
                1, 6, 9, 2, // Middle
            });
            indexBuffer.flip();
            
            glEnableClientState(GL_VERTEX_ARRAY);
            glEnableClientState(GL_COLOR_ARRAY);
            
            glVertexPointer(2, 8, vertexBuffer); // Tells OpenGL where to find vertex data. Count is the number of elements per data (2 
                                                            // floats per position) and stride is the byte offset from the start of one piece of data to the
                                                            // next. ( 2 floats = 2 x 4 = 8 )
            glColorPointer(3, 12, colourBuffer); // Same as above but for colour. 3 floats (RGB) and so stride is 3 x 4 = 12.

            glDrawElements(GL_QUADS, indexBuffer); //Draw the elements pointed to by the indices in indexBuffer as Quads.
            
            glDisableClientState(GL_COLOR_ARRAY); // If you are going to do any drawing not using vertex arrays then you must disable 
            glDisableClientState(GL_VERTEX_ARRAY); //them after drawing.

Here's a link to a pastebin of an example program you can just run and then mess around with as you like. http://pastebin.com/SHKUt4EM

More to come.

[quew8]

2) A (probably, I've never actually done this before) better way to disable drawing to the colour buffer is to call glDrawBuffer(GL_NONE); Then to enable drawing again, glDrawBuffer(GL_FRONT); The reason I say probably is that the front / back buffers are used for double buffering. So if you set it to GL_FRONT when it should be on GL_BACK, it might mess up the double buffering. A solution to this is below as part of the next improvement.

3) OpenGL is a state machine (This is one of the first lines of the red book proper so I say it whenever I get the chance). So you have to change state a lot. If you can make this more efficient then you're laughing. The way OpenGL lets you do this is the same as with efficient Matrix transforms (I don't know if you know about this so I'll just explain it from scratch). There is an stack of states, which you can push to and pop from. You can even be selective about which parts of the state you push (and implicitly therefore pop). So, set the state to the default, draw stuff like that, push the attribs you are about to change, change them, draw stuff like that and then you can just pop them rather than resetting them all individually.

Pseudo code:

//IN INIT CODE - This sets up the state in which you draw the lights "effect".
glEnable(GL_STENCIL_TEST);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
glStencilFunc(GL_EQUAL, 0, 1);
//glColorMask(true, true, true, true); No longer needed as per above
glEnable(GL_BLEND);
glBlendFunc(GL_ONE, GL_ONE);

//IN RENDER CODE
for(Light l: lights) {
    glPushAttrib( GL_COLOR_BIT | GL_STENCIL_BIT | GL_ENABLE_BIT ); // "Saves" this state.
    glDrawBuffer(GL_NONE);                                                           //}
    glStencilFunc(GL_ALWAYS, 1, 1);                                               //} Sets up the state to draw shadows.
    glStencilOp(GL_KEEP, GL_KEEP, GL_REPLACE);                              //}
    glDisable(GL_BLEND);                                                               //}
    for(Block b: blocks) {
        drawTheShadow();
    }
    glPopAttrib();                                                                         // "Loads" the "saved" state.
    drawTheLightsEffect();
}
glPushAttrib( GL_STENCIL_BUFFER_BIT );                                        // "Saves" the state of the stencil test. (I think that's all you
                                                                                               // need)
glDisable(GL_STENCIL_TEST);                                                        // Sets up the state for rendering the blocks themselves.
for(Block b: blocks) {
    drawTheBlock();
}

glPopAttrib();                                                                             // And "loads" it back up again.

Those comments were meant to be all level but obviously they aren't. Sorry. For a full list of what those constants for glPushAttrib() do (and what other possibilities there are) see the documentation: http://www.opengl.org/sdk/docs/man2/xhtml/glPushAttrib.xml

4) Not about OpenGL but I often hear that it is bad practice to use the "for each" loops in a game loop. It apparently creates a lot of garbage for the collector to collect. So I recommend using the regular "for" loops. I know it's annoying but apparently it makes a difference.

5) You seem to create a lot of unnecessary vectors each loop. Mainly I'm talking about Block's getVertices() method which not only creates a new Vertex2f[] each game loop but also the Vector2f s themselves. My humble advice, just cache them as a final field of Block and pass that on. Also for the maths part of the shadow algorithm, you could reuse all those vectors either by initializing them as final variables outside the loop or using some kind of pool. Nice(ish) tutorial here: http://www.java-gaming.org/topics/object-pooling/27133/view.html 

That last one was a bit picky I know but it's good not to get into bad habits. That's all for now folks. Hope it helps and at least some of it makes sense.

[DrAgonmoray]

Wow, thanks! 1 and 3 clear up a lot of questions I've been having, and 4 is some useful information in general.

As for the last one, I know, and I did it that way intentionally. It's a lot of heavy vector math, so I wanted to keep it all in that area and use a bunch of variables to help explain it until I can imorove and document the rest of the code.
However, I hadn't thought of the other things you said to fix it (like pools) so thanks!


I'm gonna add in an FPS counter and see how much improvement I can get from some of these fixes!

[quew8]

My advice is don't spend too long trying to optimize at this early stage. Implement the fixes and see if they increase the frame rate. If they do then great, if they don't then (think about if for a couple minutes first) just drop that and move on. You can spend days trying to get an extra 0.5 frames per second then you implement a new feature that takes it down by 80. Write the whole application then figure out where the bottlenecks are and address them one at a time until the thing goes fast enough.

The absolute worst thing you can do is optimize it now, then come back later to add something only to find you can't decipher any of the code because it has been so heavily optimized.