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

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Luca Barbieri wrote:
> My interpretation of the problem was somewhat incorrect.
> The issue is that GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS is not exposed,
> and the test does not have a sensible default for it.

Once I actually poked at the test a bit, I had a suspicion that was the
real problem.

> Indeed, if I force that to 16, the test passes.
> So your approach should work (but it is missing
> GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS).

That was intentional: the value was queried but not used.

> Checking whether the queries fail or not seems more robust (and simpler) though.

It would seem to be, but now you've made me nervous that some drivers
might not correctly generate errors for unsupported queries. ;)

> However, I'm somewhat perplexed by the error checking: the idea that
> the OpenGL specification mandates errors seems to be problematic.
> 
> In particular, extensions can introduce behavior that will cause such
> errors to no longer be raised.
> For instance, glActiveTexture(GL_TEXTURE0 + 8) will raise an error
> according to the unextended spec when GL_MAX_TEXTURE_UNITS = 8, but
> not according to ARB_fragment_program with GL_MAX_TEXTURE_UNITS = 8
> and GL_MAX_TEXTURE_IMAGE_UNITS = 16.
> 
> A client application unaware of ARB_fragment_program has no way of
> correctly predicting whether glActiveTexture(GL_TEXTURE0 + 8) will
> fail or succeed.

The intention of the GL errors is that an application can query the
error result during development when they get incorrect rendering.

I think the real source of difficulty is that you don't have to opt-in
to extensions.  This makes it easy for application developers to
accidentally rely on extension behavior that is supported on their
development machine.  When the code is run on a different system, it may
mysteriously fail.

However, this makes it all the more important to generate the right
error codes at the right time.  I can tell you from experience that the
difference between a report of "it renders wrong" vs. "it renders wrong
and it generates an error code of 0x504" is a lot bigger than you might
think.

> So, it would seem either extensions (and ARB ones at that) are allowed
> to change OpenGL in an incompatible way (which clearly is a serious
> problem), or such errors are not mandatory.
> 
> Thus, I think we should not check that they are raised at all, unless
> there is some way of resolving the above issue that I am missing.
> Instead, we can just rely on "positive tests" to check that all
> defined functionality works, and perhaps just print a warning if
> something unexpectedly succeeds.
> 
> Finally, Mesa knows the correct value of
> GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS but is artificially not exposing
> it just because ARB_vertex_shader is not supported.
> This also does not seem to make much sense.

There's nothing artificial about it.  Without GL_ARB_vertex_shader, that
query *does not exist*.  Without fragment shader (or program) support
*and* either vertex or geometry shader (or program) support, there is no
sense in "combined" texture units.  The fragment texture units are
combined with nothing.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvIKBAACgkQX1gOwKyEAw8osACeLYDib1Ajwj3DEhlEYaEPdQqe
OVkAn0Yk3XHm4RlfrGpiM0kPAt9YK0K/
=FoGt
-----END PGP SIGNATURE-----