Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Depth Textures need more tests/specs? #3353

Closed
greggman opened this issue Nov 2, 2021 · 7 comments
Closed

Depth Textures need more tests/specs? #3353

greggman opened this issue Nov 2, 2021 · 7 comments

Comments

@greggman
Copy link
Contributor

greggman commented Nov 2, 2021

I ran into 2 issues related to depth textures.

(1) Chrome doesn't allow filtering DEPTH_COMPONENT16 textures, Firefox does on Mac Only

Firefox MacOS
Screen Shot 2021-11-02 at 2 59 31 PM

Chrome Mac/Win (or Firefox Win)
Screen Shot 2021-11-02 at 3 01 57 PM

Does the spec need to specify which of those 2 is correct?

(2) Neither browser on either OS seems to allow uploading data to a D32F texture.

See the screenshots above, the first square D32F seems like it should match R32F. In fact those 2 formats seem like they are supposed be pretty similar except D32F can be used as a depth attachment and R32F as a color attachment.

There's no error trying to upload the data to the D32F texture (same data as the R32F texture) but it doesn't appear. And in fact the spec 3.7.2 specifies what values to use a for texImage2D and a DEPTH_COMPONENT32F texture.

Is this a bug?

Live version: https://jsgist.org/?src=493218139726a53b7b3affbb636148e9

@kdashg
Copy link
Contributor

kdashg commented Nov 2, 2021

Sized depth or depth-stencil formats are not filterable in GLES 3.0.6 p161:

Using the preceding definitions, a texture is complete unless any of the follow-
ing conditions hold true
:
• Any dimension of the levelbase array is not positive.
• The texture is a cube map texture, and is not cube complete.
• The minification filter requires a mipmap (is neither NEAREST nor LINEAR),
and the texture is not mipmap complete.
• The effective internal format specified for the texture arrays is a sized in-
ternal color format that is not texture-filterable (see table 3.13), and either
the magnification filter is not NEAREST or the minification filter is neither
NEAREST nor NEAREST_MIPMAP_NEAREST.
• The effective internal format specified for the texture arrays is a sized
internal depth or depth and stencil format
(see table 3.14), the value of
TEXTURE_COMPARE_MODE is NONE, and either the magnification filter is
not NEAREST or the minification filter is neither NEAREST nor NEAREST_-
MIPMAP_NEAREST.

However we marked these as filterable in Firefox a couple years back:
https://searchfox.org/mozilla-central/rev/7588d077255c5732f72c466e71d7a38d97385dd5/dom/canvas/WebGLFormats.cpp#1081
Because of this language, now on p151 of ES3.0.6:

3.8.8 Depth Component Textures
Depth textures and the depth components of depth/stencil textures can be treated
as RED textures during texture filtering and application (see section 3.8.15).

I believe all texture formats in WebGL1 (es2) were filterable, and there wasn't a concept of a non-filterable format.
I believe that's why there's the sized carve-out in the es3 language.

I believe that the specs say that d32f should not be filterable, and that this should have a test.


Good news: D32F uploads do indeed work, but sampling from a depth format always clamps to [0,1.0], so you were actually just re-scaling too late, versus this modification: https://jsfiddle.net/mzadun4L/

@kenrussell
Copy link
Member

Going in reverse order:

(2) This is probably a bug, but I'm not sure whether there are restrictions in the underlying APIs (in particular, D3D) which caused the WebGL working group to land on the current behavior. In WebGL 1.0, the WEBGL_depth_texture extension explicitly forbade trying to upload CPU-side data to depth, stencil, or depth/stencil textures. I think this decision was made because of restrictions in D3D9. Presumably D3D11 would have lifted any such restrictions.

The DEPTH32F_STENCIL8 / FLOAT_32_UNSIGNED_INT_24_8_REV internal format and type combination was problematic when specifying WebGL. There isn't a typed array type which would map well to this format for uploading interleaved data.

I'm personally not sure whether we should strive to fix this on all browsers and platforms given that it's been broken for this long. Most WebGL applications are probably only rendering to depth textures and don't expect CPU-side uploading of depth data to work. At least we should investigate in Chromium how feasible it is to get this working on Windows, Mac and Linux - all three platforms will soon be using different backend APIs. If it looks difficult to make this behavior portable then we should probably just specify and test that uploading CPU-side depth data is not allowed, like the WEBGL_depth_texture extension did.

(1) From my reading of the OpenGL ES spec, depth textures are considered incomplete if they don't use NEAREST filtering. See section 3.8.13 "Texture Completeness" of the ES 3.0.6 spec, or section 8.17 "Texture Completeness" of the ES 3.2 spec (more recent). The ES 3.2 spec says the texture's incomplete because:

The texture is not multisample; either the magnification filter is not NEAREST, or the minification filter is neither NEAREST nor NEAREST_MIPMAP_NEAREST; and...the effective internal format specified for the texture arrays is a sized internal depth or depth and stencil format (see table 8.11), and the value of TEXTURE_COMPARE_MODE is NONE.

I think the depth texture's supposed to be incomplete when linear filtering's applied to it.

@kdashg
Copy link
Contributor

kdashg commented Nov 2, 2021

FWIW, Firefox without ANGLE on Windows does indeed filter d16, though this is desktop GL. Trying on an Android Adreno device, d16 does not filter.

@greggman
Copy link
Contributor Author

greggman commented Nov 3, 2021

Like @jdashg pointed out, for (2) that was my bug, D32F is clamped 0,1. R32F is not so that's working now that the bug in my sample is fixed.

3.8.3. For depth component groups, the depth value is clamped to [0, 1].

for (1) yes, I guess FF should not filter.

@kenrussell
Copy link
Member

OK, great that there aren't that many behavioral differences!

It would be good to expand the testing in this area.

@kdashg
Copy link
Contributor

kdashg commented Nov 3, 2021 via email

@kdashg
Copy link
Contributor

kdashg commented Nov 5, 2021

I think with the tests in #3357, we'll be all set!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants