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

[Ilia Mirkin]

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.

  -ilia