[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 5:09 PM, Gregg Tavares (勤) <gman@google.com> wrote:
>
>
> On Thu, Apr 5, 2012 at 4:16 PM, Kenneth Russell <kbr@google.com> wrote:
>>
>> On Thu, Apr 5, 2012 at 3:50 PM, Gregg Tavares (勤) <gman@google.com> wrote:
>> >
>> >
>> > 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 = urlOfImageImGoingToPutInTexture;  // 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?
>>
>> You're right, in the general case it isn't going to be possible for
>> the WebGL implementation to provide "fake" answers for all of these
>> queries, to prevent null from being returned after context loss.
>>
>> As I see it, the main reason for changing the createXXX routines to
>> return non-null, invalidated objects after context loss is not really
>> to protect developers from dereferencing null. It's to allow the
>> nullable flag to be removed everywhere (or almost everywhere?) a
>> WebGLObject is passed in to WebGL.
>
>
> these require NULL
>
>     bindBuffer
>     bindFramebuffer
>     bindRenderbuffer
>     bindTexture
>     useProgram
>     framebufferTexture2D
>     framebufferRenderbuffer
>
> That leaves
>
>     deleteXXX?
>     isXXX?
>     getProgramParameter
>     getShaderParameter
>     getActiveUniform
>     getActiveAttrib
>     getAttachedShaders
>     getProgramInfoLog
>     getShaderInfoLog
>     getUniform
>     getUniformLocation
>     getAttribLocation
>
> Should deleteXXX disallow NULL? It's explicitly allowed in OpenGL. Same with
> isXXX
>
> So I guess I don't see disallowing NULL in a few functions a very compelling
> reason to do any of this.

Thanks for pointing this out again.

It's definitely a compelling point that there are other methods
dealing with these types which absolutely have to accept nullable
arguments.

Early in the development of the WebGL spec, every entry point raised
DOMException, and exceptions were thrown for various error conditions
that would be caught by the WebGL implementation, rather than the
underlying OpenGL API. It was decided some time ago to unify the
behavior to stop throwing exceptions and instead use OpenGL's error
reporting mechanism -- synthesizing OpenGL errors where necessary. The
reasons were that "real" errors in the application would necessarily
be reported via OpenGL's getError() call; that debug wrappers could
translate OpenGL errors into JavaScript exceptions anyway; and that
applications should not need to use try/catch around every GL call in
order to be robust.

Gregg, given this history, I agree with you that it's a mistake to try
to start throwing TypeError for a few methods on WebGLRenderingContext
when they are passed null arguments. It would be asymmetric compared
to the rest of the API.

Given all of this, I've updated the editor's draft to reflect the
current context loss specification:

 - The various create() methods now return nullable types.
 - The various is() queries accept nullable arguments.
 - getSupportedExtensions(), getProgramInfoLog(), and similar queries
now return nullable types.

Missing text has been added to the spec for bufferSubData regarding
the behavior when passed null. texImage2D and texSubImage2D now accept
null for a couple more overloads, and generate an INVALID_VALUE error.

webgl.idl has been rebuilt with these changes.

Each of the edits was done separately so they can be examined
independently; consult the Subversion history of
specs/latest/index.html .

http://www.khronos.org/bugzilla/show_bug.cgi?id=619 has been filed to
track the needed conformance suite updates after all of the nullable
changes in the spec.

Please review these changes and send feedback to the list. Given the
constraints, I think this is the best resolution, and hope that we can
draw the nullable discussions to a close.

-Ken


> Note: getUniform like getParameter is also problematic.
>
>  var matrix = gl.getUniform(prg, modelViewMatrixLocation);
>  var translation = matrix.slice(12, 3);  // crash
>
>> This would simplify the spec and
>> implementations a fair amount -- it would no longer be necessary to
>> explicitly generate an INVALID_VALUE OpenGL error for these entry
>> points. It also unifies the behavior before and after context loss a
>> fair amount.
>>
>> I don't know whether it's possible, or a good idea, to consider this
>> proposal independently from other return values from getParameter,
>> getActiveUniform, getUniformLocation, etc. It is probably impossible
>> to guarantee that a WebGL implementation will return the same answers
>> for calls like getActiveUniform before and after the context is lost,
>> due to the asynchronous nature of context loss at the lowest level.
>>
>> There is definitely an argument that the current behavior is uniform.
>> Every entry point, including the creation entry points, can start
>> returning null essentially at any time. Also, if the behavior is
>> changed, the 1.0.1 conformance suite will no longer run on future
>> WebGL implementations, because it tests passing null for WebGLObjects
>> and expects that no exceptions are thrown.
>>
>> What do you think? Is this worth pursuing?
>>
>> -Ken
>>
>> >>
>> >>
>> >>
>> >> >> 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
>> >
>> >
>
>

-----------------------------------------------------------
You are currently subscribed to public_webgl@khronos.org.
To unsubscribe, send an email to majordomo@khronos.org with
the following command in the body of your email:
unsubscribe public_webgl
-----------------------------------------------------------