[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 4:46 PM, Glenn Maynard <glenn@zewt.org> wrote:
On Thu, Apr 5, 2012 at 5:50 PM, Gregg Tavares (勤) <gman@google.com> wrote:
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.

Right, it can happen in a lot of places.  The reason we're focusing on the WebGLObject-creating functions right now is that there's a single, universal fix we can apply to all of them at once, without affecting any (non-pathological, at least) existing user code, since from the user's perspecitve, it just makes a particular rare case no longer happen.

I agree that we should look at other functions and see if we can at least reduce the number of cases where they return null if the context is lost, but that'll become a case-by-case thing, so it's a lot more work.  This should probably happen in parallel to further fixing the nullable return values.  (For example, getSupportedExtensions should probably always return the last-known supported extensions, not null.)

Many functions still have neither a nullable nor "any" return value, nor explicit context lost handling, which isn't valid.  A sample: getSupportedExtensions, getProgramInfoLog, getShaderPrecisionFormat, getShaderSource.  It's bad that it's so hard to detect these.  It would be a lot nicer if the "explicit context lost handling" was something we could annotate in WebIDL, eg. something like

boolean isContextLost() annotate(explicit_context_loss=True);

so these invalid cases could be detected automatically.  It might be useful for people generating bindings, too.

+CC Cameron: Is there any way to do this now, or could something like this be added, so we can annotate functions with arbitrary, API-specific strings?  This would have no effect on the IDL; it would just be for API-specific flags. 
 
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

This is still worth doing even if we can't do it everywhere; a 50% reduction in null-reference bugs is still an improvement, even if we can't reach 0% (which we probably can't, short of a broad switch to exceptions).

On Thu, Apr 5, 2012 at 6:16 PM, Kenneth Russell <kbr@google.com> wrote:
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 the main reason I suggested it (the nullable simplification is what made me think of it, but it's just a nice side-effect to me).  Returning null for errors, especially for rare errors, is one of the biggest source of obscure bugs in all languages.  It's not normally a problem in _javascript_--this is why everyone has switched to exceptions--but WebGL reintroduced it by using nulls.  I definitely wish WebGL had used exceptions like other APIs for synchronous errors like these (as we've discussed before), but if we can't fix that, then let's at least try to minimize the problem.

Wait what? How would exceptions help here? Is it even possible to use exceptions AND get performance AND get no surprises during context lost? 
 
 
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.

"It'll break the test suite" is a very bad reason not to improve something.  Test suites are meant to improve APIs and implementations; if they're preventing us from improving them, they're doing the opposite.  We should only be concerned with whether a change improves the API, and what its web-compatibility effects are.  If it breaks the tests, then the tests get updated; that shouldn't be a consideration.

--
Glenn Maynard