On Tue, Dec 2, 2014 at 8:24 AM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Tue, Dec 2, 2014 at 3:05 AM, Iago Toral <ito...@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 <ito...@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
I believe the issue is that _mesa_TextureView should be checking the target internal format against _mesa_base_tex_format() and returning a GL_INVALID_VALUE error (? the spec conveniently lists errors as 'TODO') if it returns -1. Chris -- thoughts? Maybe stick it in lookup_view_class? -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev