On Sun, Feb 19, 2017 at 6:11 AM, Max Qian <pub...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
