DiligentCore icon indicating copy to clipboard operation
DiligentCore copied to clipboard

OpenGL texture upload segfault (GLES Android)

Open cbauernhofer opened this issue 4 years ago • 4 comments

I've found a bug in your texture upload mechanism for OpenGL on Android. Basically you set the packing alignment to a fixed value of 4 with glPixelStorei(GL_UNPACK_ALIGNMENT, 4);. This can be problematic for 1 or 3 channel textures if the stride of the image is not a multiple of 4 bytes. You should basically check if the stride is a multiple of 8, 4, 2 or 1 and set it accordingly.

I'm not sure if this is GL implementation dependent (as it works on my desktop using OpenGL but not with OpenGL ES on Android). Maybe GL_UNPACK_ROW_LENGTH that you already use is ignored by GLES and therefore it doesn't work on Android. AFAIK GL_UNPACK_ALIGNMENT is used by all implementations.

cbauernhofer avatar Jun 30 '21 13:06 cbauernhofer

Can you provide a reproducer? GL_UNPACK_ALIGNMENT only matters when unpack buffer is set, e.g. when data is copied from a buffer to a texture, which I don't think what you are doing. When updating texture data from CPU memory, it is irrelevant.

The row stride is set by glPixelStorei(GL_UNPACK_ROW_LENGTH, SubresData.Stride / PixelSize);

TheMostDiligent avatar Jun 30 '21 16:06 TheMostDiligent

First I thought that I can easily reproduce it by simply changing your Tutorial03_Texturing.cpp to load a problematic texture (basically any 1channel image with a width that is not dividable by 4). But to my surprise it worked on my desktop and on Android.

So I digged deeper and found the reason. In our code we use OpenCV for all image related operations which include loading and saving. In your samples you use other third party libs (libjpeg, libpng etc). After loading a problematic image I had a look at its raw binary size and its row-stride. Here is the reason why it works on your end:

diligent: 1192288
ours    : 1186246
rows: 2014 cols: 589

(1192288 - 1186246) / 2014 = 3 --> adds 3 bytes to every line

when looking at the stride:
diligent: 592
ours    : 589
--> proofs my assumption as 592 is perfectly dividable by 4

Even though you set the row stride by glPixelStorei(GL_UNPACK_ROW_LENGTH, SubresData.Stride / PixelSize);

it still fails on Android (probably also iOS but I don't have a device here) with a segfault in glTexSubImage2D().

2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #00 pc 00000000002a0f30  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!4d0cdaba868e987aa070f5a6b168e2!85da404!+768) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #01 pc 000000000029cbb4  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!6fd1d11959478379873bee344e3720!85da404!+1140) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #02 pc 000000000026d180  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!e9a0267a4c3f12c4fb16e257d3a26e!85da404!+6192) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #03 pc 000000000027189c  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!9c0715a0352375a9ec27cf88ce6933!85da404!+468) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #04 pc 0000000000236898  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!e94336f9c3a8e90238c7c8557996da!85da404!+2776) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #05 pc 0000000000189910  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!0e6b00ab8c4b112f9f6effa6a8b2b5!85da404!+3720) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #06 pc 0000000000186504  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!4ecf3032464df959aad423cba1a73c!85da404!+1364) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #07 pc 00000000001a5164  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!141e50cb152287019aff218176d094!85da404!+268) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #08 pc 00000000001be678  /vendor/lib64/egl/libGLESv2_adreno.so (!!!0000!838e96e6042a39f699090106d8c25f!85da404!+208) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #09 pc 00000000000c95a0  /vendor/lib64/egl/libGLESv2_adreno.so (glTexSubImage2D+144) (BuildId: 3b3a133255381f2f53bde29433a1b4c2)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #10 pc 00000000000b0314  /data/app/~~Ybmy_wmJKz2vKR3QaMhrGg==/sandbox-chSFbiFTtLua33w8zZ6w8Q==/base.apk!libGraphicsEngineOpenGL.so (offset 0x4587000)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #11 pc 00000000000b007c  /data/app/~~Ybmy_wmJKz2vKR3QaMhrGg==/sandbox-chSFbiFTtLua33w8zZ6w8Q==/base.apk!libGraphicsEngineOpenGL.so (offset 0x4587000)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #12 pc 000000000009b16c  /data/app/~~Ybmy_wmJKz2vKR3QaMhrGg==/sandbox-chSFbiFTtLua33w8zZ6w8Q==/base.apk!libGraphicsEngineOpenGL.so (offset 0x4587000)
2021-07-01 12:52:20.368 8398-8398/? A/DEBUG:       #13 pc 000000000008f6f8  /data/app/~~Ybmy_wmJKz2vKR3QaMhrGg==/sandbox-chSFbiFTtLua33w8zZ6w8Q==/base.apk!libGraphicsEngineOpenGL.so (offset 0x4587000)

As I've had this issue already in the past on mobile platforms I knew that it has to be the GL_UNPACK_ALIGNMENT. So changing it to 1 with glPixelStorei(GL_UNPACK_ALIGNMENT, 1); solves the problem.

Still, this issue is not present on the desktop - that's why I assume that GL and GLES behaves differently in this regard.

cbauernhofer avatar Jul 01 '21 10:07 cbauernhofer

One thing I should have added to my previous answer is the explanation why it's not enough in this case to set the GL_UNPACK_ROW_LENGTH. Looking at the definition (https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml) and my example we have:

s = 1 (8-bit --> 1 byte)
a = 4 (set by you)
n = 1 (single channel)

since s < a:
k = a/s * ⌈(snl)/a⌉
k = 4/1 * ⌈(1*1*589)/4⌉ = 4 * ⌈147.25⌉ = 4 * 148 = 592

So the row-length is set to 592 bytes but the image has a stride of 589 bytes and therefore it makes totally sense that it crashes with a segfault.

By setting a = 1 the other if in the specification gets active and therefore k is computed as:

k = n * l
k = 1 * 589 = 589

The thing I don't understand is why it doesn't happen on the desktop as well.

cbauernhofer avatar Jul 01 '21 13:07 cbauernhofer

Row stride must be aligned by 4 bytes to be consistent with other backends. A debug check that validates parameters is missing, I will add it.

TheMostDiligent avatar Jul 01 '21 15:07 TheMostDiligent