How is glGetUniformIndices supposed to be called?

Started by Kai, December 03, 2009, 16:10:24

Previous topic - Next topic

Kai

There are some OpenGL methods that take an array of char* (char**) as a parameter.
LWJGL maps this to a single ByteBuffer. Now, I would like to know how the data in that ByteBuffer is supposed to be layouted.
In C, it would be clear, because there we have pointers that introduce a level of indirection, so that char** would be consecutive integers holding the address of the first character of each char*.
How would this work with Java/LWJGL, for example with the method 'ARBUniformBufferObject.glGetUniformIndices'?

When having 2 java.lang.String objects the JVM crashes when they are simply put into a ByteBuffer with a separating '\0' character.

I looked into the JNI-C code and found no hint as to how the strings should be layed-outed in the ByteBuffer correctly.

Can anyone comment on that?

Thanks in advance,
Kai

spasi

Quote from: Kai on December 03, 2009, 16:10:24When having 2 java.lang.String objects the JVM crashes when they are simply put into a ByteBuffer with a separating '\0' character.

You need another \0 at the end, so one null-termination per string. But you should get an exception there, not a crash, LWJGL is checking if your ByteBuffer is null-terminated. Are you sure the uniformIndices IntBuffer is set up correctly? For 2 strings, you need uniformIndices.remaining() to be equal to 2.

Kai

Thanks for your reply.

QuoteYou need another \0 at the end, so one null-termination per string[...]
Yes, sorry. That's what I meant. I put a \0 at the end of each string.

The code that makes a ByteBuffer out of String[]:
   private static ByteBuffer toByteBuffer(String[] strs) {
        int length = 0;
        for (int i = 0; i < strs.length; i++) {
            length += strs[i].length() + 1; // +1 for the NULL-character
        }
        final ByteBuffer buff = BufferUtils.createByteBuffer(length);
        for (int i = 0; i < strs.length; i++) {
            buff.put(strs[i].getBytes());
            buff.put((byte) 0); // The ending NULL-character
        }
        buff.flip();
        return buff;
    }


Then the OpenGL call:
   public int[] getUniformIndices(int programId, String[] uniformNames) {
        IntBuffer tmp = BufferUtils.createIntBuffer(uniformNames.length);
        ARBUniformBufferObject.glGetUniformIndices(programId,
                toByteBuffer(uniformNames), tmp);
        int[] ret = new int[uniformNames.length];
        tmp.get(ret);
        return ret;
    }


QuoteAre you sure the uniformIndices IntBuffer is set up correctly? For 2 strings, you need uniformIndices.remaining() to be equal to 2.
Yepp.

But looking into the generated C-code the layout in the ByteBuffer would not make any sense to me. It should be one ByteBuffer containing the "JVM-addresses" of other ByteBuffers each containing the String at position i in 'uniformNames', respectively. (Which is not possible with simple-Java methods, I guess)

EDIT:
So I think, that is a special case that has to be handled differently than other char* -> ByteBuffer mappings.
Can't we say that by convention for each OpenGL methods that expects char** as a parameter, a ByteBuffer must be constructed consisting of the Strings separated by \0-character (including \0-character at the end).
Then, in the C-code, this flattened string-array must be scanned (for each string). This could be done with going through the array with strlen which tells the length of each string (up until the next \0-character) and advancing the 'pointer'/'position' in the string array.
These positions (addresses of the first character in the string array for each string that was found during the scan) must then be stored in a temporary new char* whose length is the number of strings (2 in the above example).

spasi


Kai

Okay, what actually has to be done in "org_lwjgl_opengl_ARBUniformBufferObject.c" is (kind of) the following:

JNIEXPORT void JNICALL Java_org_lwjgl_opengl_ARBUniformBufferObject_nglGetUniformIndices(JNIEnv *env, jclass clazz, jint program, jint uniformCount, jobject uniformNames, jint uniformNames_position, jobject uniformIndices, jint uniformIndices_position, jlong function_pointer) {
  const GLchar *uniformNames_address = ((const GLchar *)(*env)->GetDirectBufferAddress(env, uniformNames)) + uniformNames_position;
  GLuint *uniformIndices_address = ((GLuint *)(*env)->GetDirectBufferAddress(env, uniformIndices)) + uniformIndices_position;
  GLchar* strings[uniformCount];

  glGetUniformIndicesPROC glGetUniformIndices = (glGetUniformIndicesPROC)((intptr_t)function_pointer);
	
  /* What glGetUniformIndices expects is a char** whose elements are the
     addresses of the first character of each char*, so we scan the uniformNames
     for \0-separated strings. */
  unsigned int i = 0;

  /* Contains the address (char*) of each consecutive string */
  GLchar* next = uniformNames_address;
  do {
    /* Store the address of the i-th string */
    strings[i++] = next;
    /* Search the beginning of the next string */
    next += strlen(next) + 1;
  } while (i < uniformCount);

  /* Old call: */
  // glGetUniformIndices(program, uniformCount, uniformNames_address, uniformIndices_address);
  /* New call: */
  glGetUniformIndices(program, uniformCount, strings, uniformIndices_address);
}


I know that this code is being auto-generated, so the generation process must somehow be aware of that.

Kai

I found the following code semantically equivalent to glGetUniformIndices, which I will use for the time being to circumvent this issue:

    public int[] getUniformIndices(int programId, String[] uniformNames) {
        int activeUniforms = getObjectParameter(programId,
                ShaderObjectParameter.OBJECT_ACTIVE_UNIFORMS);
        int[] ret = new int[uniformNames.length];
        Arrays.fill(ret, ARBUniformBufferObject.GL_INVALID_INDEX);
        for (int n = 0; n < activeUniforms; n++) {
            String name = getActiveUniformName(programId, n);
            for (int s = 0; s < uniformNames.length; s++) {
                if (name.equals(uniformNames[s])) {
                    ret[s] = n;
                }
            }
        }
        return ret;
    }


It is by far not that efficient but does the job.
I hope that the proposed change to the char** -> ByteBuffer mapping will be included in the (near) future!

spasi


Matzon

Quote from: spasi on December 04, 2009, 04:55:40
Fixed and committed. Grab a fresh build to check it out.
awesomeness - and you even stayed up all night to do it 8)

Kai

QuoteFixed and committed. Grab a fresh build to check it out.

WOW! That was pretty fast! I love you, guys, you definitely rock!

Much thanks for the fix and for your quick responsiveness!