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 <chr...@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);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
