On Fri, May 18, 2012 at 11:27:24AM -0600, Brian Paul wrote: > On 05/18/2012 05:24 AM, Oliver McFadden wrote: > > The device driver function table only implements StencilOpSeparate(), > > let's remove the obsolete 'todo' comment and code duplication. > > > > Signed-off-by: Oliver McFadden<[email protected]> > > --- > > src/mesa/drivers/common/meta.c | 4 +- > > src/mesa/main/stencil.c | 75 > > ++++++--------------------------------- > > 2 files changed, 14 insertions(+), 65 deletions(-) > > > > diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c > > index 95336fc..42bbf9d 100644 > > --- a/src/mesa/drivers/common/meta.c > > +++ b/src/mesa/drivers/common/meta.c > > @@ -2360,12 +2360,12 @@ _mesa_meta_DrawPixels(struct gl_context *ctx, > > _mesa_set_enable(ctx, GL_STENCIL_TEST, GL_TRUE); > > > > /* set all stencil bits to 0 */ > > - _mesa_StencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE); > > + _mesa_StencilOpSeparate(ctx->Stencil.ActiveFace, GL_REPLACE, > > GL_REPLACE, GL_REPLACE); > > _mesa_StencilFunc(GL_ALWAYS, 0, 255); > > _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4); > > > > /* set stencil bits to 1 where needed */ > > - _mesa_StencilOp(GL_KEEP, GL_KEEP, GL_REPLACE); > > + _mesa_StencilOpSeparate(ctx->Stencil.ActiveFace, GL_KEEP, GL_KEEP, > > GL_REPLACE); > > > > _mesa_BindProgram(GL_FRAGMENT_PROGRAM_ARB, drawpix->StencilFP); > > _mesa_set_enable(ctx, GL_FRAGMENT_PROGRAM_ARB, GL_TRUE); > > diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c > > index f47b57b..f051479 100644 > > --- a/src/mesa/main/stencil.c > > +++ b/src/mesa/main/stencil.c > > @@ -314,70 +314,24 @@ _mesa_StencilMask( GLuint mask ) > > * > > * Verifies the parameters and updates the respective fields in > > * __struct gl_contextRec::Stencil. On change flushes the vertices and > > notifies the > > - * driver via the dd_function_table::StencilOp callback. > > + * driver via the dd_function_table::StencilOpSeparate callback. > > */ > > void GLAPIENTRY > > _mesa_StencilOp(GLenum fail, GLenum zfail, GLenum zpass) > > { > > GET_CURRENT_CONTEXT(ctx); > > - const GLint face = ctx->Stencil.ActiveFace; > > - > > - if (MESA_VERBOSE& VERBOSE_API) > > - _mesa_debug(ctx, "glStencilOp()\n"); > > - > > - ASSERT_OUTSIDE_BEGIN_END(ctx); > > - > > - if (!validate_stencil_op(ctx, fail)) { > > - _mesa_error(ctx, GL_INVALID_ENUM, "glStencilOp(sfail)"); > > - return; > > - } > > - if (!validate_stencil_op(ctx, zfail)) { > > - _mesa_error(ctx, GL_INVALID_ENUM, "glStencilOp(zfail)"); > > - return; > > - } > > - if (!validate_stencil_op(ctx, zpass)) { > > - _mesa_error(ctx, GL_INVALID_ENUM, "glStencilOp(zpass)"); > > - return; > > - } > > - > > - if (face != 0) { > > Note: if face != 0, it must be 2 (there should be an assertion). More > below... > > > > - /* only set active face state */ > > - if (ctx->Stencil.ZFailFunc[face] == zfail&& > > - ctx->Stencil.ZPassFunc[face] == zpass&& > > - ctx->Stencil.FailFunc[face] == fail) > > - return; > > - FLUSH_VERTICES(ctx, _NEW_STENCIL); > > - ctx->Stencil.ZFailFunc[face] = zfail; > > - ctx->Stencil.ZPassFunc[face] = zpass; > > - ctx->Stencil.FailFunc[face] = fail; > > - > > - /* Only propagate the change to the driver if EXT_stencil_two_side > > - * is enabled. > > - */ > > - if (ctx->Driver.StencilOpSeparate&& ctx->Stencil.TestTwoSide) { > > - ctx->Driver.StencilOpSeparate(ctx, GL_BACK, fail, zfail, zpass); > > - } > > - } > > - else { > > - /* set both front and back state */ > > - if (ctx->Stencil.ZFailFunc[0] == zfail&& > > - ctx->Stencil.ZFailFunc[1] == zfail&& > > - ctx->Stencil.ZPassFunc[0] == zpass&& > > - ctx->Stencil.ZPassFunc[1] == zpass&& > > - ctx->Stencil.FailFunc[0] == fail&& > > - ctx->Stencil.FailFunc[1] == fail) > > - return; > > - FLUSH_VERTICES(ctx, _NEW_STENCIL); > > - ctx->Stencil.ZFailFunc[0] = ctx->Stencil.ZFailFunc[1] = zfail; > > - ctx->Stencil.ZPassFunc[0] = ctx->Stencil.ZPassFunc[1] = zpass; > > - ctx->Stencil.FailFunc[0] = ctx->Stencil.FailFunc[1] = fail; > > - if (ctx->Driver.StencilOpSeparate) { > > - ctx->Driver.StencilOpSeparate(ctx, > > - ((ctx->Stencil.TestTwoSide) > > - ? GL_FRONT : GL_FRONT_AND_BACK), > > - fail, zfail, zpass); > > + GLenum face; > > + > > + if (ctx->Stencil.TestTwoSide) { > > + face = GL_FRONT_AND_BACK; > > + } else { > > + if (ctx->Stencil.ActiveFace == 0) { > > + face = GL_FRONT; > > + } else { > > + face = GL_BACK; > > } > > } > > + _mesa_StencilOpSeparate(face, fail, zfail, zpass); > > } > > I think there's something subtly different here. As noted above, if > face != 0 it must be 2 so we're setting the stencil[2] state. But > that never happens in _mesa_StencilOpSeparate(). > > It's all a bit confusing because two-sided stencil is treated > differently between GL_EXT_stencil_two_side, GL_ATI_separate_stencil > and OpenGL 2.0. > > Maybe you can double-check this stuff.
OK. I'll check it on Monday and I agree it's a bit confusing. At least the few Piglit tests I manually executed passed, but that's by no means a comprehensive test suite. -- Oliver McFadden. _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
