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

[Chad Versace]

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 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.  This patch
   removes the override code.

Fixes several Piglit tests on gen7.

CC: Eric Anholt <eric at anholt.net>
Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
---
 src/mesa/drivers/dri/i965/brw_vtbl.c    |   22 +---------------------
 src/mesa/drivers/dri/intel/intel_span.c |   22 ++++++++++++++--------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vtbl.c b/src/mesa/drivers/dri/i965/brw_vtbl.c
index 7c40f27..7c23faa 100644
--- a/src/mesa/drivers/dri/i965/brw_vtbl.c
+++ b/src/mesa/drivers/dri/i965/brw_vtbl.c
@@ -95,7 +95,7 @@ brw_update_draw_buffer(struct intel_context *intel)
 {
    struct gl_context *ctx = &intel->ctx;
    struct gl_framebuffer *fb = ctx->DrawBuffer;
-   struct intel_renderbuffer *irbDepth = NULL, *irbStencil = NULL;
+   struct intel_renderbuffer *irbStencil = NULL;
    bool fb_has_hiz = intel_framebuffer_has_hiz(fb);
 
    if (!fb) {
@@ -103,27 +103,7 @@ brw_update_draw_buffer(struct intel_context *intel)
       return;
    }
 
-   /*
-    * If intel_context is using separate stencil, but the depth attachment
-    * (gl_framebuffer.Attachment[BUFFER_DEPTH]) has a packed depth/stencil
-    * format, then we must install the real depth buffer at fb->_DepthBuffer
-    * and set fb->_DepthBuffer->Wrapped before calling _mesa_update_framebuffer.
-    * Otherwise, _mesa_update_framebuffer will create and install a swras
-    * depth wrapper instead.
-    *
-    * Ditto for stencil.
-    */
-   irbDepth = intel_get_renderbuffer(fb, BUFFER_DEPTH);
-   if (irbDepth && irbDepth->Base.Format == MESA_FORMAT_X8_Z24) {
-      _mesa_reference_renderbuffer(&fb->_DepthBuffer, &irbDepth->Base);
-      irbDepth->Base.Wrapped = fb->Attachment[BUFFER_DEPTH].Renderbuffer;
-   }
-
    irbStencil = intel_get_renderbuffer(fb, BUFFER_STENCIL);
-   if (irbStencil && irbStencil->Base.Format == MESA_FORMAT_S8) {
-      _mesa_reference_renderbuffer(&fb->_StencilBuffer, &irbStencil->Base);
-      irbStencil->Base.Wrapped = fb->Attachment[BUFFER_STENCIL].Renderbuffer;
-   }
 
    /* Do this here, not core Mesa, since this function is called from
     * many places within the driver.
diff --git a/src/mesa/drivers/dri/intel/intel_span.c b/src/mesa/drivers/dri/intel/intel_span.c
index afb0f32..b103bbd 100644
--- a/src/mesa/drivers/dri/intel/intel_span.c
+++ b/src/mesa/drivers/dri/intel/intel_span.c
@@ -187,10 +187,13 @@ intel_renderbuffer_map(struct intel_context *intel, struct gl_renderbuffer *rb)
    if (!irb)
       return;
 
-   if (irb->wrapped_depth)
-      intel_renderbuffer_map(intel, irb->wrapped_depth);
-   if (irb->wrapped_stencil)
-      intel_renderbuffer_map(intel, irb->wrapped_stencil);
+   if (rb->Data) {
+      /* Renderbuffer is already mapped. This usually happens when a single
+       * buffer is attached to the framebuffer's depth and stencil attachment
+       * points.
+       */
+      return;
+   }
 
    ctx->Driver.MapRenderbuffer(ctx, rb, 0, 0, rb->Width, rb->Height,
 			       GL_MAP_READ_BIT | GL_MAP_WRITE_BIT,
@@ -211,10 +214,13 @@ intel_renderbuffer_unmap(struct intel_context *intel,
    if (!irb)
       return;
 
-   if (irb->wrapped_depth)
-      intel_renderbuffer_unmap(intel, irb->wrapped_depth);
-   if (irb->wrapped_stencil)
-      intel_renderbuffer_unmap(intel, irb->wrapped_stencil);
+   if (!rb->Data) {
+      /* Renderbuffer is already unmapped. This usually happens when a single
+       * buffer is attached to the framebuffer's depth and stencil attachment
+       * points.
+       */
+      return;
+   }
 
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
 
-- 
1.7.7.1