Bug 91433

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>