[Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Ian Romanick]

On 03/13/2014 06:33 AM, Ilia Mirkin wrote:
> On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Ilia Mirkin <imirkin at alum.mit.edu> writes:
>>
>>> EXT_packed_depth_stencil is supported by all drivers, but
>>> ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
>>> passing unexpected values down to ChooseTextureFormat.
>>>
>>> The EXT_packed_depth_stencil spec does not make any explicit references
>>> to requiring ARB_depth_texture in order to allow textures with that
>>> format, however if there is no dependency, ARB_depth_texture would be
>>> practically implied by the extension.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> It should also be noted that ARB_depth_texture became required in OpenGL 1.4,
>>> so this shouldn't affect too many drivers. Looks like i915 only has it
>>> disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
>>> the values are supported in ChooseTextureFormat, but not actually handled in
>>> radeon_texstate or r200_texstate.
>>>
>>>  src/mesa/main/teximage.c | 11 +++--------
>>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>> index a6c3581..a57a535 100644
>>> --- a/src/mesa/main/teximage.c
>>> +++ b/src/mesa/main/teximage.c
>>> @@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint internalFormat )
>>>           case GL_DEPTH_COMPONENT24:
>>>           case GL_DEPTH_COMPONENT32:
>>>              return GL_DEPTH_COMPONENT;
>>> +         case GL_DEPTH_STENCIL:
>>> +         case GL_DEPTH24_STENCIL8:
>>> +            return GL_DEPTH_STENCIL;
>>>           default:
>>>              ; /* fallthrough */
>>>        }
>>> @@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint internalFormat )
>>>        }
>>>     }
>>>
>>> -   switch (internalFormat) {
>>> -   case GL_DEPTH_STENCIL:
>>> -   case GL_DEPTH24_STENCIL8:
>>> -      return GL_DEPTH_STENCIL;
>>> -   default:
>>> -      ; /* fallthrough */
>>> -   }
>>> -
>>>     if (ctx->Extensions.EXT_texture_sRGB) {
>>>        switch (internalFormat) {
>>>        case GL_SRGB_EXT:
>>> --
>>> 1.8.3.2
>>
>>
>> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> 
> Thanks. I'm going to wait a while before pushing this to see if others
> have differing opinions on the spec interpretation. Ian, Eric, Brian?
> You guys seem to be fairly in-tune with this stuff.

The body of the spec may not be clear, but the overview at least is:

    This extension provides a new data format, GL_DEPTH_STENCIL_EXT,
    that can be used with the glDrawPixels, glReadPixels, and
    glCopyPixels commands, as well as a packed data type,
    GL_UNSIGNED_INT_24_8_EXT, that is meant to be used with
    GL_DEPTH_STENCIL_EXT.  No other data types are supported with
    GL_DEPTH_STENCIL_EXT.  If ARB_depth_texture or SGIX_depth_texture is
    supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
    also be used for textures; this provides a more efficient way to
    supply data for a 24-bit depth texture.

I thought I cleaned all this up when I made EXT_packed_depth_stencil
non-optional, but it looks like I missed it.  Thanks for taking care of
this.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>   -ilia
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev