[Mesa-dev] [PATCH 5/6] intel: Fix swrast_render_start() for depthstencil buffers with separate stencil

[Chad Versace]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/14/2011 02:05 PM, Eric Anholt wrote:
> On Sun, 13 Nov 2011 22:32:14 -0800, Chad Versace <chad.versace at linux.intel.com> wrote:
>> This patch fixes three interrelated bugs.
>>
>> Fixes many depthstencil Piglit tests on gen7, too many to list.
>>
>> 1. Don't map the depthstencil buffer twice
>> ------------------------------------------
>> Place a guard in intel_renderbuffer_map() to prevent a renderbuffer from
>> being mapped twice. This happened if a single buffer was attached to the
>> framebuffer's depth and stencil attachment points.  (Interestingly,
>> because intel_map_renderbuffer_gtt() is idempotent, the double mapping did
>> not cause bugs for depthstencil buffers *without* separate stencil).
>>
>> 2. Stop using the special stencil span accessors
>> ------------------------------------------------
>> The special stencil span accessors, as set by intel_span_init_funcs.
>> perform software W detiling. Since intel_renderbuffer_map() now uses
>> MapRenderbuffer, rb->Data points to an *untiled* stencil buffer.
>>
>> 3. Stop overriding gl_framebuffer::_DepthBuffer,_StencilBuffer
>> --------------------------------------------------------------
>> Normally, if a depthstencil buffer is attached to the framebuffer's depth
>> attachment point, then _mesa_update_framebuffer() installs a wrapper depth
>> renderbuffer at gl_framebuffer::_DepthBuffer. Ditto for the stencil
>> attachment point and gl_framebuffer::_StencilBuffer
>>
>> A depthstencil intel_renderbuffer with separate stencil contains hidden
>> depth and stencil renderbuffers, which are the *real* renderbuffers. In
>> order to force swrast to work, we were installing, in
>> brw_update_draw_buffer(), the hidden renderbuffers at
>> gl_framebuffer::_DepthBuffer and _StencilBuffer, thus overriding the
>> behavior of _mesa_update_framebuffer().  However, now that
>> intel_renderbuffer_map() is implemented with MapRenderbuffer(), overriding
>> _mesa_update_framebuffer's introduces bugs (You really don't want the
>> horrid, detailed explanation, so I'll spare you...). This patch removes
>> the override code.
> 
> I was following you up to this point.  So, Reviewed-by: for me for the
> entire series up to 2/3 of this (except for that one add-a-comment
> comment).  I believe you that there's something going wrong when we
> override _DepthBuffer and _StencilBuffer, I just don't know exactly
> what.

Without 3, there were lots of bugs. But now that you ask, I don't remember
exactly why the bug happened. I'm digging into that now.
(I wrote the patch at midnight, and I dont' remember much from that hour...) 

> I'm guessing that all of these 3 patches in this commit were squashed
> together because nothing worked without all of them?

No, it is just that I viewed them fundamentally as three aspects of the same
bug: swrast_render_start() was broken by MapRenderbuffer for separate stencil.
Since the patch so naturally decomposes, I'll split it up.

- ----
Chad Versace
chad.versace at linux.intel.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOwZZeAAoJEAIvNt057x8idpsQAIOHAbrm+QOBlE2tM6B0jNnT
fiF1YL/Qm7W4H6+/pmzRJA9GDSQU8c7gC5lgWmzHygYVcXL0aIYrhAm01zrFY20H
xhCfVAvKmw3ZXV30gIqCYD0gPPxdDitASyfo9zNCNytJVzg2DmerQWWwz+rLi2fM
WJDi3kUz1kAloUevAJCY1mJWL0fZVPn2r+91oBGacnk5XeYA+OqDInXIOkFeXPCH
itaYngGjQ5EpSd0nT6ttihMvfB4GfwMvK/9YXB9lRmEQ8okul9snz/OYkz3DNuNq
BPs5gs1QIN9duOWM/Uvs5E+aL9VqKNAWdqelYiEgf9aK/Moxv5hQpcgP+lnx4nxX
UjaIOAUsDv6G6U46vhy3huLulIA21Tas5LdVX9FC1VgkFDZiOjX9GhGvbak7/n4K
ZTOmgfgfjaIgBLAVxsAUj6+H1ZR3jMsyJ7dE31hByVRK4Grx8mj37xgCmTlmjkXD
MHLIeNvLdCg6n/TjxtZ8+DMS+bxRlbva1Tq1+51YMqetTqOTfzFfsMofIw90zfrI
95S0ESiG9mOmy/asIX/LfsD9rEO2pfAG7Z/zUFDglQlRjyTTSLl/+3rLg86YPgvl
XJIVhmEeb6+IfG4M83plI7NSoL6F+oCDuhmsa4Man/w4rGH60Htt+WcBfAtRYKBU
ODFO1WJAHww3JmNOK4vL
=4ycR
-----END PGP SIGNATURE-----