I was trying to make "high fidelity" work on mesa, but with my limited OpenGL knowledge maybe the code is wrong. This is the call in question: https://github.com/ChristophHaag/hifi/blob/ec6514b13b688a2aa6154359233dcb3f3186cc0e/libraries/gpu-gl/src/gpu/gl45/GL45BackendTexture.cpp#L161-L162 It causes this loop in compressed_tex_sub_image() to fail: https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/teximage.c#n4955 The incoming texture has width x height 64x64. Then as I understand it the texImage should be just a part of that texture, however texImage->Width x texImage->Height is 64x64 too. with that width and height, image_stride gets calculated to be 4096, which is problematic, because imageSize was 4096 too. So the loop ends up subtracting image_stride=4096 from imageSize=4096 six times. Sometimes this segfaults, sometimes it runs into an assertion. As I said, maybe the application code is wrong, but I think there is at least some error checking in mesa missing. valgrind also reported an invalid read in some maybe relevant code https://gist.github.com/ChristophHaag/1af7eb4ef207a397460ff2c6719eba2e
That seems like a bug... /* Copy in each face. */ for (int i = 0; i < 6; ++i) { That should probably be for (int i = zoffset; i < zoffset + depth; i++) { This feels somehow familiar... either it's been fixed before, or the decision was that it's actually improper to do this. Or a patch was sent but never applied. Unfortunately it's just a faint memory.
No wonder this felt familiar. commit 2259b111003f2e8c55cae42677ec45345fb1b6e3 Author: Ilia Mirkin <imirkin@alum.mit.edu> Date: Thu Aug 27 15:28:24 2015 -0400 mesa: only copy the requested teximage faces Cube maps are special in that they have separate teximages for each face. We handled that by copying the data to them separately, but in case zoffset != 0 or depth != 6 we would read off the end of the client array or modify the wrong images. zoffset/depth have already been verified by the time the code gets to this stage, so no need to double-check. Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> Reviewed-by: Brian Paul <brianp@vmware.com> Cc: "10.6 11.0" <mesa-stable@lists.freedesktop.org> But I only applied it to regular tex images, not to compressed ones.
Tried it and hifi works with it. Thanks! diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index 7bcd734204..e5dc469437 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -4968,13 +4968,13 @@ compressed_tex_sub_image(unsigned dim, GLenum target, GLuint texture, } /* Copy in each face. */ - for (int i = 0; i < 6; ++i) { + for (int i = zoffset; i < zoffset + depth; ++i) { texImage = texObj->Image[i][level]; assert(texImage); compressed_texture_sub_image(ctx, 3, texObj, texImage, texObj->Target, level, xoffset, yoffset, - zoffset, width, height, 1, format, + 0, width, height, 1, format, imageSize, pixels); /* Compressed images don't have a client format */
Care to send a proper patch?
https://lists.freedesktop.org/archives/mesa-dev/2017-August/166827.html
Pushed. commit 87556a650ad363b41d86f4e25d5c4696f9af4550 Author: Christoph Haag <haagch@frickel.club> Date: Sun Aug 20 01:59:43 2017 +0200 mesa: only copy requested compressed teximage cubemap faces This is analogous to commit 2259b11 which only fixed the regular case Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102308 Signed-off-by: Christoph Haag <haagch+mesadev@frickel.club> Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu> Cc: mesa-stable@lists.freedesktop.org
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.