[Mesa-dev] [PATCH] mesa: Fix meta smashing stencil reference value on restore
Kenneth Graunke
kenneth at whitecape.org
Mon May 13 01:17:56 PDT 2013
On 05/13/2013 12:15 AM, Chris Forbes wrote:
> After a meta op which touches the stencil state, the stencil reference
> value would be revalidated against the current stencil depth, rather
> than simply being restored.
>
> In particular, the following call sequence would cause the stencil
> reference value to be smashed to zero:
>
> - BindFramebuffer( /* fbo with a stencil attachment */ )
> - StencilFuncSeparate(...)
> - BindFramebuffer( /* some fbo without a stencil attachment */ )
> - Meta()
Yep, that makes sense...excellent work tracking this down, Chris!
> The GL spec says the reference value is clamped at specification time
> only.
I read the text a bit differently. From the GL 3.2 Core spec, page 190/204:
"Stencil comparison operations and queries of ref clamp its value to the
range [0, 2^s - 1], where s is the number of bits in the stencil buffer
attached to the draw framebuffer."
That sounds to me like it should not be clamped on specification, but
rather should be clamped when used or queried. In particular, what
about this situation:
1. BindFramebuffer(...fbo with NO stencil...)
2. StencilFuncSeparate(...0xff...)
3. BindFramebuffer(...fbo with stencil...)
It seems like the current code would clamp Ref to 0x0 at step #2, which
could result in bad rendering after step #3.
Instead, I think perhaps we should add an accessor function:
static GLint
_mesa_get_stencil_ref(struct gl_context *ctx, int face)
{
GLint stencilMax = (1 << ctx->DrawBuffer->Visual.stencilBits) - 1;
GLint ref = ctx->Stencil.Ref[face];
return CLAMP(ref, 0, stencilMax);
}
and then convert all users of Stencil.Ref[] to use this, so they get the
properly clamped value. Then, finally, remove the clamping in
StencilFuncSeparate and StencilFuncSeparateATI.
It's a bit more invasive but I think fixes more cases.
> Fixes broken rendering in Portal, when a portal is visible onscreen.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
> src/mesa/drivers/common/meta.c | 16 +++++-----
> src/mesa/main/stencil.c | 69 ++++++++++++++++++++++++------------------
> src/mesa/main/stencil.h | 8 +++++
> 3 files changed, 56 insertions(+), 37 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index ca5f5a1..ec0b4d6 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -987,19 +987,19 @@ _mesa_meta_end(struct gl_context *ctx)
> ? GL_BACK : GL_FRONT);
> }
> /* front state */
> - _mesa_StencilFuncSeparate(GL_FRONT,
> - stencil->Function[0],
> - stencil->Ref[0],
> - stencil->ValueMask[0]);
> + _mesa_stencil_func(ctx, GL_FRONT,
> + stencil->Function[0],
> + stencil->Ref[0],
> + stencil->ValueMask[0]);
> _mesa_StencilMaskSeparate(GL_FRONT, stencil->WriteMask[0]);
> _mesa_StencilOpSeparate(GL_FRONT, stencil->FailFunc[0],
> stencil->ZFailFunc[0],
> stencil->ZPassFunc[0]);
> /* back state */
> - _mesa_StencilFuncSeparate(GL_BACK,
> - stencil->Function[1],
> - stencil->Ref[1],
> - stencil->ValueMask[1]);
> + _mesa_stencil_func(ctx, GL_BACK,
> + stencil->Function[1],
> + stencil->Ref[1],
> + stencil->ValueMask[1]);
> _mesa_StencilMaskSeparate(GL_BACK, stencil->WriteMask[1]);
> _mesa_StencilOpSeparate(GL_BACK, stencil->FailFunc[1],
> stencil->ZFailFunc[1],
> diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c
> index cbdee6b..564bf68 100644
> --- a/src/mesa/main/stencil.c
> +++ b/src/mesa/main/stencil.c
> @@ -113,6 +113,42 @@ _mesa_ClearStencil( GLint s )
> ctx->Stencil.Clear = (GLuint) s;
> }
>
> +/**
> + * Set the function and reference value for stencil testing.
> + * Assumes all validation and clamping has already been done.
> + *
> + * This is used as a helper by the StencilFunc* API, and also directly
> + * called by meta to restore the state without smashing the reference
> + * value if the stencil depth is different from the depth it was
> + * validated against.
> + */
> +void
> +_mesa_stencil_func(struct gl_context *ctx,
> + GLenum face,
> + GLenum func,
> + GLint ref,
> + GLuint mask)
> +{
> + FLUSH_VERTICES(ctx, _NEW_STENCIL);
> +
> + if (face != GL_BACK) {
> + /* set front */
> + ctx->Stencil.Function[0] = func;
> + ctx->Stencil.Ref[0] = ref;
> + ctx->Stencil.ValueMask[0] = mask;
> + }
> + if (face != GL_FRONT) {
> + /* set back */
> + ctx->Stencil.Function[1] = func;
> + ctx->Stencil.Ref[1] = ref;
> + ctx->Stencil.ValueMask[1] = mask;
> + }
> + if (ctx->Driver.StencilFuncSeparate) {
> + ctx->Driver.StencilFuncSeparate(ctx, face, func, ref, mask);
> + }
> +}
> +
> +
>
> /**
> * Set the function and reference value for stencil testing.
> @@ -158,17 +194,9 @@ _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum backfunc, GLint ref, GLui
> ctx->Stencil.Ref[0] == ref &&
> ctx->Stencil.Ref[1] == ref)
> return;
> - FLUSH_VERTICES(ctx, _NEW_STENCIL);
> - ctx->Stencil.Function[0] = frontfunc;
> - ctx->Stencil.Function[1] = backfunc;
> - ctx->Stencil.Ref[0] = ctx->Stencil.Ref[1] = ref;
> - ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask;
> - if (ctx->Driver.StencilFuncSeparate) {
> - ctx->Driver.StencilFuncSeparate(ctx, GL_FRONT,
> - frontfunc, ref, mask);
> - ctx->Driver.StencilFuncSeparate(ctx, GL_BACK,
> - backfunc, ref, mask);
> - }
> +
> + _mesa_stencil_func(ctx, GL_FRONT, frontfunc, ref, mask);
> + _mesa_stencil_func(ctx, GL_BACK, backfunc, ref, mask);
> }
>
>
> @@ -478,24 +506,7 @@ _mesa_StencilFuncSeparate(GLenum face, GLenum func, GLint ref, GLuint mask)
> }
>
> ref = CLAMP(ref, 0, stencilMax);
> -
> - FLUSH_VERTICES(ctx, _NEW_STENCIL);
> -
> - if (face != GL_BACK) {
> - /* set front */
> - ctx->Stencil.Function[0] = func;
> - ctx->Stencil.Ref[0] = ref;
> - ctx->Stencil.ValueMask[0] = mask;
> - }
> - if (face != GL_FRONT) {
> - /* set back */
> - ctx->Stencil.Function[1] = func;
> - ctx->Stencil.Ref[1] = ref;
> - ctx->Stencil.ValueMask[1] = mask;
> - }
> - if (ctx->Driver.StencilFuncSeparate) {
> - ctx->Driver.StencilFuncSeparate(ctx, face, func, ref, mask);
> - }
> + _mesa_stencil_func(ctx, face, func, ref, mask);
> }
>
>
> diff --git a/src/mesa/main/stencil.h b/src/mesa/main/stencil.h
> index 1d5e01c..1aeadfe 100644
> --- a/src/mesa/main/stencil.h
> +++ b/src/mesa/main/stencil.h
> @@ -61,6 +61,14 @@ extern void GLAPIENTRY
> _mesa_StencilOpSeparate(GLenum face, GLenum fail, GLenum zfail, GLenum zpass);
>
>
> +void
> +_mesa_stencil_func(struct gl_context *ctx,
> + GLenum face,
> + GLenum func,
> + GLint ref,
> + GLuint mask);
> +
> +
> extern void GLAPIENTRY
> _mesa_StencilFuncSeparate(GLenum face, GLenum func, GLint ref, GLuint mask);
>
>
More information about the mesa-dev
mailing list