[Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

[Ian Romanick]

On 01/18/2012 12:31 PM, Chad Versace wrote:
> --- snip
>
> Supporting Z32 ->  Z32_FLOAT is a bad idea. So I've amended the patch
> to avoid that.
>
> --- end snip
>
>
> This loosens the format validation in glBlitFramebuffer. When blitting
> depth bits, don't require an exact match between the depth formats; only
> require that the two formats have the same number of depth bits and the
> same depth datatype (float vs uint). Ditto for stencil.
>
> Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
> glBlitFramebuffer to blit the depth and stencil bits separately. So I see
> no reason to prevent blitting the depth bits between X8_Z24 and S8_Z24 or
> the stencil bits between S8 and S8_Z24. However, we of course don't want
> to allow blitting from Z32 to Z32_FLOAT.
>
> --------
>
> Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil
> enabled.
>
> The problem was that, on Intel drivers with separate stencil, the default
> framebuffer has separate depth and stencil buffers with formats X8_Z24 and
> S8. The test attempts to blit the depth bits from a S8_Z24 buffer into the
> default framebuffer.
>
> v2: Check that depth datatypes match in order prevent Z32 ->  Z32_FLOAT.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
> Note: This is a candidate for the 8.0 branch.
> CC: Brian Paul<brianp at vmware.com>
> CC: Kenneth Graunke<kenneth at whitecape.org>
> CC: Iam Romanick<idr at freedesktop.org>

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> Reported-by: Xunx Fang<xunx.fang at intel.com>
> Signed-off-by: Chad Versace<chad.versace at linux.intel.com>
> ---
>   src/mesa/main/fbobject.c |   13 ++++++++++---
>   1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 0524959..2b3ac2e 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -2709,9 +2709,13 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
>         if ((readRb == NULL) || (drawRb == NULL)) {
>   	 mask&= ~GL_STENCIL_BUFFER_BIT;
>         }
> -      else if (readRb->Format != drawRb->Format) {
> +      else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
> +	       _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
> +	 /* There is no need to check the stencil datatype here, because
> +	  * there is only one: GL_UNSIGNED_INT.
> +	  */
>            _mesa_error(ctx, GL_INVALID_OPERATION,
> -                     "glBlitFramebufferEXT(stencil buffer format mismatch)");
> +                     "glBlitFramebufferEXT(stencil buffer size mismatch)");
>            return;
>         }
>      }
> @@ -2731,7 +2735,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
>         if ((readRb == NULL) || (drawRb == NULL)) {
>   	 mask&= ~GL_DEPTH_BUFFER_BIT;
>         }
> -      else if (readRb->Format != drawRb->Format) {
> +      else if ((_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) !=
> +	        _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) ||
> +	       (_mesa_get_format_datatype(readRb->Format) !=
> +		_mesa_get_format_datatype(drawRb->Format))) {
>            _mesa_error(ctx, GL_INVALID_OPERATION,
>                        "glBlitFramebufferEXT(depth buffer format mismatch)");
>            return;