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-sta...@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-sta...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-stable _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
