Attachment 55657 Details for Bug 44665 – potential fix

[patch] potential fix

0001-mesa-Enforce-looser-requirements-when-blitting-the-w.patch (text/plain), 7.45 KB, created by Chad Versace on 2012-01-16 17:15:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chad Versace

Created: 2012-01-16 17:15:25 UTC

Size: 7.45 KB

patch

obsolete

>From 3356d9aa433bb3a83ccd2d809fe3d02da6fa1838 Mon Sep 17 00:00:00 2001
>From: Chad Versace <chad.versace@linux.intel.com>
>Date: Mon, 16 Jan 2012 15:31:48 -0800
>Subject: [PATCH] mesa: Enforce looser requirements when blitting the
> window-system framebuffer
>
>Fixes Piglit fbo/fbo-blit-d24s8 on intel/gen6 (hiz enabled) and intel/gen7
>(hiz irrelevant). The test was broken by commit:
>    commit 3f1fab06844f696de44d9a56e83ff62e8ea576bd
>    Author: Brian Paul <brianp@vmware.com>
>    Date:   Mon Jan 9 08:11:33 2012 -0700
>
>        mesa: check depth, stencil formats (not depths) in glBlitFramebuffer
>
>        We were only comparing the number of depth and stencil bits but the
>        extension spec actually says the formats must match:
>
>The problem is that the Intel driver creates a separate depth and stencil
>buffer for the window-system framebuffer. Since the user is unable to
>choose or query the format of the window-system depth and stencil buffers,
>Mesa core should be generous: when blitting the window-system depth and
>stencil buffers, only their bit sizes, only require that the bit sizes
>match and defer final blit approval to the driver.
>
>To eliminate comment confusion, I've moved the spec quotes into their
>respective if-branches. I've also made the GL error messages more
>informative.
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
>CC: Brian Paul <brianp@vmware.com>
>Note: This is a candidate for the 8.0 branch.
>Signed-off-by: Chad Versace <chad.versace@linux.intel.com>
>---
> src/mesa/main/fbobject.c |   96 +++++++++++++++++++++++++++++++--------------
> 1 files changed, 66 insertions(+), 30 deletions(-)
>
>diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>index 4d649ad..4ae7992 100644
>--- a/src/mesa/main/fbobject.c
>+++ b/src/mesa/main/fbobject.c
>@@ -2700,33 +2700,69 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
>       struct gl_renderbuffer *drawRb =
>          drawFb->Attachment[BUFFER_STENCIL].Renderbuffer;
> 
>-      /* From the EXT_framebuffer_object spec:
>-       *
>-       *     "If a buffer is specified in <mask> and does not exist in both
>-       *     the read and draw framebuffers, the corresponding bit is silently
>-       *     ignored."
>-       */
>       if ((readRb == NULL) || (drawRb == NULL)) {
>+	 /* From the EXT_framebuffer_object spec:
>+	  *
>+	  *     If a buffer is specified in <mask> and does not exist in both
>+	  *     the read and draw framebuffers, the corresponding bit is
>+	  *     silently ignored.
>+	  */
> 	 mask &= ~GL_STENCIL_BUFFER_BIT;
>       }
>-      else {
>+      else if (_mesa_is_winsys_fbo(readFb) || _mesa_is_winsys_fbo(drawFb)) {
>+	 /* From the EXT_framebuffer_blit spec:
>+	  *
>+	  *     Attempting to blit between depth or stencil buffers of
>+	  *     different [bit] size generates INVALID_OPERATION.
>+	  *
>+	  * The requirement enforced here (bit size must match) is weaker than
>+	  * that enforced below for non-window-system renderbuffers (formats
>+	  * must match). There is some ambiguity in the spec's intent, because
>+	  * the two spec quotes occur in two different sections and their
>+	  * intents seem to differ slightly. Since the user is unable to
>+	  * choose or query the format of the window-system depth and stencil
>+	  * buffers with the GL API, only their bit sizes, Mesa chooses to be
>+	  * generous: require only that the bit sizes match and defer the
>+	  * final blit decision to the driver.
>+	  *
>+	  * This generosity of interpretation is intended to work around the
>+	  * following problem. Suppose the user requests a framebuffer config
>+	  * with 24 depth bits and 8 stencil bits. The driver may choose to
>+	  * create system framebuffer with a single Z24_S8 buffer or separate
>+	  * Z24_X8 and S8 buffers. Since the user is unable to influence or
>+	  * query the formats, they are an implementation detail that should
>+	  * not concern him.
>+	  */
> 	 GLint readBits = _mesa_get_format_bits(readRb->Format,
> 	                                        GL_STENCIL_BITS);
> 	 GLint drawBits = _mesa_get_format_bits(drawRb->Format,
> 	                                        GL_STENCIL_BITS);
> 	 if (readBits != drawBits) {
>-	    /* From the EXT_framebuffer_blit spec:
>-	     *   Attempting to blit between depth or stencil buffers of
>-	     *   different size generates INVALID_OPERATION.
>-	     */
> 	    _mesa_error(ctx, GL_INVALID_OPERATION,
>-			"glBlitFramebufferEXT: stencil buffer sizes do not"
>-			"match: read_format=%s draw_format=%s\n",
>-			_mesa_get_format_name(readRb->Format),
>-			_mesa_get_format_name(drawRb->Format));
>+	                "glBlitFramebufferEXT: stencil buffer bit sizes do not"
>+	                "match: read_bits=%d draw_bits=%d\n",
>+	                readBits, drawBits);
> 	    return;
> 	 }
>       }
>+      else if (readRb->Format != drawRb->Format) {
>+	 /* From the EXT_framebuffer_blit spec:
>+	  *
>+	  *     Calling BlitFramebufferEXT will result in an INVALID_OPERATION
>+	  *     error if <mask> includes DEPTH_BUFFER_BIT or
>+	  *     STENCIL_BUFFER_BIT and the source and destination depth and
>+	  *     stencil buffer formats do not match.
>+	  *
>+	  * Note that the requirement enforced here is stronger than that
>+	  * above enforced for window-system framebuffers.
>+	  */
>+	 _mesa_error(ctx, GL_INVALID_OPERATION,
>+	             "glBlitFramebufferEXT: stencil buffer formats do not"
>+	             "match: read_format=%s draw_format=%s\n",
>+	             _mesa_get_format_name(readRb->Format),
>+	             _mesa_get_format_name(drawRb->Format));
>+	 return;
>+      }
>    }
> 
>    if (mask & GL_DEPTH_BUFFER_BIT) {
>@@ -2735,31 +2771,31 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
>       struct gl_renderbuffer *drawRb =
>          drawFb->Attachment[BUFFER_DEPTH].Renderbuffer;
> 
>-      /* From the EXT_framebuffer_object spec:
>-       *
>-       *     "If a buffer is specified in <mask> and does not exist in both
>-       *     the read and draw framebuffers, the corresponding bit is silently
>-       *     ignored."
>-       */
>       if ((readRb == NULL) || (drawRb == NULL)) {
>+	 /* See comment above in analogous section for stencil buffer. */
> 	 mask &= ~GL_DEPTH_BUFFER_BIT;
>       }
>-      else {
>+      else if (_mesa_is_winsys_fbo(readFb) || _mesa_is_winsys_fbo(drawFb)) {
>+	 /* See comment above in analogous section for stencil buffer. */
> 	 GLint readBits = _mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS);
> 	 GLint drawBits = _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS);
> 	 if (readBits != drawBits) {
>-	    /* From the EXT_framebuffer_blit spec:
>-	     *   Attempting to blit between depth or stencil buffers of
>-	     *   different size generates INVALID_OPERATION.
>-	     */
> 	    _mesa_error(ctx, GL_INVALID_OPERATION,
>-			"glBlitFramebufferEXT: depth buffer sizes do not"
>-			"match: read_format=%s draw_format=%s\n",
>-			_mesa_get_format_name(readRb->Format),
>-			_mesa_get_format_name(drawRb->Format));
>+	                "glBlitFramebufferEXT: depth buffer bit sizes do not"
>+	                "match: read_bits=%d draw_bits=%d\n",
>+	                readBits, drawBits);
> 	    return;
> 	 }
>       }
>+      else if (readRb->Format != drawRb->Format) {
>+	 /* See comment above in analogous section for stencil buffer. */
>+	 _mesa_error(ctx, GL_INVALID_OPERATION,
>+	             "glBlitFramebufferEXT: depth buffer formats do not"
>+	             "match: read_format=%s draw_format=%s\n",
>+	             _mesa_get_format_name(readRb->Format),
>+	             _mesa_get_format_name(drawRb->Format));
>+	 return;
>+      }
>    }
> 
>    if (readFb->Visual.samples > 0 &&
>-- 
>1.7.7.4
>

Actions: View | Splinter Review

Attachments on bug 44665: 55657

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.