[Mesa-dev] [PATCH] mesa: add NV_read_{depth, stencil, depth_stencil} extensions

[Rob Clark]

On Mon, Aug 10, 2015 at 3:28 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Mon, Aug 10, 2015 at 9:15 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Mon, Aug 10, 2015 at 12:54 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>> On Mon, Aug 10, 2015 at 1:02 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>
>>>> These extensions allow reading depth/stencil for GLES contexts, which is
>>>> useful for tools like apitrace.
>>>>
>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>> ---
>>>> I have a patch, which I will send out after some cleanup, that makes
>>>> apitrace able to dump depth/stencil buffers with GLES, thanks to this
>>>> extension.
>>>>
>>>>  src/mesa/main/extensions.c |  3 +++
>>>>  src/mesa/main/readpix.c    | 25 +++++++++++++++++++------
>>>>  2 files changed, 22 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
>>>> index 2dbfabd..d934d19 100644
>>>> --- a/src/mesa/main/extensions.c
>>>> +++ b/src/mesa/main/extensions.c
>>>> @@ -385,6 +385,9 @@ static const struct extension extension_table[] = {
>>>>     { "GL_NV_point_sprite",                         o(NV_point_sprite),                         GL,             2001 },
>>>>     { "GL_NV_primitive_restart",                    o(NV_primitive_restart),                    GLL,            2002 },
>>>>     { "GL_NV_read_buffer",                          o(dummy_true),                              ES2,            2011 },
>>>> +   { "GL_NV_read_depth",                           o(dummy_true),                              ES2,            2011 },
>>>> +   { "GL_NV_read_depth_stencil",                   o(dummy_true),                              ES2,            2011 },
>>>> +   { "GL_NV_read_stencil",                         o(dummy_true),                              ES2,            2011 },
>>>>     { "GL_NV_texgen_reflection",                    o(dummy_true),                              GLL,            1999 },
>>>>     { "GL_NV_texture_barrier",                      o(NV_texture_barrier),                      GL,             2009 },
>>>>     { "GL_NV_texture_env_combine4",                 o(NV_texture_env_combine4),                 GLL,            1999 },
>>>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>>>> index 2744232..65751aa 100644
>>>> --- a/src/mesa/main/readpix.c
>>>> +++ b/src/mesa/main/readpix.c
>>>> @@ -917,7 +917,9 @@ read_pixels_es3_error_check(GLenum format, GLenum type,
>>>>     GLboolean is_unsigned_int = GL_FALSE;
>>>>     GLboolean is_signed_int = GL_FALSE;
>>>>
>>>> -   if (!_mesa_is_color_format(internalFormat)) {
>>>> +   /* TODO just drop the check?  Are there any formats to filter out? */
>>>
>>> Yes, please drop it.
>>>
>>>> +   if (!(_mesa_is_color_format(internalFormat) ||
>>>> +         _mesa_is_depth_or_stencil_format(internalFormat))) {
>>>>        return GL_INVALID_OPERATION;
>>>>     }
>>>>
>>>> @@ -950,6 +952,22 @@ read_pixels_es3_error_check(GLenum format, GLenum type,
>>>>            (is_unsigned_int && type == GL_UNSIGNED_INT))
>>>>           return GL_NO_ERROR;
>>>>        break;
>>>> +   case GL_DEPTH_STENCIL:
>>>> +      if ((internalFormat == GL_DEPTH24_STENCIL8) &&
>>>> +          (type == GL_UNSIGNED_INT_24_8))
>>>> +         return GL_NO_ERROR;
>>>> +      if ((internalFormat == GL_DEPTH32F_STENCIL8) &&
>>>> +          (type == GL_FLOAT_32_UNSIGNED_INT_24_8_REV))
>>>> +         return GL_NO_ERROR;
>>>> +      break;
>>>> +   case GL_DEPTH_COMPONENT:
>>>> +      if ((internalFormat == GL_DEPTH_COMPONENT32F) &&
>>>> +          (type == GL_FLOAT))
>>>> +         return GL_NO_ERROR;
>>>> +      if ((internalFormat == GL_DEPTH_COMPONENT24) &&
>>>> +          (type == GL_UNSIGNED_INT))
>>>> +         return GL_NO_ERROR;
>>>> +      break;
>>>
>>> What about GL_STENCIL_INDEX? Mesa does support GL_OES_stencil8.
>>
>> hmm.. the gles3 glTexImage2D page (where I got the mapping of
>> internalFormat and type) didn't include GL_STENCIL_INDEX.  So I'm a
>> bit unclear what (if any) the check for internalFormat should be?  Or
>> should we just check that type is GL_STENCIL_INDEX (which is all the
>> NV_read_* spec says)?
>
> This is ReadPixels, not TexImage.

Correct, but it gives an association between allowed combinations of
internalFormat, format, and type.

I'm still unclear (since there does not seem to be sw upload to a
stencil8 buffer), what if any restrictions there should be on the
internalFormat..

BR,
-R

> GL_OES_stencil8 only allows GL_STENCIL_INDEX for renderbuffers.
>
> Marek