This is triggered on a HSW (no idea about other hardware): $ bin/fbo-depth copypixels GL_DEPTH_COMPONENT32F -auto ... Mesa 10.7.0-devel implementation error: Incorrectly writing swrast's integer depth values to MESA_FORMAT_Z_FLOAT32 depth buffer Please report at https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa See full output at http://people.freedesktop.org/~imirkin/all-comparison/hsw-2015-07-13-ilia/spec@arb_depth_buffer_float@fbo-depth-gl_depth_component32f-copypixels.html nouveau and radeonsi appear to cope with this just fine.
The right answer must be to fix the pixel transfer routines to do the copy properly, but we can workaround the issue and ignore it for another day: diff --git a/src/mesa/drivers/dri/i965/intel_pixel_copy.c b/src/mesa/drivers/dri/i965/intel_pixel_copy.c index 4313588..910f334 100644 --- a/src/mesa/drivers/dri/i965/intel_pixel_copy.c +++ b/src/mesa/drivers/dri/i965/intel_pixel_copy.c @@ -81,12 +81,38 @@ do_blit_copypixels(struct gl_context * ctx, read_irb = intel_renderbuffer(read_fb->Attachment[BUFFER_DEPTH].Renderbuffer); break; + + /* We can only blit between separate depth/stencil buffers of identical + * pitches (so that W-tiling anomalies are hidden by identical swizzling + * on both sides). + */ case GL_DEPTH: - perf_debug("glCopyPixels() fallback: GL_DEPTH\n"); - return false; + draw_irb = + intel_renderbuffer(fb->Attachment[BUFFER_DEPTH].Renderbuffer); + read_irb = + intel_renderbuffer(read_fb->Attachment[BUFFER_DEPTH].Renderbuffer); + if (draw_irb->mt->format != MESA_FORMAT_Z_FLOAT32 || + read_irb->mt->format != MESA_FORMAT_Z_FLOAT32) { + perf_debug("glCopyPixels() fallback: GL_DEPTH\n"); + return false; + } + break; case GL_STENCIL: - perf_debug("glCopyPixels() fallback: GL_STENCIL\n"); - return false; + draw_irb = + intel_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer); + read_irb = + intel_renderbuffer(read_fb->Attachment[BUFFER_STENCIL].Renderbuffer); + if (draw_irb->mt->format != MESA_FORMAT_S_UINT8 || + read_irb->mt->format != MESA_FORMAT_S_UINT8) { + perf_debug("glCopyPixels() fallback: GL_STENCIL\n"); + return false; + } + if (draw_irb->mt->stencil_mt || read_irb->mt->stencil_mt) { + /* Too much complexity for proof-of-principle */ + perf_debug("glCopyPixels() fallback: GL_STENCIL\n"); + return false; + } + break; default: perf_debug("glCopyPixels(): Unknown type\n"); return false; @@ -112,7 +138,7 @@ do_blit_copypixels(struct gl_context * ctx, return false; } - if (ctx->Depth.Test) { + if (ctx->Depth.Test && ctx->Depth.Func != GL_ALWAYS) { perf_debug("glCopyPixels(): Unsupported depth test state\n"); return false; }
keep in mind that depth is Y-tiled - only stencil is W-tiled.
https://patchwork.freedesktop.org/patch/249252/ fixes fail. When problem was reported intelCopyPixels() was using do_blit_copypixels(). Now intelCopyPixels() uses _mesa_meta_CopyPixels(). And pack_uint_Z_FLOAT32 doesn't work correctly. I'm looking for a way of not receiving this message(Mesa 10.7.0-devel implementation error: Incorrectly writing swrast's integer depth values to MESA_FORMAT_Z_FLOAT32 depth buffer)
(In reply to Illia from comment #3) > https://patchwork.freedesktop.org/patch/249252/ fixes fail. When problem was > reported intelCopyPixels() was using do_blit_copypixels(). Now > intelCopyPixels() uses _mesa_meta_CopyPixels(). And pack_uint_Z_FLOAT32 > doesn't work correctly. > I found your v2 (https://patchwork.freedesktop.org/patch/249295/). Good catch. I don't think we can delete pack_uint_Z_FLOAT32_X24S8 however, we need to copy the S8 component of the buffer. I'm not sure why this test case still works with your patch: $ bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto Maybe we need to adjust the piglit test? > I'm looking for a way of not receiving this message(Mesa 10.7.0-devel > implementation error: Incorrectly writing swrast's integer depth values to > MESA_FORMAT_Z_FLOAT32 depth buffer) You can find the code which emits this error by grepping for part of the message in the tree. I like using `git grep` personally.
(In reply to Nanley Chery from comment #4) > I don't think we can delete pack_uint_Z_FLOAT32_X24S8 however, we need to > copy the S8 component of the buffer. I'm not sure why this test case still > works with your patch: > > $ bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto > > Maybe we need to adjust the piglit test? Test works correct. I've greped mesa and found only one mention to "pack_uint_Z_FLOAT32_X24S8" there is in "_mesa_get_pack_uint_z_func" and I've edited its behavior. When something calls "_mesa_get_pack_uint_z_func" with "MESA_FORMAT_Z32_FLOAT_S8X24_UINT" it will get "pack_uint_Z_FLOAT32" it works like this in "_mesa_get_pack_float_z_func". I think we can add comments to both functions if someone will look for "pack_uint_Z_FLOAT32_X24S8" or "pack_float_Z_FLOAT32_X24S8". > > I'm looking for a way of not receiving this message(Mesa 10.7.0-devel > > implementation error: Incorrectly writing swrast's integer depth values to > > MESA_FORMAT_Z_FLOAT32 depth buffer) > > You can find the code which emits this error by grepping for part of the > message in the tree. I like using `git grep` personally. Thanks, I've found commit which implement sending this error. It close related with our bug. I think we can painless revert this commit. it's done its work. commit fceff14450e48a90778e0d1e79c13fa7a40631e6 Author: Eric Anholt <eric@anholt.net> mesa: Add a _mesa_problem to document a piglit failure on i965. Having figured out what was going on with piglit fbo-depth copypixels GL_DEPTH_COMPONENT32...
Test case ./bin/fbo-depth copypixels GL_DEPTH_COMPONENT32F -auto was verified on the mesa without Illia's patch and it failed. Also after applying Illia's patch on the newest 18.3.0 mesa this test case got pass. Also test case ./bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto is passed with and without this patch. Following environment was used: Haswell; Intel® HD Graphics 4600 ; Intel Core i5-4300M 16.04.1-Ubuntu kernel 4.15.0-34-generic
Also no suspicious output after test cases execution is observed like "Mesa 18.3.0-devel implementation error: Incorrectly writing swrast's integer depth values to MESA_FORMAT_Z32_FLOAT_S8X24_UINT depth buffer" for './bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto' and './bin/fbo-depth copypixels GL_DEPTH_COMPONENT32F -auto' with Illia's "[v3] mesa/format_pack: Fix pack_uint_Z_FLOAT32()" patch (https://patchwork.freedesktop.org/patch/254261/). Now output for test cases execution is user@HRK1-LHP-F49171:~/Work/piglit$ ./bin/fbo-depth copypixels GL_DEPTH_COMPONENT32F -auto Testing GL_DEPTH_COMPONENT32F. Testing glCopyPixels(depth). PIGLIT: {"result": "pass" } user@HRK1-LHP-F49171:~/Work/piglit$ ./bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto Testing GL_DEPTH32F_STENCIL8. Testing glCopyPixels(depth). PIGLIT: {"result": "pass" } Environment: Haswell; Intel® HD Graphics 4600 ; Intel Core i5-4300M 16.04.1-Ubuntu kernel 4.15.0-34-generic
(In reply to Illia from comment #5) > (In reply to Nanley Chery from comment #4) > > > I don't think we can delete pack_uint_Z_FLOAT32_X24S8 however, we need to > > copy the S8 component of the buffer. I'm not sure why this test case still > > works with your patch: > > > > $ bin/fbo-depth copypixels GL_DEPTH32F_STENCIL8 -auto > > > > Maybe we need to adjust the piglit test? > > Test works correct. I've greped mesa and found only one mention to > "pack_uint_Z_FLOAT32_X24S8" there is in "_mesa_get_pack_uint_z_func" and > I've edited its behavior. When something calls > "_mesa_get_pack_uint_z_func" with "MESA_FORMAT_Z32_FLOAT_S8X24_UINT" it will > get "pack_uint_Z_FLOAT32" it works like this in > "_mesa_get_pack_float_z_func". > You're right. I think I understand how this function works now. It packs a uint_Z value into a Z32_X24S8 and aims to leave the stencil component alone. Sorry for the noise. > I think we can add comments to both functions if someone will look for > "pack_uint_Z_FLOAT32_X24S8" or "pack_float_Z_FLOAT32_X24S8". > > > > I'm looking for a way of not receiving this message(Mesa 10.7.0-devel > > > implementation error: Incorrectly writing swrast's integer depth values to > > > MESA_FORMAT_Z_FLOAT32 depth buffer) > > > > You can find the code which emits this error by grepping for part of the > > message in the tree. I like using `git grep` personally. > Thanks, I've found commit which implement sending this error. It close > related with our bug. I think we can painless revert this commit. it's done > its work. > > commit fceff14450e48a90778e0d1e79c13fa7a40631e6 > Author: Eric Anholt <eric@anholt.net> > > mesa: Add a _mesa_problem to document a piglit failure on i965. > > Having figured out what was going on with piglit fbo-depth copypixels > GL_DEPTH_COMPONENT32... Sounds good. I'll leave some comments on your v3.
This issue is fixed by commit, commit b18f8e63ef79b5098cb32de1670983ad1e6892fe Author: Illia Iorin <illia.iorin@gmail.com> Date: Thu Oct 11 18:06:18 2018 +0300 mesa: Fix pack_uint_Z_FLOAT32() Fixed pack_uint_Z_FLOAT32 by casting row data to float instead uint. Remove code duplicate function pack_uint_Z_FLOAT32_X24S8. Edited case in "_mesa_get_pack_uint_z_func". Now it looks like "_mesa_get_pack_float_z_func". Remove _mesa_problem call, which was added for debuging this issue. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91433 Signed-off-by: Illia Iorin <illia.iorin@globallogic.com> Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
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.