Do you have the latest piglit sources?  In commit 68b0f40dc8f8bae430126d6fbba9f7e7043ad67c I added a check to compare src/dest depth sizes to avoid hitting the glBlitFramebuffer error.

Maybe that's what's going on here.

Yes, I have the latest piglit sources, but it still fails.

My hunch is that we're comparing two formats like MESA_FORMAT_Z24_S8 and MESA_FORMAT_Z24_X8 and failing.

I suspect that GL_INVALID_OPERATION is being generated.

Can you re-run in gdb, set a breakpoint on _mesa_error, then go up one call when it gets hit and print readRb->Format and drawRb->Format and see what the values are?

I probably need to relax the format comparison code.

(In reply to comment #3)                                                        
> My hunch is that we're comparing two formats like MESA_FORMAT_Z24_S8 and
> MESA_FORMAT_Z24_X8 and failing.
> 
> I suspect that GL_INVALID_OPERATION is being generated.

The bug is in the test. Ken's change to make the check accurately reflect the
spec ("We were only comparing the number of depth and stencil bits but the
extension spec actually says the formats must match") just revealed that bug.
The test wrongly assumes that the format of the window-system framebuffer's
depth and stencil buffers is GL_DEPTHSTENCIL_S8_Z24. On Sandybridge and above,
it's not; the window-system uses a separate buffer for depth and stencil.

To be exact:
    drawFb->Attachment[BUFFER_DEPTH]->Format   == MESA_FORMAT_X8_Z24
    drawFb->Attachment[BUFFER_STENCIL]->Format == MESA_FORMAT_S8

    readFb->Attachment[BUFFER_DEPTH]->Format   == MESA_FORMAT_S8_Z24
    readFb->Attachment[BUFFER_STENCIL]->Format == MESA_FORMAT_S8_Z24

To fix the regression by modifying Mesa, we have two options, both of which
are unacceptable to me:
    1. Revert Ken's patch and stop following the spec. If we needed to do
       this to satisfy some closed-source app, then I would consider it. But
       to *deviate from the spec* to fix a test in a *conformance* test
       suite --- that's just wrong.
       this if we got a bug report from an important proprietary app).
    2. Make Intel's window-system framebuffer a chameleon that can adapt to
       the situation to appear as if it has a packed depthstencil buffer or as
       if it has separate depth and stencil buffers. This would be hell to
       implement for little benefit.

To fix the test, though, I don't see a clean way to do it. I don't see a way
for the test to detect this scenario with the GL API and then skip. Perhaps
the test should just skip if GL_INVALID_OPERATION is generated.

Brian, any comment on how to fix the test?
~                                                                               
~                                                                               
~                                                                               
~

Yeah, this is kind of a problem.  There's no way to query through OpenGL the exact formats used by the FBO and window buffers to see if they match.

The test has value in that it checks for potential "upside down" errors when blitting from/to FBOs and Windows so we don't want to throw that out.

My first take on a "fix" is to do you as you said and just skip the blit test if  glBlitFramebuffer() raises GL_INVALID_OPERATION.

Created attachment 55657 [details] [review]
potential fix

Brian,

I've reconsidered some of what I've said, and I think the best thing to do is that, for the window-system framebuffer, Mesa core should require only that the bit sizes match and defer the final blit decision to the driver. Since the user can't query or choose the window-system framebuffer formats, they are really an implementation detail that he need not be concerned with. Mesa should be generous and try to accommodate the user there. After discussing this with Ken, Ian, and Paul, they concurred.

This avoids the strange problem, from the app writer's perspective, that an app that does this kind of blitting worked on gen6 in 7.11 (pre-hiz) but not 8.0 (post-hiz).

Patch is attached. What do you think?

I'm always happy to make life easier for the user so this sounds good.

But one question: why make an exception just for window system FBOs?  I'd be happy with simply reverting my commit that changed the test from bit depths to formats.

Suppose a user creates a depth texture and a depth renderbuffer and wants to blit between them.  It seems to me that the driver might choose different formats for the texture vs. the renderbuffer but they could both be 24-bit.  A path like that might be critical for the app's performance so loosening the test to just compare bit depths seems in all cases seems desirable.

(In reply to comment #7)
> But one question: why make an exception just for window system FBOs?  I'd be
> happy with simply reverting my commit that changed the test from bit depths to
> formats.

I tried to think of a good reason why, but just couldn't.

After rereading the spec, the revert really seems what the spec intended.
The spec allows the user to blit just the depth bits, even if the buffer is packed depthstencil. So there is no reason to care about the stencil bits in that case, whether they're X8 or S8. And, since it's legal to blit the depth and stencil bits separately, then it should be legal to blit from a packed buffer to separate buffers.

Revert away.

Mark RESOLVED/FIXED. Fixed by:

commit f74d8aacbf2f6d140381e272d17c31162a3481b3
Author: Chad Versace <chad.versace@linux.intel.com>
Date:   Tue Jan 17 12:01:34 2012 -0800

    mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

System Environment:
--------------------------
Libdrm:         (master)2.4.30-18-gb643b0713aefdc0611e47654e88263b53b0de6f5
Mesa:           (8.0)caebd7929dca802ece8ef36b0f85094d66133b57
Xserver:                (server-1.11-branch)xorg-server-1.11.3
Xf86_video_intel:              
(master)2.17.0-599-gca252e5b51d7b2f5a7b2c2e0d8fdb024b08096db

verify with mesa 8.0 branch, this case can pass on huronriver. so it has been fixed.

(In reply to comment #9)
> Mark RESOLVED/FIXED. Fixed by:
> commit f74d8aacbf2f6d140381e272d17c31162a3481b3
> Author: Chad Versace <chad.versace@linux.intel.com>
> Date:   Tue Jan 17 12:01:34 2012 -0800
>     mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)