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

Re: [Public WebGL] Spec Comments



Hi Gregg,

Few comments inline below :

On Sat, May 8, 2010 at 01:12, Gregg Tavares <gman@google.com> wrote:
> Section 4.1: WebGLArrayBuffer
> Should we choose 1 way to handle accessing out of range values? Or can we
> at least make it consistent from a programming point of view? I think the
> idea was that some systems can enforce this in hardware so we didn't want to
> dictate a solution. In that case it seems the spec should say disallow
> throwing an exception so that from the programmers point of view he doesn't
> have to put try {} catch {} around every access if he's excepting user
> data.


> 5.15.8 Texture Objects
> OpenGL ES 2.0 has the idea of default textures. There is a default 2D
> texture and a default 3D texture. Both textures can be modified as in
> (...)
> Right now WebGL explicitly generates INVALID_OPERATION if no texture is
> bound or if any texture functions are called after
> ctx.bindTexture(GL_TEXTURE_2D, null). It seems like WebGL should be the
> same as OpenGL here.

I agree.
Maybe we should also define *what* is the default texture in WebGL ?
I'm not sure if this is formally specified in OpenGL though most
(all?) implementations use opaque white afaik.
This could be useful for consistency between WebGL implementations on
top of non-OpenGL APIs (with or without Angle).
For instance, Direct3D has a slightly different model wrt default
[null] texture (you cannot modify it, it can be black or white
depending the driver, and in the worst case even allow access to
uninitialized video memory from the shaders).
A WebGL implementation on top of something else than OpenGL would need
to create e.g a 1x1 opaque white texture bound at context
initialization and bind to it whenever gl.bindTexture(null) is used.


For instance opaque white as used in OpenGL, in that case some WebGL
implementations
would need


> 5.15.3 getParameter
> The spec should probably be explicit whether
> ctx.getParameter(ctx.TEXTURE_BINDING_2D) and similar texture related enums
> should return null or a WebGLTexture for the default texture (id = 0). I'd
> pick null I think.

Yes, and null should also be used in case an error happens imho.
More generally, right now it is not specified what is returned when a
GL error is generated by a "get function", it should be specified that
any error in one of those functions make it return null.

Regarding your subsequent comment, I would think null should also be
returned when "get functions" are called in context lost state, doing
so
would exercise the same error path, and null-checking should be indeed
encouraged. It should not be that much of a burden, considering "get
functions" are usually not used in 'hot' rendering code anyways but
mostly at initialization etc...


> 5.15.12 ReadPixels
> The spec needs to specify that out of range pixels must be 0,0,0,0 and that
> a lost context will still return a pixels, all of which are 0,0,0,0

Why not return null in case of lost context?
If we still return pixels, for consistency we would also have to
return pixels when an error is generated (e.g
GL_INVALID_FRAMEBUFFER_OPERATION).

Actually imho we'd probably better fix both above issues by changing
the signature of readPixels to something more similar to GL's :

void readPixels(GLint x, GLint y, GLsizei width, GLsizei height,
GLenum format, GLenum type, WebGLArrayBuffer data, optional int
offset);

If an error is generated, no change is made to the contents of data
(exactly as specified in OpenGL).
Additionally, this signature allows reuse of same buffer by multiple
calls, reducing GC pressure of creating new array at every call, and
allowing faster compositing of multiple buffers in same destination.


> 5.15.14 extension strings case-insensitive?
>> The spec here is unclear. It seems like what it means to say is extension
> strings passed to getExtension are case-insensitive. What it currently says
> suggests that the strings passed back by getSupportedExtensions() are
> case-insensitive which suggests something like this would be possible.
> var extensions = ctx.getSupportedExtensions();
> if (extensions["SoMeRaNdOmExTeNtIoN") !== undefined) {
> Â // this extension exists.
> }

Being explicit that extension strings are case-sensitive is a good
idea, however it is largely implied currently by :
"Any string in this list, when passed to getExtension must return a
valid object. Any other string passed to getExtension must return
null."

Also getSupportedExtensions returns an array of string, not an object
with named properties, so your example would need to be:

var extensions = ctx.getSupportedExtensions();
if (extensions.indexOf("SoMeRaNdOmExTeNtIoN") >= 0) {
   // this extension exists
}

(Array.indexOf is not case-insensitive)


Cheers!

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