-----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.vers...@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.vers...@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----- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev