[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Public WebGL] Null return values from create*





On Thu, Apr 5, 2012 at 3:28 PM, Kenneth Russell <kbr@google.com> wrote:
On Wed, Apr 4, 2012 at 8:13 PM, Glenn Maynard <glenn@zewt.org> wrote:
> On Wed, Apr 4, 2012 at 9:18 PM, Gregg Tavares (勤) <gman@google.com> wrote:
>>
>> Where are all the places one would return these invalid objects?
>>
>> createProgram,
>> createShader,
>> createTexture,
>> createBuffer,
>> createRenderbuffer
>> createFramebuffer,
>> getUniformLocation?
>> getActiveAttrib
>> getActiveUniform
>>
>> getUniformLocation seems a little weird in that passing it a uniform that
>> does not exist returns NULL but passing it a uniform after context lost
>> returns an invalid location? Or still NULL?
>
>
> We're only really talking about functions that return WebGLObject
> subclasses, since those are the ones that have an invalidated flag to use
> instead of a null return value.  WebGLUniformLocation and WebGLActiveInfo
> don't have an invalidated state, since it's not a WebGLObject.  I'd look at
> these later, if we start looking at reducing other null return values.

Gregg: after context loss, getUniformLocation is covered by the rules
in http://www.khronos.org/registry/webgl/specs/latest/#5.14, which
means that it would return null, not an invalid location.

The issue for me is, it seems like we're fixing createXXX because people currently write code like this

tex = gl.createTexture();
tex.url = ""  // crash

and that second line will fail if the context is lost. This type of currently invalid code is all over learningwebgl.com :-(

Well, that same problem exists with getUniformLocation and, as Glenn pointed out in my own code, with getActiveAttrib and getActiveUniform.

For getUniformLocation it's certainly possible that people are attaching things to the location object (though less common)

var mvpLoc = gl.getUniformLocation(prg, "u_modelViewProjection");
mvpLoc.name = "u_modelViewProjection"  // crash

And of course getActiveAttrib and getActiveUniform are guaranteed to have this problem.

var info = getActiveAttrib(program, index);
if (info.size > 1 && endsWith(info.name, "[0]")) {  // crash
   ...
}

so all I'm saying is if we're trying to fix these issues shouldn't we fix them everywhere?

The thing is, even that won't save you. Lots of people do stuff this

// link program I know has "u_modelViewProjection"
gl.linkProgram(prg);
// Make setters for it
var numUniforms = gl.getProgramParameter(gl.ACTIVE_UNIFORMS);
for (var ii = 0; ii < numUniforms; ++ii) {
   var info = gl.getActiveUniform(prog, ii);
   if (!info) continue;
   var loc = gl.getUniformLocation(prog, info.name);
   // make a setter for each uiform
   switch (info.type) {
   case gl.FLOAT_MAT4:
     prg[name] = function(loc) {
        return function(matrix) {
            gl.uniformMatrix4fv(loc, 1, false, matrix);
        };
     }(loc);
     break;
    ...

They then write code like this

    prg.u_modelViewProjection(someMatrix);

That code will crash on lost context since "prg.u_modelViewProjection" will not exist.

Which begs the question, if we can't project them from that do we need to protect them from the others?


 




 


>> how about getParameter?
>
>
> getParameter is hard, since there are a lot of possible arguments with
> varying reasonable "placeholder" results.  I'd defer this, too.
>
> While I'm thinking about it: does getParameter() define that values like
> MAX_VIEWPORT_DIMS and VIEWPORT always return a new object, as opposed to
> returning the same object each time?

They should. I've clarified getParameter, getUniform and
getVertexAttrib in this area.

-Ken