-
Notifications
You must be signed in to change notification settings - Fork 3.4k
glCompressedTexImage2D error #19300
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
Comments
Is there any update? |
Its hard to say what this might be without more details. Can you share a full backtrace? And the line of code at the call site? If possible a complete program would be even better. There are several tests in the |
We started experiencing this problem too after upgrading from emscripten version 3.1.39 to version 3.1.64. Here is a minimal repro based on the anisotropic test:
Compile like this:
The output is:
The two necessary conditions to reproduce the issue are to use This is how glCompressedTexImage2D implementation looks in 3.1.39:
And this is how it looks in 3.1.64:
The different part is |
Thanks for the testcase @matusfedorko , I bisected this to "Avoid garbage-free WebGL APIs when memory size is over 2gb." (#21445) @sbc100 PTAL |
What is the i.e. what does it mean to set the pixels argument to NULL? It seems like before #21445 we would end up calling I'm not sure how this is supposed to work I guess. @juj might know? Also @kainino0x ? |
According the MDN docs it should be possible to call |
Actually the 6 argument version fails here with webgl1 too: |
So this code also fails without memory growth on WebGL1: |
Doing a little research it sounds like passing From the docs it looks like maybe glTexImage2D supports this See: https://community.khronos.org/t/glcompressedteximage2d-and-null-data/41505 Node that glTexImage2D mentions "data may be a null pointer. In this case, texture memory is allocated to accommodate a texture of width width and height height." but ping @kainino0x @juj |
Yes, we pass NULL to allocate space for the texture. I tried that suggestion with glTexImage2D, but it does not work. In desktop GL (NVidia) the output is all black and in Chrome I get some error (I don't remember what it was exactly, but if you want you can try to modify the repro sample). I also noticed that reference docs do not explicitly mention NULL, but docs are not specification and it seems only logical to me that glCompressedTexImage2D/glCompressedTexSubImage2D would behave analogously to glTexImage2D/glTexSubImage2D. |
Actually. in the OpenGL 4.6 specification (https://registry.khronos.org/OpenGL/specs/gl/glspec46.core.pdf) on page 230 I found this:
|
Sadly i looks like I can't omit the last argument or set it to null using WebGL2's I wonder if this is bug since https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/compressedTexImage2D mentions a six argument version in WebGL1 and says that that WebGL API adds overloads. |
Perhaps we can map https://registry.khronos.org/webgl/specs/latest/1.0/#COMPRESSEDTEXIMAGE2D
@kenrussell maybe you can help here? |
And how about the first overload |
Well, so I did some tests. That overload did not work (maybe because I have WebGL 2 context and not 1), but |
Posted a note over to Khronos GitHub tracker. Indeed it looks like Emscripten code has all this time been uploading garbage bytes starting from memory address 0. I don't remember if that was intentional, or if it was accidental. However, I recommend to restore that original behavior, and add a comment in the code to point to KhronosGroup/WebGL#3686 to explain why that is done. |
What about the idea of using |
@matusfedorko does you sample code above actually draw anything to screen? I'm like to use it as a test case for landing this fix, but it would be good if it actually used a compressed subimage and drew something I think. |
Unfortunately calling Or later calling |
It generates an error (I tested in Chrome). Precisely |
No, but feel free to adjust it so that it does. I did not have time to make anything fancy, but a barebones sample that demonstrates the issue clearly. |
I see. How about passing Or we could us a static empty buffer than we |
From the beginning of the WebGL spec, It was only during the introduction of WebGL 2.0, when applications could control textures' Therefore, yes, if the application passes NULL to @matusfedorko there is no overload https://registry.khronos.org/webgl/specs/latest/1.0/ https://registry.khronos.org/webgl/specs/latest/2.0/ and are: For WebGL 1.0:
From WebGL 2.0, taking the compressed texture data from a (GPU-side) pixel buffer object:
and WebGL 2.0's "garbage-free" entry point which selects the region from the passed ArrayBufferView:
|
This is the part that doesn't seem to be working according to @matusfedorko. Hopefully I can create a test/sample to verify this. |
Oh wait, but
Thanks for the info. I went off of the MDN docs in my comment, which show that overload in the first place. |
Note that We can not call
That would work, but would be pretty disappointing. If someone has their "GPU memory is compromised, but CPU memory is intact" op-sec hat on, then we could certainly do that. |
Agreed; as I mentioned above, such allocations are not cheap and are also not guaranteed to be promptly garbage collected. It would be better to initialize the compressed texture with whatever random data is at address 0 in the Wasm heap, as Jukka points out the code used to work before. |
How about reusing a global allocation:
That would limit us to single allocation that only happens rarely, and avoid reading from address zero. Regardless is seems like a bug in the spec that the final argument it not optional right? |
Textures in 3D applications tend to be very large. This global allocation will be wasted space that is only present in order to give these compressed textures zeroed-out data. The data in the Wasm heap is controlled entirely by the application; there is no chance that any data uploaded to the texture will cause a leak of information that the application shouldn't be able to see. The guarantees in OpenGL ES's C API are much weaker than those provided by WebGL, where nearly all undefined behavior was resolved and turned into well-defined behavior. Applications compiled with Emscripten are only relying on the C API's semantics. For this reason I once again strongly support @juj 's suggestion to just upload data from the 0 address in the Wasm heap if the app calls |
OK lets go with that previous/odd behaviour. Is it compatible though? I thought passing NULL as the last argument guaranteed that the new space was zero-initialized. The data at address zero in the wasm memory is likely not going to be zeros. We put C static data at address 1024, and we write cookies to the first 8 bytes of memory. |
The GLES 3.0 spec says (per Jukka's comment above) that if NULL is passed as the |
I believe this has always been an issue with emscripten's WebGL1 code which only recently became more prevalent with emscripten-core#21445. Fixes: emscripten-core#19300
I believe this has always been an issue with emscripten's WebGL1 code which only recently became more prevalent with emscripten-core#21445. Fixes: emscripten-core#19300
Hi!
I got the error "Uncaught TypeError TypeError: Failed to execute 'compressedTexImage2D' on 'WebGLRenderingContext': parameter 7 is not of type 'ArrayBufferView'", when I called glCompressedTexImage2D and passed null to the "data" parameter. How to avoid it? Is there any workaround?
The text was updated successfully, but these errors were encountered: