On Thu, Mar 13, 2014 at 6:55 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote: >> On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez <curroje...@riseup.net> >> wrote: >>> Ilia Mirkin <imir...@alum.mit.edu> writes: >>> >>>> Hi Francisco, >>>> >>>> I'm looking at some piglit failures on nv10... full run at >>>> >>>> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html >>>> >>>> The specific one is: >>>> >>>> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html >>>> >>>> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And >>>> indeed, that case isn't handled. Looking through the #defines, I see a >>>> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And >>>> even in the nv20 case, it's not actually used in the driver. >>>> >>>> Does that mean that EXT_packed_depth_stencil should never have been >>>> enabled on nouveau? I do see that it's a valid renderbuffer format >>>> though. [It is now always enabled, since commit a92b9e60, but at the >>>> time of the commit, it was also enabled for everything. >>>> >>>> Any advice? >>>> >>> >>> AFAICT EXT_packed_depth_stencil only requires the implementation to >>> support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported >>> in addition -- which, as you guessed correctly, could be supported on >>> nv20 or higher. >>> >>> Shouldn't the piglit test be checking if the implementation supports >>> depth texturing before doing that? >> >> Mmmaybe. But that's not such a big deal -- worst case, the piglit >> should get a gl error of some sort. Sounds like the real issue is that >> mesa core is letting the values through to the driver. I'll re-read >> EXT_packed_depth_stencil more carefully, I didn't see the >> ARB_depth_texture dependency, but I only scanned it very quickly. > > Aha, and in _mesa_base_tex_format, we have > > if (ctx->Extensions.ARB_depth_texture) { > switch (internalFormat) { > case GL_DEPTH_COMPONENT: > case GL_DEPTH_COMPONENT16: > case GL_DEPTH_COMPONENT24: > case GL_DEPTH_COMPONENT32: > return GL_DEPTH_COMPONENT; > default: > ; /* fallthrough */ > } > } > > followed by > > switch (internalFormat) { > case GL_DEPTH_STENCIL: > case GL_DEPTH24_STENCIL8: > return GL_DEPTH_STENCIL; > default: > ; /* fallthrough */ > } > > And I think you're right about these only being available for > ARB_depth_texture: > > 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. > > This doesn't reference GL_DEPTH24_STENCIL8 explicitly, but I can only > assume that's a spec bug.
Hmmmm... OTOH, that's perhaps more correctly interpreted as "You can pass GL_DEPTH_STENCIL data to a GL_DEPTH_COMPONENT texture". Which would in turn mean that EXT_packed_depth_stencil does require GL_DEPTH_STENCIL texture support... Anyone else have opinions on this? -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
