texSubImage3D: Wrong List of Formats

CodeSmith32 commented 6 months ago

MDN URL

https://developer.mozilla.org/en-US/docs/Web/API/WebGL2RenderingContext/texSubImage3D#format

What specific section or headline is this issue about?

Parameter format

What information was incorrect, unhelpful, or incomplete?

The list of possible enum values under the format parameter is wrong. It lists OpenGL internal formats.

What did you expect to see?

I believe it needs to list OpenGL formats not internal formats.

By experimentation, using the internal format gl.R8 does not work. Chromium gives the error "WebGL: INVALID_ENUM: texSubImage3D: invalid format"

It needs to be the format equivalent, gl.RED, which works correctly, but is not on this list.

Do you have any supporting links, references, or citations?

Following to the specification linked on the MDN page, under the first texSubImage3D overload definition, it states:

The combination of format, type, and WebGLTexture's internal format must be listed in Table 1 or 2 from man page.

This link takes you to the OpenGL specification for glTexImage3D, which lists both internal formats and formats. But comparing with the list of formats in tables 1 and 2 (formats which matches the parameter name, not internal formats), you get the following list:

Table 1

GL_RGB
GL_RGBA
GL_LUMINANCE_ALPHA
GL_LUMINANCE
GL_ALPHA

Table 2

GL_RED
GL_RED_INTEGER
GL_RG
GL_RG_INTEGER
GL_RGB
GL_RGB_INTEGER
GL_RGBA
GL_RGBA_INTEGER

Do you have anything more you want to share?

Since texSubImage3D is only an upload method, and not a texture configuration method (like texStorage3D / texImage3D) it makes sense that it doesn't require layout configuration.

Even still, I'd prefer that someone else double-check my understanding here. Otherwise, I'd have probably just updated the page myself.

MDN metadata

Page report details

* Folder: `en-us/web/api/webgl2renderingcontext/texsubimage3d` * MDN URL: https://developer.mozilla.org/en-US/docs/Web/API/WebGL2RenderingContext/texSubImage3D * GitHub URL: https://github.com/mdn/content/blob/main/files/en-us/web/api/webgl2renderingcontext/texsubimage3d/index.md * Last commit: https://github.com/mdn/content/commit/acfe8c9f1f4145f77653a2bc64a9744b001358dc * Document last modified: 2023-07-07T07:19:19.000Z

hamishwillee commented 6 months ago

@wbamberg I think this is missing some methods that should be in the syntax - those that use the internalFormat. It seems likely to be that @CodeSmith32 is correct that the things documented as format are actually the internalFormat. The TDLR for this is below. But I'm not an expert - is the best way to confirm to query against the spec github or is there a person to ask?

@CodeSmith32 Thank you. I followed the same process looking for a matching function prototype in the linked spec - found:

undefined texSubImage3D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, [AllowShared] ArrayBufferView? srcData, optional unsigned long long srcOffset = 0)

The top level man page link for this method states:

That explicitly states that the allowed set for format are: GL_RED, GL_RED_INTEGER, GL_RG, GL_RG_INTEGER, GL_RGB, GL_RGB_INTEGER, GL_RGBA, GL_RGBA_INTEGER, GL_DEPTH_COMPONENT, GL_DEPTH_STENCIL, GL_LUMINANCE_ALPHA, GL_LUMINANCE, and GL_ALPHA.

The comment in the body of the method states:

The combination of format, type, and WebGLTexture's internal format must be listed in Table 1 or 2 from man page.

But that man page is for a method that includes the "internal" format. I think the spec authors have just decided to keep the information about the various values in one place. So it is probably that the set is: GL_RED, GL_RED_INTEGER, GL_RG, GL_RG_INTEGER, GL_RGB, GL_RGB_INTEGER, GL_RGBA, GL_RGBA_INTEGER, GL_DEPTH_COMPONENT, GL_DEPTH_STENCIL, GL_LUMINANCE_ALPHA, GL_LUMINANCE, and GL_ALPHA.

IN addition

IDL in spec and also in Firefox sour e appears to include internalformat too, so that needs to be documented - and added to the Syntax box
The example uses gl.RGBA which we think is internal.
I couldn't be sure about options for format from Firefox code.

wbamberg commented 6 months ago

@wbamberg I think this is missing some methods that should be in the syntax - those that use the internalFormat.

I don't think so? In https://registry.khronos.org/webgl/specs/latest/2.0/#3.7.6 I see three IDL overloads for this method:

undefined texSubImage3D(
    GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset,
    GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type,
    GLintptr pboOffset);

undefined texSubImage3D(
    GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset,
    GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type,
    TexImageSource source); // May throw DOMException

undefined texSubImage3D(
    GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset,
    GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type,
    [AllowShared] ArrayBufferView? srcData, optional unsigned long long srcOffset = 0);

Since srcOffset is optional, we get four syntax variants out of that:

texSubImage3D(
  target, level, xoffset, yoffset, zoffset,
  width, height, depth, format, type,
  pboOffset)

texSubImage3D(
  target, level, xoffset, yoffset, zoffset,
  width, height, depth, format, type,
  source)

texSubImage3D(
  target, level, xoffset, yoffset, zoffset,
  width, height, depth, format, type,
  srcData)

texSubImage3D(
  target, level, xoffset, yoffset, zoffset,
  width, height, depth, format, type,
  srcData, srcOffset)

MDN does have these, although confusingly, it uses different names for some of the parameters:

pboOffset -> offset
source->pixels

It seems likely to be that @CodeSmith32 is correct that the things documented as format are actually the internalFormat.

But yes, I'm also no expert but it seems totally plausible.

Table 1

* GL_RGB

* GL_RGBA

* GL_LUMINANCE_ALPHA

* GL_LUMINANCE

* GL_ALPHA

Table 2

* GL_RED

* GL_RED_INTEGER

* GL_RG

* GL_RG_INTEGER

* GL_RGB

* GL_RGB_INTEGER

* GL_RGBA

* GL_RGBA_INTEGER

I think maybe also GL_DEPTH_COMPONENT and GL_DEPTH_STENCIL? The page actually appear to contain three tables, but the last one is not marked "Table 3" so maybe it should be included? Then the list would match Hamish's list, and also the list for the format item in https://registry.khronos.org/OpenGL-Refpages/es3.0/html/glTexImage3D.xhtml#parameters.

Again, not an expert, but it looks as if the internal format can be inferred from the format and the type, which is why there isn't a need to pass it as an extra parameter.

IDL in spec and also in Firefox sour e appears to include internalformat too, so that needs to be documented - and added to the Syntax box

I don't see that, can you point to it?

mdn / content