Error after setting GL_UNPACK_ROW_LENGTH

[seven332]

3.1.59

The function emscriptenWebGLGetTexPixelData doesn't check GL_UNPACK_ROW_LENGTH.
https://github.com/emscripten-core/emscripten/blob/3.1.59/src/library_webgl.js#L1600

For example in glTexSubImage:

glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
glPixelStorei(GL_UNPACK_ROW_LENGTH, 512);
glTexSubImage2D(
    GL_TEXTURE_2D,
    0, /*level*/
    0, /*xoffset*/
    0, /*yoffset*/
    36, /*width*/
    40, /*height*/
    GL_ALPHA,
    GL_UNSIGNED_BYTE,
    data);

The length of subarray should be 39 * 512 + 36 = 20004 at least, but it's 36 * 40 = 1440

Chrome and Safari:

WebGL: INVALID_OPERATION: texSubImage2D: ArrayBufferView not big enough for request

FireFox:

WebGL warning: texSubImage: Desired upload requires more bytes (20004) than are available (1440). 

[seven332]

Actually every parameters in glPixelStorei affects the expected length of pixels data, not only GL_UNPACK_ROW_LENGTH. Additionally packing and unpacking are different.
https://registry.khronos.org/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml

Here is how FireFox calculate the totalBytesUsed.
https://searchfox.org/mozilla-central/source/dom/canvas/WebGLContext.cpp#2741

Maybe we should just ignore the end in subarray to make life easier. There is no data length in c api after all.

[sbc100]

Actually every parameters in glPixelStorei affects the expected length of pixels data, not only GL_UNPACK_ROW_LENGTH. Additionally packing and unpacking are different. https://registry.khronos.org/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml

Are there particular glPixelStorei paramaters that you use and are interesting in have work in emscripten? We tend to implement this stuff reactively to get a surface area can captures that things folks actually use in practice.

Here is how FireFox calculate the totalBytesUsed. https://searchfox.org/mozilla-central/source/dom/canvas/WebGLContext.cpp#2741

Maybe we should just ignore the end in subarray to make life easier. There is no data length in c api after all.

[seven332]

I use skia in emscripten. It use

• GR_GL_UNPACK_ROW_LENGTH
• GR_GL_UNPACK_ALIGNMENT
• GR_GL_PACK_ROW_LENGTH
• GR_GL_PACK_ALIGNMENT
  https://github.com/google/skia/blob/main/src/gpu/ganesh/gl/GrGLGpu.cpp

[sbc100]

Interesting.. IIUC there are many folks using Skia built with emscripten (CanvasKit is basically that isn't it?). Do you know why nobody else has run into this over the years? Is this usage optional in some way?

[seven332]

This commit in 3.1.56 break it.
4de9bbb

In webgl2 before this commit:

GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, heap, toTypedArrayIndex(pixels, heap));

After this commit, in most cases,WEBGL_USE_GARBAGE_FREE_APIS is disabled.

var pixelData = pixels ? emscriptenWebGLGetTexPixelData(type, format, width, height, pixels, 0) : null;
GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, pixelData);

skia still uses 3.1.44.
https://github.com/google/skia/blob/main/bin/activate-emsdk

[sbc100]

So are you saying that skia was always broken when the garbage free APIs are not used? IIUC the garbage-free APIs didn't always exist, so presumably skia was also broken before that point? (I'm not sure just how long ago it was that they were added but maybe its worth doing some history spelunking here).

[seven332]

Yes. In webgl2, skia was always broken when the garbage free APIs are not used. Because skia use GL_UNPACK_ROW_LENGTH and GR_GL_PACK_ROW_LENGTH, and the length of pixelData is shorter then expected.

[seven332]

The patch for my project is:

sed -i'.original' -e 's/return heap.subarray(toTypedArrayIndex(pixels, heap), toTypedArrayIndex(pixels + bytes, heap));/return heap.subarray(toTypedArrayIndex(pixels, heap));/g' emsdk/src/library_webgl.js

[sbc100]

The patch for my project is:

sed -i'.original' -e 's/return heap.subarray(toTypedArrayIndex(pixels, heap), toTypedArrayIndex(pixels + bytes, heap));/return heap.subarray(toTypedArrayIndex(pixels, heap));/g' emsdk/src/library_webgl.js

I see, so you just take a subarray from the start to the very end of the memory. Does that solve all of these issue? Seems like a fairly reasonable approach since the cost of the subarray is constant regardless, right?

[seven332]

It works for me. It passes all my tests. I haven't received any bug reports related to webgl this week.

[seven332]

The patch for my project is:

sed -i'.original' -e 's/return heap.subarray(toTypedArrayIndex(pixels, heap), toTypedArrayIndex(pixels + bytes, heap));/return heap.subarray(toTypedArrayIndex(pixels, heap));/g' emsdk/src/library_webgl.js

This patch may cause errors on FireFox

WebGL2RenderingContext.readPixels: Argument 7 can't be an ArrayBuffer or an ArrayBufferView larger than 2 GB

[sbc100]

Are you using a memory larger than 2gb?

[seven332]

Yes

[sbc100]

Might be worth filing a bug with FF to see if we can get that limit increased. Would you mind opening a bug here? We can of course try to come up with other ideas here at the same time.

[seven332]

The user who reported this issue was using Firefox 115. I am trying to reproduce this problem on the latest version.

[seven332]

Here it is. #23151