[Mesa-dev] Z24S8 textures on nv04/nv10

[Francisco Jerez]

Ilia Mirkin <imirkin at alum.mit.edu> writes:

> On Thu, Mar 13, 2014 at 6:55 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>> Ilia Mirkin <imirkin at 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...
>

Why would they do something like that?  If ARB_depth_texture is not
supported, I don't see how it would make sense to forbid "passing" Z24S8
data to a texture, but still require support for it as internal format.

I just checked what the nvidia blob does: It exposes
EXT_packed_depth_stencil on nv1x and it doesn't support depth texturing,
just like we do.

> Anyone else have opinions on this?
>
>   -ilia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140313/5e28e2f1/attachment-0001.pgp>