[Piglit] [PATCH 1/6] shader_runner and glslparsertest: choose GL version more accurately.

[Paul Berry]

On 19 February 2013 23:11, Michel Dänzer <michel at daenzer.net> wrote:

> On Die, 2013-02-19 at 12:52 +0100, Michel Dänzer wrote:
> > On Mon, 2013-02-18 at 10:14 -0800, Paul Berry wrote:
> > > On 18 February 2013 07:53, Michel Dänzer <michel at daenzer.net> wrote:
> > >         On Don, 2013-02-14 at 08:50 -0800, Paul Berry wrote:
> > >         > Previously, both shader_runner and glslparsertest contained
> > >         switch
> > >         > statements that attempted to choose a GL version for testing
> > >         based on
> > >         > a GLSL version requirement, and those switch statements
> > >         didn't choose
> > >         > a GL version very accurately (for example, they converted
> > >         GLSL version
> > >         > 1.50 to GL version 3.1, which doesn't work).
> > >         >
> > >         > This patch replaces the switch statements with a single
> > >         common
> > >         > function (in piglit-util-gl-common) that does the right
> > >         thing.
> > >
> > >
> > >         This change caused quick-driver.tests to skip more than 2000
> > >         tests on
> > >         radeonsi (GL 2.1, GLSL 1.2). Almost all of the skipped tests
> > >         were
> > >         previously passing.
> > >
> > >         Was this intended?
> > >
> > >
> > > No, this wasn't expected, and probably indicates a bug in the driver.
> > >
> > > The specific behavioural change in Piglit is that previously, when
> > > running a test that required GLSL 1.20, Piglit would request (via
> > > Waffle) a GL 1.0 context.  Then it would query the context to see if
> > > it supported GLSL 1.20.  After this patch, when running a test that
> > > requires GLSL 1.20, Piglit requests a GL 2.1 context, and then skips
> > > if a GL 2.1 context can't be created.  On an implementation that
> > > supports GL 2.1 and GLSL 1.20, both methods should yield the same
> > > results.
> > >
> > >
> > > Note that there is code in core Mesa to determine, after context
> > > creation, which versions of GL and GLSL are supported.  But at the
> > > time of context creation, it is the responsibility of driver-specific
> > > code to determine whether the requested context version is supported
> > > (see, for instance, validate_context_version() in
> > > src/mesa/drivers/dri/intel/intel_context.c).  This patch caused piglit
> > > to base its decision for whether to skip on the driver-specific code
> > > rather than the core Mesa code, so if there was a lurking bug in the
> > > driver-specific code, this patch would have exposed it.
> > >
> > >
> > > So my suspicion is that there's a bug in the driver-specific context
> > > creation code causing it to incorrectly reject requests for GL 2.1
> > > contexts.  Can you check if that's what's happening?  I don't know my
> > > way around the radeon context creation code well enough to check it
> > > myself.
> >
> > AFAICT the Gallium DRI/OpenGL state trackers should handle that
> > correctly. The problem seems to be that I'm testing on xserver 1.12,
> > which doesn't support GLX_ARB_create_context. Is it really no longer
> > feasible to run those tests without it?
>
> In particular, creating a 2.1 context doesn't really require
> GLX_ARB_create_context, does it?
>

Sorry for not responding sooner--yesterday was unexpectedly busy.  You are
right, creating a 2.1 context shouldn't require GLX_ARB_create_context.

It seems to me that the correct fix here is to have either the Piglit
framework or Waffle (I'm not sure which) detect the situation where
GLX_ARB_create_context is unsupported, and in that case do the following:

(a) If the test is trying to create a "compatibility" context with GL
version 1.0, go ahead and create an ordinary context without using
GLX_ARB_create_context.

(b) If the test is trying to create a "compatibility" context with GL
version other than 1.0, go ahead and create an ordinary context without
using GLX_ARB_create_context, and then check using glGetString() whether a
sufficiently high version of GL was returned.  If not, skip the test.

(c) If the test is trying tocreate a core context, skip.

The Piglit framework and Waffle collectively implement behaviours (a) and
(c) already; all that would need to be added is (b).

Chad, does this seem like a reasonable proposal to you?  If so, where do
you think it's appropriate to implement (Piglit framework or Waffle)?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130220/8b65a75c/attachment.html>