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

[Iago Toral]

On Tue, 2014-12-02 at 08:24 -0500, Ilia Mirkin wrote:
> 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.

Yes, I didn't build with --enable-texture-float, so I guess that
explains it.

Thanks,
Iago

> 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
>