Closed Bug 1048741 Opened 10 years ago Closed 8 years ago

WebGL2 - Implement Texture Objects

Categories

(Core :: Graphics: CanvasWebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: u480271, Unassigned)

References

Details

(Keywords: dev-doc-complete)

Attachments

(5 files, 1 obsolete file)

Blocks: webgl2
Attachment #8493663 - Flags: review?(bjacob)
Attachment #8493664 - Flags: review?(bjacob)
Keywords: leave-open
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.
Attachment #8493663 - Flags: review?(bjacob) → review+
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.
Attachment #8493664 - Flags: review?(bjacob) → review+
(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.
Attachment #8494918 - Flags: review?(jgilbert)
Attachment #8493663 - Attachment is obsolete: true
Attachment #8494918 - Flags: review?(jgilbert) → review+
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)
Attachment #8524357 - Flags: review?(jgilbert) → review-
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;
Attachment #8527430 - Flags: review?(jgilbert) → review+
Fix silly logic bug discovered by testing demo.
Attachment #8537500 - Flags: review?(jgilbert)
Attachment #8537500 - Flags: review?(jgilbert) → review+
Depends on: 1216004
Assignee: dglastonbury → nobody
Status: ASSIGNED → NEW
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
You need to log in before you can comment on or make changes to this bug.