On Thu, Mar 13, 2014 at 11:40 AM, Ian Romanick <i...@freedesktop.org> wrote: > On 03/13/2014 06:33 AM, Ilia Mirkin wrote: >> On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez <curroje...@riseup.net> >> wrote: >>> Ilia Mirkin <imir...@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 <imir...@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 <curroje...@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.roman...@intel.com>
Great. Another possible interpretation would be that if ARB_depth_texture is supported, then one is allowed to pass a (GL_DEPTH_STENCIL_EXT, GL_UNSIGNED_INT_24_8_EXT) format/type data to a GL_DEPTH_COMPONENT texture. But I much prefer to interpret it as "if depth textures are supported, then they can also have a DEPTH_STENCIL internal format", since that makes it workable on nouveau_vieux with this simple change :) -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev