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.xml4) 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.