[Mesa-stable] [Mesa-dev] [PATCH] glsl: always re-validate program pipeline

[Timothy Arceri]

On Tue, 2015-12-01 at 13:46 +0200, Tapani Pälli wrote:
> On 12/01/2015 02:13 AM, Timothy Arceri wrote:
> > Just because the validation passed the last time is was called
> > doesn't
> > automatically mean it will pass again the next time its called.
> 
> This is a rather large hammer though :/ Maybe we should look at 
> invalidating the pipeline when samplers are changed instead. Was
> there 
> other issues this resolves/fixes than sampler validation?

There are no other tests that this fixes however I believe that's just
because the tests use ValidateProgramPipeline to check for problems
rather than issuing a draw call. For example this patch is the
equivalent of doing this in ValidateProgramPipeline

if (!ctx->_Shader->Validated)
   _mesa_validate_program_pipeline(ctx, pipe, false);

And this causes the CTS pipeline test to fail. The interesting thing is
that the piglit test this fixes also fails on Nvidia.

The spec is somewhat confusing in its wording:

"Therefore validation is done when the first rendering command which
triggers shader invocations is issued, to determine if the set of
active program objects can be executed."

This seem like its saying we only need to check it on the first call
however even then ctx->_Shader->Validated may have been set by
ValidateProgramPipeline rather than validation triggered a rendering
command.

Also the spec goes on to say:

"An INVALID_OPERATION error is generated by any command that trans-fers
vertices to the GL or launches compute work"

Which seems to conflict with the first statement.

It seems to me the intention is to call validation each time as
relinking or changing samplers could cause validation to fail.

Maybe we should have a test which does a draw call, then relinks with
SEPARABLE to false, then calls draw again and checks for a failure.

A less hammer type approach may be to reset ctx->_Shader->Validated
after linking and when samplers are changed but we really need more
tests to make sure we get this right.

> 
> > Cc: "11.1" <mesa-stable at lists.freedesktop.org>
> > https://bugs.freedesktop.org/show_bug.cgi?id=93180
> > ---
> >   src/mesa/main/context.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> > index be542dd..11747d9 100644
> > --- a/src/mesa/main/context.c
> > +++ b/src/mesa/main/context.c
> > @@ -2030,7 +2030,7 @@ _mesa_valid_to_render(struct gl_context *ctx,
> > const char *where)
> >      }
> >   
> >      /* A pipeline object is bound */
> > -   if (ctx->_Shader->Name && !ctx->_Shader->Validated) {
> > +   if (ctx->_Shader->Name) {
> >         /* Error message will be printed inside
> > _mesa_validate_program_pipeline.
> >          */
> >         if (!_mesa_validate_program_pipeline(ctx, ctx->_Shader,
> > GL_TRUE)) {
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable