[Mesa-dev] [PATCH] mesa/blit: only error when the stencil and depth bits do not match (v3)
Ilia Mirkin
imirkin at alum.mit.edu
Mon Feb 20 00:26:11 UTC 2017
On Sun, Feb 19, 2017 at 6:11 AM, Max Qian <public at maxqia.com> wrote:
> This fixes some situations where glBlitFrameBuffer would error in Mesa,
glBlitFramebuffer
> but wouldn't error in other drivers (e.g Nvidia)
Please be descriptive here. What situations does this fix?
Also, there's not a whole lot of caring in the mesa community as to
what other drivers do, but rather what the spec says. This commit
description should be worded along the lines of "spec says this, but
mesa messed it up in this way".
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97921
> v2 : fix unintentional line removal
> v3 : author correction, add commit message, and change the error message
> ---
> src/mesa/main/blit.c | 84 +++++++++++++++++++++-------------------------------
> 1 file changed, 34 insertions(+), 50 deletions(-)
>
> diff --git a/src/mesa/main/blit.c b/src/mesa/main/blit.c
> index e739130f98..62cc8c2360 100644
> --- a/src/mesa/main/blit.c
> +++ b/src/mesa/main/blit.c
> @@ -334,41 +334,13 @@ _mesa_blit_framebuffer(struct gl_context *ctx,
> mask &= ~GL_STENCIL_BUFFER_BIT;
> }
> else {
> - int read_z_bits, draw_z_bits;
> -
> if (_mesa_is_gles3(ctx) && (drawRb == readRb)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "%s(source and destination stencil "
> "buffer cannot be the same)", func);
> return;
> }
> -
> - 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,
> - "%s(stencil attachment format mismatch)", func);
> - return;
> - }
> -
> - read_z_bits = _mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS);
> - draw_z_bits = _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS);
> -
> - /* If both buffers also have depth data, the depth formats must match
> - * as well. If one doesn't have depth, it's not blitted, so we should
> - * ignore the depth format check.
> - */
> - if (read_z_bits > 0 && draw_z_bits > 0 &&
> - (read_z_bits != draw_z_bits ||
> - _mesa_get_format_datatype(readRb->Format) !=
> - _mesa_get_format_datatype(drawRb->Format))) {
> -
> - _mesa_error(ctx, GL_INVALID_OPERATION,
> - "%s(stencil attachment depth format mismatch)", func);
> - return;
> - }
> + // check formats later
This would be the second comment with // in src/mesa/main. Please use
/* */ like everywhere else. [Not 100% sure what this comment is
adding...]
> }
> }
>
> @@ -388,36 +360,48 @@ _mesa_blit_framebuffer(struct gl_context *ctx,
> mask &= ~GL_DEPTH_BUFFER_BIT;
> }
> else {
> - int read_s_bit, draw_s_bit;
> -
> if (_mesa_is_gles3(ctx) && (drawRb == readRb)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "%s(source and destination depth "
> "buffer cannot be the same)", func);
> return;
> }
> + // check formats later
> + }
> + }
>
> - 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,
> - "%s(depth attachment format mismatch)", func);
> - return;
> - }
> + /* From the glBlitFramebuffer Spec :
> + *
> + * "GL_INVALID_OPERATION is generated if mask contains
> + * GL_DEPTH_BUFFER_BIT or GL_STENCIL_BUFFER_BIT and the source and
> + * destination depth and stencil formats do not match."
> + */
> + if (mask & GL_STENCIL_BUFFER_BIT || mask & GL_DEPTH_BUFFER_BIT) {
> + struct gl_renderbuffer *readStencilRb =
> + readFb->Attachment[BUFFER_STENCIL].Renderbuffer;
> + struct gl_renderbuffer *drawStencilRb =
> + drawFb->Attachment[BUFFER_STENCIL].Renderbuffer;
> + struct gl_renderbuffer *readDepthRb =
> + readFb->Attachment[BUFFER_DEPTH].Renderbuffer;
> + struct gl_renderbuffer *drawDepthRb =
> + drawFb->Attachment[BUFFER_DEPTH].Renderbuffer;
>
> - read_s_bit = _mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS);
> - draw_s_bit = _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS);
> + int read_z_bits, draw_z_bits, read_s_bit, draw_s_bit;
> + read_z_bits = _mesa_get_format_bits(readDepthRb->Format, GL_DEPTH_BITS);
I haven't read the surrounding code, but does this work you have bound
e.g. a GL_STENCIL_INDEX8 texture to stencil and nothing to depth?
(Won't you get a null deref?)
> + draw_z_bits = _mesa_get_format_bits(drawDepthRb->Format, GL_DEPTH_BITS);
> + boolean depth = read_z_bits > 0 && draw_z_bits > 0 &&
bool
> + (read_z_bits != draw_z_bits ||
> + _mesa_get_format_datatype(readDepthRb->Format) !=
> + _mesa_get_format_datatype(drawDepthRb->Format));
>
> - /* If both buffers also have stencil data, the stencil formats must
> - * match as well. If one doesn't have stencil, it's not blitted, so
> - * we should ignore the stencil format check.
> - */
> - if (read_s_bit > 0 && draw_s_bit > 0 && read_s_bit != draw_s_bit) {
> - _mesa_error(ctx, GL_INVALID_OPERATION,
> - "%s(depth attachment stencil bits mismatch)", func);
> - return;
> - }
> + read_s_bit = _mesa_get_format_bits(readStencilRb->Format, GL_STENCIL_BITS);
> + draw_s_bit = _mesa_get_format_bits(drawStencilRb->Format, GL_STENCIL_BITS);
> + boolean stencil = read_s_bit > 0 && draw_s_bit > 0 && read_s_bit != draw_s_bit;
bool
> +
> + if (stencil && depth) {
So let's say you have a GL_DEPTH_COMPONENT24 src and
GL_DEPTH_COMPONENT32F dst, you shouldn't get an error because they
don't have stencil? Or GL_DEPTH24_STENCIL8 and GL_DEPTH32F_STENCIL8
because they have the same stencil bitness (and thus stencil ==
false)?
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(depth and stencil attachment bits and format mismatch)", func);
> + return;
> }
> }
>
> --
> 2.11.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list