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

[Brian Paul]

On 03/13/2014 07: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.

Looks OK to me.  Do you want to tag this for the stable branches?

-Brian