On Sat, 10 Dec 2011 12:00:52 -0700, Brian Paul <bri...@vmware.com> wrote: > Another step toward getting rid of the renderbuffer PutRow/etc functions.
> + if (buffers & BUFFER_DS) { > + struct gl_renderbuffer *rb = > + ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer; > + > + if ((buffers & BUFFER_DS) == BUFFER_DS && rb && > + _mesa_is_format_packed_depth_stencil(rb->Format)) { > + /* clear depth and stencil together */ > + _swrast_clear_depth_stencil_buffer(ctx); I think this would be wrong for the case of Attachment[BUFFER_DEPTH] = depthstencil texture Attachment[BUFFER_STENCIL = separate stencil RB Granted, that's a pretty crazy setup, but I think this just needs an rb == Attachment[BUFFER_STENCIL].Renderbuffer check. > + mapMode = GL_MAP_WRITE_BIT; > + if (_mesa_get_format_bits(rb->Format, GL_STENCIL_BITS) > 0) { > + mapMode |= GL_MAP_READ_BIT; > + } You only set the READ bit if there are stencil bits... > + case MESA_FORMAT_S8_Z24: > + case MESA_FORMAT_X8_Z24: > + case MESA_FORMAT_Z24_S8: > + case MESA_FORMAT_Z24_X8: > for (i = 0; i < height; i++) { > - rb->PutMonoRow(ctx, rb, width, x, y + i, &clearVal16, NULL); > + GLuint *row = (GLuint *) map; > + for (j = 0; j < width; j++) { > + row[j] = (row[j] & mask) | clearVal; > + } > + map += rowStride; > } > + but then the S8_Z24 and X8_Z24 paths both do the same set of reads. > + case MESA_FORMAT_Z32_FLOAT_X24S8: > + /* XXX untested */ > + { > + GLfloat clearVal = (GLfloat) ctx->Depth.Clear; > for (i = 0; i < height; i++) { > - rb->PutMonoRow(ctx, rb, width, x, y + i, &clearValue, NULL); > + GLfloat *row = (GLfloat *) map; > + for (j = 0; j < width; j++) { > + row[j * 2] = clearVal; > + } > + map += rowStride; > } It looks good, at least. Once I land my outstanding driver bits it should be easy to test. > +/** > + * Clear both depth and stencil values in a combined depth+stencil buffer. > + */ > +void > +_swrast_clear_depth_stencil_buffer(struct gl_context *ctx) > +{ > + case MESA_FORMAT_Z32_FLOAT_X24S8: > + /* XXX untested */ > + { > + GLfloat zClear = (GLfloat) ctx->Depth.Clear; > + GLuint sClear = ctx->Stencil.Clear; > + for (i = 0; i < height; i++) { > + GLfloat *zRow = (GLfloat *) map; > + GLuint *sRow = (GLuint *) map; > + for (j = 0; j < width; j++) { > + zRow[j * 2 + 0] = zClear; > + } > + for (j = 0; j < width; j++) { > + sRow[j * 2 + 1] = sClear; > + } Missing stencil mask treatment. > diff --git a/src/mesa/swrast/s_stencil.c b/src/mesa/swrast/s_stencil.c > index 101ee50..5c5ebd4 100644 > --- a/src/mesa/swrast/s_stencil.c > +++ b/src/mesa/swrast/s_stencil.c > @@ -1124,121 +1124,108 @@ _swrast_write_stencil_span(struct gl_context *ctx, > GLint n, GLint x, GLint y, > > > /** > - * Clear the stencil buffer. > + * Clear the stencil buffer. If the buffer is a combined > + * depth+stencil buffer, only the stencil bits will be touched. > */ > void > -_swrast_clear_stencil_buffer( struct gl_context *ctx, struct gl_renderbuffer > *rb ) > +_swrast_clear_stencil_buffer(struct gl_context *ctx) > { > + struct gl_renderbuffer *rb = > + ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer; > const GLubyte stencilBits = ctx->DrawBuffer->Visual.stencilBits; > - const GLuint mask = ctx->Stencil.WriteMask[0]; > - const GLuint invMask = ~mask; > - const GLuint clearVal = (ctx->Stencil.Clear & mask); > + const GLuint writeMask = ctx->Stencil.WriteMask[0]; > + const GLuint clearVal = (ctx->Stencil.Clear & writeMask); > const GLuint stencilMax = (1 << stencilBits) - 1; > GLint x, y, width, height; > + GLubyte *map; > + GLint rowStride, i, j; > + GLbitfield mapMode; > + mapMode = GL_MAP_WRITE_BIT; > + if ((writeMask & stencilMax) != stencilMax) { > + /* need to mask stencil values */ > + mapMode |= GL_MAP_READ_BIT; > + } > + else if (_mesa_get_format_bits(rb->Format, GL_DEPTH_BITS) > 0) { > + /* combined depth+stencil, need to mask Z values */ > + mapMode |= GL_MAP_READ_BIT; > + } > + switch (rb->Format) { > + case MESA_FORMAT_S8: > + { > + GLubyte clear = clearVal & 0xff; > + GLubyte mask = (~writeMask) & 0xff; > + if (mask != 0) { mask != 0xff, right? 0 was already tested at the top, and there's that else case below. > - _mesa_memset16((short unsigned int*) stencil, clearVal, > width); note, _mesa_memset16 is dead code now.
pgpW2Cgm29FPs.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev