[Mesa-dev] [PATCH 1/2] swrast: Fix fastpaths in glRead/WritePixels(GL_DEPTH_STENCIL)

[Brian Paul]

On 10/11/2011 09:57 PM, Eric Anholt wrote:
> On Mon, 10 Oct 2011 18:10:59 -0700, Chad Versace<chad at chad-versace.us>  wrote:
>> For glReadPixels, the user supplied pixels have format
>> GL_UNSIGNED_INT_24_8.  But, when the depthstencil buffer's format was
>> MESA_FORMAT_S8_Z24, the fastpath read from the buffer without reordering
>> the depth and stencil bits. To fix this, this patch just skips the
>> fastpath when the format is not MESA_FORMAT_Z24_S8.
>>
>> The problem and fix for glWritePixels is analagous.
>>
>> Fixes the Piglit tests below on i965/gen6 and causes no regressions.
>>     general/depthstencil-default_fb-drawpixels-24_8
>>     general/depthstencil-default_fb-readpixels-24_8
>>     EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-24_8
>>     EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-readpixels-24_8
>>
>> Signed-off-by: Chad Versace<chad at chad-versace.us>
>
> This code is all ridiculous -- while this fixes the problem, the real
> issue is that GetRow is returning GL_UNSIGNED_INT_24_8 data whose
> ordering actually depends on the underlying renderbuffer format.  That's
> not how I understand GetRow is supposed to work.
>
> But we all hate GetRow and friends, I think, so I've started on a branch
> doing to renderbuffers what was done to textures with MapTextureImage.
> It's in the rbmap branch of my Mesa tree.  Non-intel swrast users are
> broken by it for now.

I was going to tackle renderbuffers next but I'm happy to let you do 
it. :)

I took a quick look at your branch it looks great so far.

-Brian