[Mesa-dev] [PATCH] mesa: Clamp ValueMask to [0, 255].

[Chris Forbes]

I don't see any spec justification for masking this. dEQP is broken here.
Implementations have the flexibility to retain more bits in the mask (and
have more bits set in the initial state) than the depth of the deepest
stencil buffer supported. From the ES3 spec, 4.1.4, second to last para:

   "In the initial state, ... , and the front and back stencil mask are
both set to the value 2^s - 1, where s is greater than or equal to the
number of bits in the deepest stencil buffer supported by the GL
implementation"

/ref/ is specified as being clamped on use and on query, which ought to be
indistinguishable from clamping it upfront.

- Chris

On Sun, Dec 18, 2016 at 4:44 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> Should ref also get clamped?
>
> On Dec 17, 2016 1:03 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:
>
>> Commit b8b1d83c71fd148d2fd84afdc20c0aa367114f92 partially fixed
>> dEQP-GLES3.functional.state_query.integers.stencil*value*mask*getfloat
>> by changing the initial value masks from 32-bit ~0 (0xFFFFFFFF) to 0xFF.
>>
>> However, the application can call glStencilFunc and related functions
>> to set a new value mask, which is a 32-bit quantity.  The application
>> might specify 0xFFFFFFFF, bringing us back to the original problem.
>>
>> In particular, dEQP's state reset code seems to do this, so the tests
>> still fail when running the entire suite from one process, rather than
>> running the tests individually.
>>
>> This patch clamps the value masks to 0xFF when setting them.  Higher
>> bits have no effect on an 8-bit stencil buffer anyway.
>>
>> This might break apps that set a value mask then try to query it back
>> with glGet and expect to get the same value.  I'm unclear whether apps
>> can reasonably expect that anyway.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/mesa/main/stencil.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c
>> index b303bb7..608c564 100644
>> --- a/src/mesa/main/stencil.c
>> +++ b/src/mesa/main/stencil.c
>> @@ -161,7 +161,7 @@ _mesa_StencilFuncSeparateATI( GLenum frontfunc,
>> GLenum backfunc, GLint ref, GLui
>>     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;
>> +   ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask & 0xFF;
>>     if (ctx->Driver.StencilFuncSeparate) {
>>        ctx->Driver.StencilFuncSeparate(ctx, GL_FRONT,
>>                                        frontfunc, ref, mask);
>> @@ -206,7 +206,7 @@ _mesa_StencilFunc( GLenum func, GLint ref, GLuint
>> mask )
>>        FLUSH_VERTICES(ctx, _NEW_STENCIL);
>>        ctx->Stencil.Function[face] = func;
>>        ctx->Stencil.Ref[face] = ref;
>> -      ctx->Stencil.ValueMask[face] = mask;
>> +      ctx->Stencil.ValueMask[face] = mask & 0xFF;
>>
>>        /* Only propagate the change to the driver if EXT_stencil_two_side
>>         * is enabled.
>> @@ -227,7 +227,7 @@ _mesa_StencilFunc( GLenum func, GLint ref, GLuint
>> mask )
>>        FLUSH_VERTICES(ctx, _NEW_STENCIL);
>>        ctx->Stencil.Function[0]  = ctx->Stencil.Function[1]  = func;
>>        ctx->Stencil.Ref[0]       = ctx->Stencil.Ref[1]       = ref;
>> -      ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask;
>> +      ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask &
>> 0xFF;
>>        if (ctx->Driver.StencilFuncSeparate) {
>>           ctx->Driver.StencilFuncSeparate(ctx,
>>                                          ((ctx->Stencil.TestTwoSide)
>> @@ -472,13 +472,13 @@ _mesa_StencilFuncSeparate(GLenum face, GLenum
>> func, GLint ref, GLuint mask)
>>        /* set front */
>>        ctx->Stencil.Function[0] = func;
>>        ctx->Stencil.Ref[0] = ref;
>> -      ctx->Stencil.ValueMask[0] = mask;
>> +      ctx->Stencil.ValueMask[0] = mask & 0xFF;
>>     }
>>     if (face != GL_FRONT) {
>>        /* set back */
>>        ctx->Stencil.Function[1] = func;
>>        ctx->Stencil.Ref[1] = ref;
>> -      ctx->Stencil.ValueMask[1] = mask;
>> +      ctx->Stencil.ValueMask[1] = mask & 0xFF;
>>     }
>>     if (ctx->Driver.StencilFuncSeparate) {
>>        ctx->Driver.StencilFuncSeparate(ctx, face, func, ref, mask);
>> --
>> 2.10.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161218/ca3f7f9b/attachment.html>