[Piglit] [Mesa-dev] [PATCH] piglit/general/texunits: fix to check the correct extension and be less pedantic

[Luca Barbieri]

> Note: I'm moving this discussion to the piglit list.
Sorry, did not realize there was one (doesn't seem mentioned on the webpage).

> Neither version of the code is correct.  :(  I suspect the correct thing
> is to check for either extension, since both add this query.  From the
> GL_ARB_fragment_program spec:

Ah right the ARB file lists the non-_ARB-suffixed and the ARB-suffixed
one separately:
	MAX_TEXTURE_COORDS				= 0x8871    # ARB_vertex_shader, ARB_fragment_shader

However, MAX_COMBINED_TEXTURE_IMAGE_UNITS_ARB still requires
ARB_vertex_shader as far as I can tell.

> On which drivers do you see this?  I don't think it's correct.  If
> completely non-programmable GPU advertises N texture units, the spec is
> pretty clear that using units beyond N-1 is an error.  I'm not aware of
> any ambiguity in the spec language here.  Or does this only occur when
> GL_ARB_vertex_shader is not supported but GL_ARB_fragment_program is?

On the nv40 Gallium driver without GLSL exposed (but with
GL_ARB_fragment_program, of course).

[this is a temporary situation, and GLSL will be introduced soon, but
this still holds]

Does the spec really force the implementation to return an error for
things it does not advertise, even if they are actually supported?

The issue here is that we have only 8 texcoords varyings, but 16
fragment textures.
The driver/state tracker correctly set the internal Mesa constants,
which Mesa apparently honors.
On the other hand, since the extension is not exposed, Mesa decides to
refuse to give out the constants and raises errors instead.

Thus, the application only sees GL_MAX_TEXTURE_UNITS = 8, but you can
use texture 8-15 as well.

Basically leaving the test as is means forcing Mesa to not support
textures 8-15 when GLSL is not supported, just because it cannot
advertise them, which seems stupid.

In general the following things seem dubious to me:
1. The test checking the presence of a (not very related, except for
happening to use the functionality) extension instead of whether
glGetInteger returns an error or not
2. Mesa checking the presence of the extension and, seeing it is
absent, not giving out the constants it does in fact have and raising
an error instead
3. The test checking that errors are triggered for unadvertised
functionality, as opposed to only checking whether both the advertised
and the non-error-raising functionality works, or better, leaving that
to specific tests

As far as I can see, either fixing (3) (as the current patch does), or
both (1) and (2) will solve the problem.
Castrating Mesa to only support  8 textures in this case will too, but
it doesn't seem great.

Ideally maybe we should fix all three points.

Perhaps you could add a third mode that only asks for
GL_MAX_TEXTURE_COORDS in the "fp but not GLSL case", but then you
still have the problem for vertex textures (which could be exposed via
GL_NV_vertex_program3 or some other extension instead of GLSL).