[Mesa-dev] [PATCH 1/2] mesa/compute: move compute checks around for tests.

[Antía Puentes]

Hi!

I have taken a look to the GL44-CTS.compute_shader.api-indirect test
and I agree with Piñeiro that this test is not correct. A call to
glUseProgram to set the active program is missing in the test.

The test passes invalid values as parameters to
glDispatchComputeIndirect
like,
	glDispatchComputeIndirect(-2);

and checks if the error returned by the the implementation is the one
expected.

However, as the test does not set the active program, Mesa returns the
INVALID_OPERATION ("no active compute shader") error instead of the one
expected by the test but, this can not be considered as a nonconformity
because implementations can choose which error generate when several
error conditions are met. For that reason the test is invalid unless it
sets the active program.

Adding the missing glUseProgram call to the test, Mesa returns the
expected errors and the test passes in i965. Notice that the lack of an
active compute shader when calling to glDispatchComputeIndirect is
already tested by a different test:
GL44-CTS.compute_shader.api-no-active-program.

On jue, 2016-05-05 at 09:51 +0200, Alejandro Piñeiro wrote:
> So as far as I understand, on that test there is no active program
> and
> indirect length is wrong, and fails because it was expecting the
> second
> error. Is that right?
> 
> Unless Im wrong, when the OpenGL spec specifies the Error cases, it
> doesn't specify any kind of priority (which error should be raised
> first
> if more of one condition happens).
> 
> Im somewhat biased to think that it is a problem with the test. What
> would happen if a new test is written to check that INVALID_OPERATION
> is
> generated when no active program, and it uses a wrong length? IMHO,
> if
> the test is setting two wrong conditions, it should check for the two
> possible errors.
> 
> On 05/05/16 02:41, Dave Airlie wrote:
> > 
> > From: Dave Airlie <airlied at redhat.com>
> > 
> > This fixes GL43-CTS.compute_shader.api-indirect
> > which tests the length/4 before anything else.
> > 
> > Signed-off-by: Dave Airlie <airlied at redhat.com>
> > ---
> >  src/mesa/main/api_validate.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/mesa/main/api_validate.c
> > b/src/mesa/main/api_validate.c
> > index 688408f..d455f19 100644
> > --- a/src/mesa/main/api_validate.c
> > +++ b/src/mesa/main/api_validate.c
> > @@ -1109,9 +1109,6 @@ valid_dispatch_indirect(struct gl_context
> > *ctx,
> >  {
> >     GLintptr end = (GLintptr)indirect + size;
> >  
> > -   if (!check_valid_to_compute(ctx, name))
> > -      return GL_FALSE;
> > -
> >     /* From the OpenGL 4.3 Core Specification, Chapter 19, Compute
> > Shaders:
> >      *
> >      * "An INVALID_VALUE error is generated if indirect is negative
> > or is not a
> > @@ -1153,6 +1150,9 @@ valid_dispatch_indirect(struct gl_context
> > *ctx,
> >        return GL_FALSE;
> >     }
> >  
> > +   if (!check_valid_to_compute(ctx, name))
> > +      return GL_FALSE;
> > +
> >     return GL_TRUE;
> >  }
> >  
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev