glGetUniformLocation() doesn't seem conform to GL spec

[Floh]

I just noticed that emscripten's glGetUniformLocation() returns a new value after each call even for identical parameters(?):

E.g.

GLuint prog = some_linked_gl_program;

GLint loc0 = glGetUniformLocation(prog, "Bla");

GLint loc1 = glGetUniformLocation(prog, "Bla");

On the GL implementations I have encountered so far, loc0 would be the same as loc1, but in emscripten, loc1 is loc0+1. My shader wrapper code is currently confused by this, but I can workaround that (just need to make sure that each lookup only happens exactly once, but I *think* this behaviour is against the GL Spec:

http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf

Search for GetUniformLocation, and a few lines below:

"After a program is linked, the location of a uniform variable will not change, unless the program is re-linked."

Cheers,

-Floh.

[Alon Zakai]

Yeah, good catch, you are correct. We didn't port a codebase that
noticed this yet I guess. I fixed this on the incoming branch now.

- azakai

[Floh]

Ok, just checked this and it works. Thanks for the quick fix.

-Floh.

[Floh]

Uh, wait a minute :)

I'm looking at the glGetUniformLocation wrapper in library_gl.js right now, if I'm not mistaken the function would now return the same location for the same name string, but doesn't take the GL program into account, is this right? However, the location of a named parameter may be different across different programs.

I think the function must use the program *and* name to create a unique key into GL.uniformTabe[] map. 

I'm currently having strange errors in more complex rendering cases, and this might explain it, but this was late yesterday evening and I didn't do detailed debugging yet.

Cheers,

-Floh.

[Floh]

Ok, I quickly hacked the glGetUniformLocation function in library_gl.js locally like this, and it works. But I'm not exactly a JavaScript expert, don't know whether this is efficient, GL.uniformTable would basically be a 2-dimensional map, with program and name as keys:

  glGetUniformLocation: function(program, name) {

    name = Pointer_stringify(name);

    if (!GL.uniformTable[program])

    {

        GL.uniformTable[program] = {};

    }

    var id = GL.uniformTable[program][name];

    if (id) return id; 

    var loc = Module.ctx.getUniformLocation(GL.programs[program], name);

    if (!loc) return -1;

    id = GL.getNewId(GL.uniforms);

    GL.uniforms[id] = loc;

    GL.uniformTable[program][name] = id;

    return id;

  },

And lo-and-behold, the first working skinned-character-rendering in Nebula3, this is only the NormalDepth-Pass in the pre-light-pass renderer:

I'll continue with the renderer stuff, and then look at optimization options. Guess I'll be back here soon with more questions ;)

Cheers,

-Floh

[Alon Zakai]

Yeah, I am noticing some new bugs after that change ;) I guess I was too hasty. The test suite shows problems too.

Thanks for the patch, I am investigating this now.

- azakai

[Alon Zakai]

Ok, fixed that problem in incoming.

Incoming is still broken for another reason though, working on that now. (This is why we only merge to master after tests pass ;)

- azakai

[Floh]

Ok, I did quick check with your changes, and some additional lights (one directional light, 2 point lights), and the complete pre-light-pass pipeline (normal-depth pass, lighting pass, material pass, and finally compose pass), so there's quite a bit of shader stuff going on and it looks fine:

I'll go back to the master branch for now and only use the updated library_gl in my local copy to make sure that I'm not affected from new issues in incoming. Thanks for the quick fixes!

-Floh.

[Alon Zakai]

Glad to hear it, and nice screenshot :)

incoming branch should be ok now. (But as always, only master should be considered stable.)

- azakai