On Mon, Dec 12, 2011 at 10:28 AM, Eric Anholt <e...@anholt.net> wrote:
> 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.
Will do.
>> + 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.
Will fix.
>> 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.
I think I have that right. mask is the complement of the stencil
WriteMask value here. And if mask==0 it means that we're replacing
the dest values completely. That's what the else clauses do.
>> - _mesa_memset16((short unsigned int*) stencil, clearVal,
>> width);
>
> note, _mesa_memset16 is dead code now.
OK, I'll rm that later.
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
