API comments

Started by NateS, October 24, 2010, 01:22:27

Previous topic - Next topic

NateS

A friend of mine had these comments on LWJGL. What do you guys think?

- why do those freaking methods take ByteBuffer, FloatBuffer and DoubleBuffer only? Why not just Buffer? As they all have to be direct buffers it doesn't matter.
- GL13.glCompressedTexImage2D() has two methods, one with a long pointer and one with a buffer. In case of the buffer the imageSize is ignored. So where does the method get that info from?
- GL13.glCompressedSubTexImage2D(), same shit
- GL11.glDrawElements(), again seperate methods for typed Buffers. You can't specify a count with those. So i assume the limit of the buffer is taken instead. Awesome! Seriously...
- GL11.glNormalPointer() can either be specified via a long pointer and a stride or via a Buffer in which ase no stride can be given. So how do interleaved vertex arrays work then if the pointer version only works with VBOs?
- GL11.glTexCoordPointer has a stride but no ByteBuffer variant? Wtf?

jediTofu

Why does your friend sound angry?

I would think that the use of ByteBuffer/FloatBuffer/etc. is fairly simple.  The methods need an array of type float, type byte, etc.  They can't just use any type of buffer.  If there are overloaded methods (for example, one method that has ByteBuffer/FloatBuffer/etc.), I would think that it's because different types need to be handled differently and for speed (don't have to check what type of buffer it is, etc.).
cool story, bro

NateS

Quote from: jediTofu on October 24, 2010, 01:46:38
Why does your friend sound angry?

Oh don't mind that, he is just very... expressive. :)

Quote
I would think that the use of ByteBuffer/FloatBuffer/etc. is fairly simple.  The methods need an array of type float, type byte, etc.  They can't just use any type of buffer.  If there are overloaded methods (for example, one method that has ByteBuffer/FloatBuffer/etc.), I would think that it's because different types need to be handled differently and for speed (don't have to check what type of buffer it is, etc.).

Well, it leads to this kind of code (an OpenGL ES API that uses LWJGL):

public void glBufferData (int target, int size, Buffer data, int usage) {
	if (data instanceof ByteBuffer)
		GL15.glBufferData(target, (ByteBuffer)data, usage);
	else if (data instanceof IntBuffer)
		GL15.glBufferData(target, (IntBuffer)data, usage);
	else if (data instanceof FloatBuffer)
		GL15.glBufferData(target, (FloatBuffer)data, usage);
	else if (data instanceof DoubleBuffer)
		GL15.glBufferData(target, (DoubleBuffer)data, usage);
	else if (data instanceof ShortBuffer) //
		GL15.glBufferData(target, (ShortBuffer)data, usage);
}

jediTofu

That code will be slower.  Granted, we're probably talking about a few 0.00001 seconds, but if that's called 1,000 times in a continuous game loop, the programmer should have the option to opt for the faster direct methods.  Maybe that method could be added to GLXX or to a util package as an alternative.  I don't build the code, so maybe it's easier to have th overloaded methods during code generation?  Personally, I think that you should call the appropriate methods directly and not use if statements and instanceof; is it mandatory in your code to have this?  Just my opinion though  :-\
cool story, bro

NateS

Quote from: jediTofu on October 24, 2010, 02:01:13
Personally, I think that you should call the appropriate methods directly and not use if statements and instanceof; is it mandatory in your code to have this?

This is for an OpenGL ES wrapper. LWJGL is used on the desktop, OpenGL ES is used on Android. So the method definition "void glBufferData (int target, int size, Buffer data, int usage)" has to stay, the LWJGL API can't be used directly. The issue is that the LWJGL API forces the if statements.

It is workable as-is, but I thought I'd put this out there and see what people thought about improvements.

jediTofu

QuoteGL13.glCompressedTexImage2D() has two methods, one with a long pointer and one with a buffer. In case of the buffer the imageSize is ignored. So where does the method get that info from?

I assume from either Buffer.array().length or Buffer.capacity();.  Also, with the parameters passed (width, height, border), you can probably figure it out.  I think that's all the questions I can answer.
cool story, bro

NateS

Here is another example:

public final void glColorPointer (int size, int type, int stride, Buffer pointer) {
	if (pointer instanceof FloatBuffer && type == GL10.GL_FLOAT)
		GL11.glColorPointer(size, stride, (FloatBuffer)pointer);
	else if (pointer instanceof ByteBuffer && type == GL10.GL_FLOAT)
		GL11.glColorPointer(size, stride, ((ByteBuffer)pointer).asFloatBuffer());
	else if (pointer instanceof ByteBuffer && type == GL10.GL_UNSIGNED_BYTE)
		GL11.glColorPointer(size, true, stride, (ByteBuffer)pointer);
	else
		throw new RuntimeException("Can't use " + pointer.getClass().getName() + " with this method!");
}


The asFloatBuffer is required since LWJGL won't take a Buffer and an int type. This allocates memory in the game loop. :(

jediTofu

Why don't you have (or the OpenGL ES have) overloaded methods like this (to get rid of the if statements and instanceof's)?

public void glBufferData (int target, int size, FloatBuffer data, int usage)
public void glBufferData (int target, int size, ByteBuffer data, int usage)
public void glBufferData (int target, int size, IntBuffer data, int usage)
public void glBufferData (int target, int size, DoubleBuffer data, int usage)

FloatBuffer myFloatBuffer;
glBufferData(target,size,myFloatBuffer,usage); //automatically calls FloatBuffer method

As for glColorPointer, there seems no way out since you need to test for the type; you'll have to have an if statement.  If LWJGL were to provide these methods, they would be done in the same way (with if statements), but I guess it would be more convenient so that you wouldn't have to write them.
cool story, bro

NateS

Quote from: jediTofu on October 24, 2010, 02:31:52
Why don't you have (or the OpenGL ES have) overloaded methods like this (to get rid of the if statements and instanceof's)?

Why does LWJGL deviate from the C API it is wrapping and force a type on the buffer?

Quote
As for glColorPointer, there seems no way out since you need to test for the type; you'll have to have an if statement.  If LWJGL were to provide these methods, they would be done in the same way (with if statements), but I guess it would be more convenient so that you wouldn't have to write them.

I'm pretty sure LWJGL does something like this, in psuedo native code:

public final void glColorPointer (int size, int type, int stride, Buffer pointer) {
	glColorPointer(size, stride, GL_FLOAT, pointer);
}


There are no if statements needed.

I think it is great for LWJGL to provide typed methods so you don't have to specify the type, but it should also provide a method that takes java.nio.Buffer and an int type.

Matthias

All glXYZPointer accept a stride parameter - I suggest you read the API carefully.

When you use a XYZBuffer LWJGL will take the position and the limit into account, but both are given in units of the buffer. So LWJGL needs to multiply these by the element size (which you can easily see by looking at the code) so you would still need the instanceof check which is slow.

Even if your OpenGL ES binding uses methods with type Buffer you can still use it correctly in your app (eg by calling it with a typed Buffer).

Methods taking a long "offset" as last parameter are to be used with VBO (Vertex Buffer Objects) - the offset is into the bound buffer object.

Not all glXYZPointer have all types - only the useful types are provided. You can always use shaders and glVertexAttribPointer.

And by using NIO Buffers you get the position, limit and type in a consistent form. This does not "derivate" from the C API. It offers all possibilities in a type safe way.

NateS

Why not do it both ways? Why not mimic the C API 100% in addition to the potentially more convenient type buffer methods?

LWJGL touts the ability to paste in C code without edits using static imports.

spasi

Quote from: NateS on October 24, 2010, 01:22:27- GL13.glCompressedTexImage2D() has two methods, one with a long pointer and one with a buffer. In case of the buffer the imageSize is ignored. So where does the method get that info from?
- GL13.glCompressedSubTexImage2D(), same shit
- GL11.glDrawElements(), again seperate methods for typed Buffers. You can't specify a count with those. So i assume the limit of the buffer is taken instead. Awesome! Seriously...

