Comment on attachment 8493663 [details] [diff] [review] WebGL2 - GL symbols for texture storage. Review of attachment 8493663 [details] [diff] [review]: ----------------------------------------------------------------- I'll just grab this review, since it's simple.

Comment on attachment 8493664 [details] [diff] [review] WebGL2 - GL symbols for 3D textures. Review of attachment 8493664 [details] [diff] [review]: ----------------------------------------------------------------- I'll just grab this review, since it's simple. ::: gfx/gl/GLContext.cpp @@ +1211,5 @@ > + { (PRFuncPtr*) &mSymbols.fTexSubImage3D, { "TexSubImage3DEXT", "TexSubImage3DOES", nullptr } }, > + END_SYMBOLS > + }; > + > + bool useCore = IsFeatureProvidedByCoreSymbols(GLFeature::texture_3D); I hate that we have to do this stuff. ::: gfx/gl/GLContextSymbols.h @@ +640,5 @@ > + typedef void (GLAPIENTRY * PFNGLTEXIMAGE3DPROC) (GLenum target, GLint level, GLint internalformat, > + GLsizei width, GLsizei height, GLsizei depth, > + GLint border, GLenum format, GLenum type, > + const GLvoid* pixels); > + PFNGLTEXIMAGE3DPROC fTexImage3D; Technically, we should only need TexSubImage3D in WebGL2. IIRC, we require 3D textures to be allocated with TexStorage3D.

https://hg.mozilla.org/integration/mozilla-inbound/rev/231c5214e07c https://hg.mozilla.org/integration/mozilla-inbound/rev/f782e395482d

sorry had to backout https://hg.mozilla.org/integration/mozilla-inbound/rev/231c5214e07c for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=48747573&tree=Mozilla-Inbound

(In reply to Carsten Book [:Tomcat] from comment #6) > sorry had to backout > https://hg.mozilla.org/integration/mozilla-inbound/rev/231c5214e07c for test > failures like > https://tbpl.mozilla.org/php/getParsedLog.php?id=48747573&tree=Mozilla- > Inbound So it appears ANGLE reports that EXT_texture_storage is available, but we don't get all the symbols.

Comment on attachment 8524357 [details] [diff] [review] [WebGL2] Partner code doesn't specify all mipmaps for compressed textures allocated via glTexStorage Review of attachment 8524357 [details] [diff] [review]: ----------------------------------------------------------------- It shouldn't matter if they don't specify it. We need to handle this case. Is this case sufficiently hard to handle correctly that we should come back to it? I imagine they must not be *using* uninitialized texImages, so they're probably setting base/max tex levels for the textures. Are we not picking up on this? Are they not doing it? I suppose spec-wise this needs clarification, so here's my proposal: For a texture being mip-sampled, if the subset of levels [base...max] are not mip-complete, the texture will sample [0,0,0,1]. (fake-black)

Comment on attachment 8527430 [details] [diff] [review] [WebGL2] texParameter: TEXTURE_COMPARE_MODE and TEXTURE_COMPARE_FUNC support Review of attachment 8527430 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +1595,5 @@ > + intParam != LOCAL_GL_GREATER && > + intParam != LOCAL_GL_EQUAL && > + intParam != LOCAL_GL_NOTEQUAL && > + intParam != LOCAL_GL_ALWAYS && > + intParam != LOCAL_GL_NEVER); So many conditionals! switch (intParam) { case GOOD: case GOOD: case GOOD: case GOOD: case GOOD: paramValueInvalid = false; break; default: paramValueInvalid = true; break;

Fix silly logic bug discovered by testing demo.

Not sure if this is the right place but I think I stumbled upon a bug in texture state handling. In the latest Firefox Nightly 50.0a1 (2016-07-13) I get error like this with WebGL2: "Error: WebGL: drawElements: Active texture 5 for target 0x0de1 is 'incomplete', and will be rendered as RGBA(0,0,0,1), as per the GLES 2.0.24 $3.8.2: A depth or depth-stencil format with TEXTURE_COMPARE_MODE of NONE must have minification or magnification filtering of NEAREST or NEAREST_MIPMAP_NEAREST." But the thing is: at the place where this happens TEXTURE_COMPARE_MODE is set to COMPARE_REF_TO_TEXTURE, not NONE. Filtering is set to LINEAR (this is depth texture used for shadowmap sampling). This is WebGL2 / OpenGL ES3 feature, not WebGL1 / OpenGL ES2, so seems like validation here is incorrect? You can observe the problem for example here: http://alteredqualia.com/xg/examples/car_zastava.html Chrome's WebGL2 implementations works ok for this. -------- As far as I understood OpenGL ES3 specs, it seems to allow LINEAR filtering here (in fact it's intended feature for shadow samplers): "If the value of TEXTURE_COMPARE_MODE is COMPARE_REF_TO_TEXTURE, then r depends on the texture comparison function as shown in table 3.23. ... If the value of TEXTURE_MAG_FILTER is not NEAREST, or the value of TEXTURE_MIN_FILTER is not NEAREST or NEAREST_MIPMAP_NEAREST, then r may be computed by comparing more than one depth texture value to the texture reference value. The details of this are implementation-dependent, but r should be a value in the range [0, 1] which is proportional to the number of comparison passes or failures." https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf (Section 3.8.2, page 164)

(In reply to AlteredQualia from comment #27) > Not sure if this is the right place but I think I stumbled upon a bug in > texture state handling. > > In the latest Firefox Nightly 50.0a1 (2016-07-13) I get error like this with > WebGL2: > > "Error: WebGL: drawElements: Active texture 5 for target 0x0de1 is > 'incomplete', and will be rendered as RGBA(0,0,0,1), as per the GLES 2.0.24 > $3.8.2: A depth or depth-stencil format with TEXTURE_COMPARE_MODE of NONE > must have minification or magnification filtering of NEAREST or > NEAREST_MIPMAP_NEAREST." > > But the thing is: at the place where this happens TEXTURE_COMPARE_MODE is > set to COMPARE_REF_TO_TEXTURE, not NONE. Filtering is set to LINEAR (this is > depth texture used for shadowmap sampling). > > This is WebGL2 / OpenGL ES3 feature, not WebGL1 / OpenGL ES2, so seems like > validation here is incorrect? > > You can observe the problem for example here: > > http://alteredqualia.com/xg/examples/car_zastava.html > > Chrome's WebGL2 implementations works ok for this. > > -------- > > As far as I understood OpenGL ES3 specs, it seems to allow LINEAR filtering > here (in fact it's intended feature for shadow samplers): > > "If the value of TEXTURE_COMPARE_MODE is COMPARE_REF_TO_TEXTURE, then r > depends on the texture comparison function as shown in table 3.23. > > ... > > If the value of TEXTURE_MAG_FILTER is not NEAREST, or the value of > TEXTURE_MIN_FILTER is not NEAREST or NEAREST_MIPMAP_NEAREST, then r may be > computed by comparing more than one depth texture value to the texture > reference value. The details of this are implementation-dependent, but r > should be a value in the range [0, 1] which is proportional to the number of > comparison passes or failures." > > https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf (Section > 3.8.2, page 164) This is now bug 1291083.