[Mesa-dev] [PATCH v2 06/23] mesa: Fix incorrect assertion in init_teximage_fields_ms

[Ilia Mirkin]

On Tue, Dec 2, 2014 at 3:05 AM, Iago Toral <itoral at igalia.com> wrote:
> On Mon, 2014-12-01 at 13:25 -0500, Ilia Mirkin wrote:
>> On Mon, Dec 1, 2014 at 6:04 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
>> > _BaseFormat is a GLenum (unsigned int) so testing if its value is
>> > greater than 0 to detect the cases where _mesa_base_tex_format
>> > returns -1 doesn't work.
>> >
>> > Fixing the assertion breaks the arb_texture_view-lifetime-format
>> > piglit test on nouveau, since that test calls
>> > _mesa_base_tex_format with GL_R16F with a context that does not
>> > have ARB_texture_float, so it returns -1 for the BaseFormat, which
>> > was not being catched properly by the ASSERT in init_teximage_fields_ms
>> > until now.
>>
>> Sorry to hijack the thread, but... can you elaborate why this would
>> fail on nouveau but not on i965? Does st/mesa do something differently
>> wrt exposing ARB_texture_float vs i965? Does nouveau do something
>> wrong?
>
> Hi Illia, I didn't investigate the source of the problem in Nouveau or
> why this is different than the other drivers, however I can give more
> details:
>
> We are running piglit against i965, llvmpipe, classic swrast, nouveu and
> radeon, but we only hit the problem with nouveau. The code that triggers
> the assertion is _mesa_base_tex_format (teximage.c), which is called
> with internalFormat = GL_R16F, and then, in that function there is this
> code:
>
> if (ctx->Extensions.ARB_texture_rg) {
>    switch (internalFormat) {
>    case GL_R16F:
>    case GL_R32F:
>       if (!ctx->Extensions.ARB_texture_float)
>          break;
>       return GL_RED;
> ...
>
> On i965, the code returns GL_RED, but in Nouveau it hits the break
> because ctx->Extensions.ARB_texture_float is false in this case (and
> later will return -1 after being unable to find a proper base format).
>
> In the case of Intel, ARB_texture_float is always enabled. In the case
> of Gallium I see this in st_extensions.c:
>
> static const struct st_extension_format_mapping rendertarget_mapping[] =
> {
>       { { o(ARB_texture_float) },
>         { PIPE_FORMAT_R32G32B32A32_FLOAT,
>           PIPE_FORMAT_R16G16B16A16_FLOAT } },
> ...
>
> so if I understand that right, for ARB_texture_float to be activated I
> need PIPE_FORMAT_R32G32B32A32_FLOAT and PIPE_FORMAT_R16G16B16A16_FLOAT
> to be supported, so I guess that at least one of these is not supported
> in the nVidia hardware I am using (NVIDIA GT218 [ION]). If that is not
> the problem, then I guess it would need further investigation, but I
> don't see any other places where Gallium or nouveau set that extension.

The only way I can think of that it wouldn't have ARB_texture_float is
if you didn't build with --enable-texture-float. This would lose you
GL3 support, among other things... Perhaps you're only seeing the
issue on nouveau because it's the only driver other than i965 that
implements ARB_texture_view.

You can see the set of extensions that should be enabled...
http://people.freedesktop.org/~imirkin/glxinfo/glxinfo.html (look at
the GT21x column).

  -ilia