[Mesa-dev] [PATCH] mesa: add GL_OES_texture_stencil8 support

[Ilia Mirkin]

On Tue, Feb 16, 2016 at 3:21 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> On 02/16/2016 06:05 AM, Ilia Mirkin wrote:
>> It's basically the same thing as GL_ARB_texture_stencil8 except that
>> glCopyTexImage isn't supported, so add STENCIL_INDEX to the list of
>> invalid GLES formats for glCopyTexImage.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Seems to pass the few dEQP tests that are there. The ext is nearly identical to the desktop version.
>>
>>  docs/GL3.txt                     | 2 +-
>>  src/mesa/main/extensions_table.h | 1 +
>>  src/mesa/main/teximage.c         | 2 ++
>>  3 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/GL3.txt b/docs/GL3.txt
>> index 3c4db06..26847b9 100644
>> --- a/docs/GL3.txt
>> +++ b/docs/GL3.txt
>> @@ -256,7 +256,7 @@ GLES3.2, GLSL ES 3.2
>>    GL_OES_texture_border_clamp                          DONE (all drivers)
>>    GL_OES_texture_buffer                                not started (based on GL_ARB_texture_buffer_object, GL_ARB_texture_buffer_range, and GL_ARB_texture_buffer_object_rgb32 that are all done)
>>    GL_OES_texture_cube_map_array                        not started (based on GL_ARB_texture_cube_map_array, which is done for all drivers)
>> -  GL_OES_texture_stencil8                              not started (based on GL_ARB_texture_stencil8, which is done for some drivers)
>> +  GL_OES_texture_stencil8                              DONE (all drivers that support GL_ARB_texture_stencil8)
>>    GL_OES_texture_storage_multisample_2d_array          DONE (all drivers that support GL_ARB_texture_multisample)
>>
>>  More info about these features and the work involved can be found at
>> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
>> index 43dc358..bcd12a2 100644
>> --- a/src/mesa/main/extensions_table.h
>> +++ b/src/mesa/main/extensions_table.h
>> @@ -343,6 +343,7 @@ EXT(OES_texture_half_float                  , OES_texture_half_float
>>  EXT(OES_texture_half_float_linear           , OES_texture_half_float_linear          ,  x ,  x ,  x , ES2, 2005)
>>  EXT(OES_texture_mirrored_repeat             , dummy_true                             ,  x ,  x , ES1,  x , 2005)
>>  EXT(OES_texture_npot                        , ARB_texture_non_power_of_two           ,  x ,  x , ES1, ES2, 2005)
>> +EXT(OES_texture_stencil8                    , ARB_texture_stencil8                   ,  x ,  x ,  x ,  30, 2014)
>>  EXT(OES_texture_storage_multisample_2d_array, ARB_texture_multisample                ,  x ,  x , ES1,  31, 2014)
>>  EXT(OES_texture_view                        , ARB_texture_view                       ,  x ,  x ,  x ,  31, 2014)
>>  EXT(OES_vertex_array_object                 , dummy_true                             ,  x ,  x , ES1, ES2, 2010)
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 57765d7..8a4c628 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -2285,8 +2285,10 @@ copytexture_error_check( struct gl_context *ctx, GLuint dimensions,
>>        }
>>        if (baseFormat == GL_DEPTH_COMPONENT ||
>>            baseFormat == GL_DEPTH_STENCIL ||
>> +          baseFormat == GL_STENCIL_INDEX ||
>>            rb_base_format == GL_DEPTH_COMPONENT ||
>>            rb_base_format == GL_DEPTH_STENCIL ||
>> +          rb_base_format == GL_STENCIL_INDEX ||
>>            ((baseFormat == GL_LUMINANCE_ALPHA ||
>>              baseFormat == GL_ALPHA) &&
>>             rb_base_format != GL_RGBA) ||
>>
>
> Handling GL_STENCIL_INDEX8 should also be added to at least two
> functions in glformats.c:
>
> Based on these two constrains from the extension spec:
>
>     "An INVALID_OPERATION error is generated by TexImage3D, TexImage2D,
>     TexSubImage3D or TexSubImage2D, if <format> is STENCIL_INDEX and
>     <type> is not <UNSIGNED_BYTE>"
>
> and
>
>     "An INVALID_OPERATION error is generated by TexImage3D and
>      TexImage2D, if <format> is <STENCIL_INDEX> and internal format is
>      not <STENCIL_INDEX8>"
>
> The corresponding case for STENCIL_INDEX should be added to:
>
> * _mesa_es3_effective_internal_format_for_format_and_type(), which
> should return STENCIL_INDEX8 out of STENCIL_INDEX + UNSIGNED_BYTE.

That only gets called when the internalFormat is unsized, which should
never happen. (And if it does, it's an error.)

>
> * _mesa_es3_error_check_format_and_type(), which should fail any
> combination other than STENCIL_INDEX8 + UNSIGNED_BYTE + STENCIL_INDEX.

Ah indeed. Missed that one. Will resend with this small bit included
as well. (As it turns out, that function will return GL_NO_ERROR when
it totally doesn't know about the internal format...)

>
>
> These are the two cases off the top of my head, but there can be other
> function that miss handling of STENCIL_INDEX8. Ideally, tests should
> catch those though.

Yeah... I was hoping that dEQP would have more coverage, but it
doesn't -- just a handful of tests, none of which tried feeding it a
bad format/type.