On 12/08/2015 07:15 AM, Timothy Arceri wrote:
On Mon, 2015-12-07 at 11:29 +0200, Tapani Pälli wrote:
Validation checks that we do not have active shader stages that are
not used by the pipeline.

This fixes a subtest in following CTS test:
        ES31-CTS.sepshaderobjs.StateInteraction

Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
  src/mesa/main/api_validate.c | 38
++++++++++++++++++++++++++++++++++++++
  1 file changed, 38 insertions(+)

diff --git a/src/mesa/main/api_validate.c
b/src/mesa/main/api_validate.c
index d693ec6..6a0cbb0 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -47,6 +47,44 @@ check_valid_to_render(struct gl_context *ctx,
const char *function)
switch (ctx->API) {
     case API_OPENGLES2:
When moving this to pipline validation as per my previous email this
also needs to be left enabled for desktop GL too as this rule is in the
4.5 core spec also.

OK cool, will enable for both.


+      /* If SSO in use, validate that all linked program stages are
used by
+       * the current pipeline.
+       *
+       * OpenGL ES 3.1 spec (11.1.3.11 Validation):
+       *
+       *    "An INVALID_OPERATION error is generated by any command
that trans-
+       *    fers vertices to the GL or launches compute work if the
current set
+       *    of active program objects cannot be executed, for
reasons including:
+       *
+       *    ...
+       *
+       *    "A program object is active for at least one, but not
all of
+       *    the shader stages that were present when the program was
linked."
+       *
+       */
+      if (ctx->Pipeline.Current) {
+          struct gl_pipeline_object *pipe = ctx->Pipeline.Current;
+          unsigned i;
+          for (i = 0; i < MESA_SHADER_STAGES; i++) {
+             if (!pipe->CurrentProgram[i])
+                continue;
+
+             /* Check that each active stage of linked program is
used.
+              * This is done by comparing ActiveStages masks of
program and
+              * pipeline. Program mask must not contain active
stages that
+              * are not marked used by the pipeline.
+              */
+             struct gl_shader_program *shProg = pipe
->CurrentProgram[i];
+             if (((pipe->ActiveStages & shProg->ActiveStages) ^
+                 shProg->ActiveStages) != 0) {
The problem that I see here is that this will test to see that all
stages that exist in the program are used in the pipeline, however it
doesn't check that the stages used are the stages from the program the
could be from another program.

For example the pipeline could have a program with both vert and frag
shaders where only the vert shader is marked as used and another
program just with a frag shader which is marked as used. In which case
this patch would pass when it should in fact fail.

Right, did not spot this as this is something that is not tested by CTS. But we do have pointers to programs from pipeline so I *think* this check could be added here.

I believe the intention of this validation is to make sure a
combination like the scenario above is not used as when the vert and
frag shader are linked in the first program then any inactive varyings
will be optimised away which the second program containing the frag
shader might have been expecting.

It also does not make sense to have linked stage that is not used. I'm currently rewriting the interface match check 'validate_io' which is the final check that everything is ok with varyings.



+                _mesa_error(ctx, GL_INVALID_OPERATION,
+                            "Shader program active for shader stages
that "
+                            "are not used by the pipeline");
+                return false;
+             }
+          }
+      }
+
        /* For ES2, we can draw if we have a vertex program/shader).
*/
        return ctx->VertexProgram._Current != NULL;

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to