Suggestions (to LWJGL team)

Hi! I have examined your binding and I have some suggestions on how to make it better. There are some things that I consider very weird.

  1. There are no functions like
glVertex3fv(float[])

or like

glMaterialfv(...)

. I would rather prefer to use arrays than a “float, float, float” chain or Buffers in this case.
2. Why do use subclasses of Buffer rather than the Buffer itself? It is very strange.
3. Why don’t you make a private constructor in GL11-15 classes and all other utility classes. They shouldn’t have a public constructor and shouldn’t be instatiated as all methods and fields are static!
4. Why do use only the ByteBuffer in gluBuild2DMipmaps function?
5. Why did you change GL names? I find it very inconvenient

[quote]Hi! I have examined your binding and I have some suggestions on how to make it better. There are some things that I consider very weird.
[/quote]
Hi, and thanks for your comments. LWJGL has evolved a long way since its first inception. The way it is today is basically because it feels right, for the majority of the developers. That said, there will be some that want it to behave differently - we can’t please everybody :wink:

[quote]1. There are no functions like

glVertex3fv(float[])

or like

glMaterialfv(...)

. I would rather prefer to use arrays than a “float, float, float” chain or Buffers in this case.
[/quote]
Yes, it is more convenient - unfortunately this is exactly how you kill performance. When OpenGL renders, it wants its data fast. By using arrays you are forced to move data from within the VM which is considerably slower than using a direct ByteBuffer or variants thereof. See 5 too.

[quote]2. Why do use subclasses of Buffer rather than the Buffer itself? It is very strange.
[/quote]
We use subclasses of bytebuffers so that we know the datatype, and thus the offset into the buffer for elements.

[quote]3. Why don’t you make a private constructor in GL11-15 classes and all other utility classes. They shouldn’t have a public constructor and shouldn’t be instatiated as all methods and fields are static!
[/quote]
Sure we could do that… but I actually don’t see any user benefit from doing it… you can’t use a GL11 instance for anything and it’s a final class anyway - it would simply be for the sake of being more OOish

[quote]4. Why do use only the ByteBuffer in gluBuild2DMipmaps function?
[/quote]
See 1. Why not? - an image is just bytes - why wrap it in something else?

[quote]5. Why did you change GL names? I find it very inconvenient
[/quote]
The names that were changed was because they didn’t make sense in Java (they were removed entirely) or because the actual purpose of the method [gl*f, i, b] was implied by the type of Buffer supplied (FloatBuffer (f), IntBuffer(i), ByteBuffer(b)).

Actually the constructors should indeed be private to avoid any confusion.

Cas :slight_smile:

Orion,

Some people here, myself included, have developed simple wrapper classes that restore the functionality as you described, I found this very useful when porting code from the many C++ OpenGL examples on the web.

There are a few tricky ones though that you need to watch out for.


public static void glVertexPointer( int size, int type, int stride, float[] buffer ) {
  glVertexPointer( size, stride, toBuffer( buffer ) );
}

public static void glColorPointer( int size, int type, int stride, float[] buffer ) {
  glColorPointer( size, stride, toBuffer( buffer ) );
}

public static void glTexCoordPointer( int size, int type, int stride, float[] buffer ) {
  glColorPointer( size, stride, toBuffer( buffer ) );
}

These methods don’t execute immediately, the information is only used when a glDrawArrays call is made, so if you use both color and texture, you need 2 buffers to hold the data, rather than just reusing 1.

This was covered in a previous topic, here:-

http://www.java-gaming.org/cgi-bin/JGNetForums/YaBB.cgi?board=LWJGL;action=display;num=1077796504;

Andy.

Thanks for answering so soon! I understand now what makes you use such methods.

I’ve now added private constructors to all OpenAL and OpenGL static classes in CVS.

  • elias

[quote]I’ve now added private constructors to all OpenAL and OpenGL static classes in CVS.

  • elias
    [/quote]
    This makes sense, but unfortunately it seems to break spgl’s AL class, which subclasses the org.lwjgl.openal.AL class.

Although it is otherwise a static class, it seems Java still wants the spgl class to inherit the constructor from its superclass.

I’d suggest it would be better to either make the org.lwjgl.openal.AL constructor protected, so subclasses can still implicitly inherit it, or remove it completely, since it will never be called anyway.

Go for protected for all of them. There’s no way of getting confused and instantiating them without some odd-looking syntax (which will tip most people off), but it still allows for subclassing.

Hm. Why does spgl’s AL have to extend AL?

  • elias

seems to be adding some convenience methods which may have been useful at some stage, but I can’t see that they’re adding much to the spgl API…

still, the fact is that it’s more correct to have a protected than a private constructor unless the class is declared final.

IF AL, GL1x, etc really shouldn’t be subclassed, they should probably be declared final for the sake of consistency. But then I’ve always been a pedantic sod :wink:

SPGL’s AL shouldn’t really be subclassed, it’s just an anachronism…

Cas :slight_smile:

Besides, about the buffers. I have a class for textures that (apart from all else) is capable of loading an image from a file. I have a field in this class that is aimed for storing the image information. And it’s type is Buffer (superclass for ALL Buffers). It’s very convenient to represent data this way, because this Buffer object can be an istance of either ByteBuffer, DoubleBuffer, FloatBuffer, or any other subclass. (as image data tends to be stored in various Java types). In C/C++ you can just have a void* field and it will be capable to store data of any type. But since we use Java, you can’t store data in void array. So, what am I talking about? Well, it would be a LOT more convenient to use the Buffer superclass in all methods of LWJGL. Because if I had to use a subclass, I would do the following, which is a very nerve-racking task:


if(someBuffer instanceof ByteBuffer)
   //pass it as a ByteBuffer argument 

if(someBuffer instanceof DoubleBuffer)
   //pass it as a DoubleBuffer argument 

etc.

But if LWJGL would have support for methods with Buffer only arguments, my code would be a lot shorter. And besides, this is how it’s done in JOGL.
So, why don’t you check for the buffer type internally? I really hope you consider my advice!

P.S: if you have problems with private constructors, check Joshua Bloch’s “Effective Java” book.

Zeph: The GL* and AL* classes are final! That’s what they’re meant to be anyway, so tell me if I forgot some (a few are not, like the ARBProgram class which is extended by ARBVertexProgram and ARBFragmentProgram. This is intended, and ARBProgran does indeed have a protected constructor (it has to)).

Orion: We’ve chosen not to use the instanceof “switch” in handling buffers. And LWJGL needs the specific type, because LWJGL, unlike JOGL, honors remaining() and position() (for which we need the element size of the buffer).

  • elias

Oh well, AL isn’t final (why could SPGL extend it if it was not). It’s fixed now.

  • elias

well. crisis averted… we now return to our regularly scheduled programming! :wink:

2 Elias: but what’s so wrong with the instanceof operator???

it’s farking slow!
besides, it would bloat LWJGL and make it slower for everybody - if you need this feature you can easily wrap it yourself.

It’s a really weird feature to ask for… I can’t think of any task that could possibly require it that could not be designed better so that it wasn’t needed.

Cas :slight_smile:

Look. I don’t know which data will be stored in my buffer (bytes, floats, doubles). So I’ll have to call glTexImage2D for every different Buffer type (6 or more times), which would look like:



// if type == ByteBuffer 
 GL11.glTexImage2D(GL11.GL_TEXTURE_2D, 0, txt.getComponentCount(), 
txt.getWidth(), txt.getHeight(), 0, fmt, typ, (ByteBuffer)txt.getData());

//if type == FloatBuffer

 GL11.glTexImage2D(GL11.GL_TEXTURE_2D, 0, txt.getComponentCount(), 
        txt.getWidth(), txt.getHeight(), 0, fmt, 
        typ, (FloatBuffer)txt.getData());

//if type == DoubleBuffer

 GL11.glTexImage2D(GL11.GL_TEXTURE_2D, 0, txt.getComponentCount(), 
        txt.getWidth(), txt.getHeight(), 0, fmt, 
        typ, (DoubleBuffer)txt.getData());


This is VERY ugly. How can I possibly avoid this?