The image size is not ignored in the buffer case. It is automatically passed to the native code as the value of data.remaining(). Most LWJGL methods (like the other 2 examples you mention) that accept a buffer argument with a corresponding data size work this way. This is done for both convenience (the buffer almost always has the proper position/limit) and safety (no buffer overflows due to bugs).

Quote from: NateS on October 24, 2010, 01:22:27- GL11.glNormalPointer() can either be specified via a long pointer and a stride or via a Buffer in which ase no stride can be given. So how do interleaved vertex arrays work then if the pointer version only works with VBOs?

Actually, there is a stride argument in the buffer case.

Quote from: NateS on October 24, 2010, 01:22:27- GL11.glTexCoordPointer has a stride but no ByteBuffer variant? Wtf?

GL_BYTE is not allowed in the type argument of glTexCoordPointer (see table 2.5 of the latest GL spec), that's why a ByteBuffer variant does not exist.

Quote from: NateS on October 24, 2010, 01:22:27A friend of mine had these comments on LWJGL. What do you guys think?

- why do those freaking methods take ByteBuffer, FloatBuffer and DoubleBuffer only? Why not just Buffer? As they all have to be direct buffers it doesn't matter.

There are a few reasons LWJGL needs to know the concrete Buffer type, the first 2 of which are important:

- When buffer checks are not disabled, all LWJGL methods that accept a buffer argument check to see if that buffer is direct. Unfortunately, the .isDirect() method has only been moved to the base java.nio.Buffer class in JDK 6. Since we had to support JDK 1.4 until recently (and now JDK 5), we have to call the type-specific equivalent.

- As you know all buffer arguments are translated to pointers at the C level. Since JNI's GetDirectBufferAddress function returns the base address of a direct buffer, we need to know by how many bytes to multiply the current buffer position in order to pass the correct address to the C function. Again, we need the concrete buffer type for that.

- For methods that require a data type argument (e.g. glVertexPointer), we need to know the buffer type to pass the corresponding GL type.

- For methods that require a data size argument (e.g. glBufferData), we need to know the buffer type to pass the corresponding size (data.remaining() * <bytes_per_element>).

If we used Buffer arguments, we'd need 2 things:

- Expose more method arguments (size, type, etc). That's extra work for the library user to pass information that in 99% of the cases is already there. It would be an additional source of bugs and possible security problems (buffer overflows, etc).

- Perform runtime checks to discover the concrete buffer type, sacrificing performance.

Anyway, the fact is that no LWJGL user has ever complained about this before. You make one valid point about .asTypeBuffer() doing an extra allocation, but in a properly designed engine you'd never do that in your loops. Most buffer usages happen in the initialization code (where a few extra allocations are simple noise) and in the few cases that you do use buffer arguments in your loops (e.g. glUniformMatrixX) there's no real reason to not have those IntBuffers/FloatBuffers/etc cached. The engine code that fills them would accept a concrete type anyway (why use a ByteBuffer to upload your MVP matrix?).

Quote from: NateS on October 24, 2010, 06:14:25Why not do it both ways? Why not mimic the C API 100% in addition to the potentially more convenient type buffer methods?

LWJGL touts the ability to paste in C code without edits using static imports.

Unfortunately static imports won't do anything about the fact that C code uses pointers and Java code uses buffers. That code needs to be editted, no matter how LWJGL chooses to expose buffer arguments.

Momoko_Fan

In jMonkeyEngine3, the buffer is stored in its desired format so there's no need to call as***Buffer():
switch (vb.getFormat()){
                case Byte:
                case UnsignedByte:
                    glBufferData(target, (ByteBuffer) vb.getData(), usage);
                    break;
                case Short:
                case UnsignedShort:
                    glBufferData(target, (ShortBuffer) vb.getData(), usage);
                    break;
                case Int:
                case UnsignedInt:
                    glBufferData(target, (IntBuffer) vb.getData(), usage);
                    break;
                case Float:
                    glBufferData(target, (FloatBuffer) vb.getData(), usage);
                    break;
                case Double:
                    glBufferData(target, (DoubleBuffer) vb.getData(), usage);
                    break;
                default:
                    throw new RuntimeException("Unknown buffer format.");
            }