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.
I'll send a patch.
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
