[Mesa-dev] [PATCH] glsl: implement recent spec update to SSO validation

[Timothy Arceri]

On Mon, 2015-11-23 at 15:19 +0200, Tapani Pälli wrote:
> Hi;
> 
> On 11/23/2015 02:24 PM, Timothy Arceri wrote:
> > From: Timothy Arceri <timothy.arceri at collabora.com>
> > 
> > Enables 200+ dEQP SSO tests to proceed passed validation,
> > while not regressing ES31-CTS.sepshaderobjs.PipelineApi.
> > 
> > Cc: Tapani Pälli <tapani.palli at intel.com>
> > Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
> > ---
> >   src/mesa/main/pipelineobj.c | 25 ++++++++++++++++++++++++-
> >   1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/main/pipelineobj.c
> > b/src/mesa/main/pipelineobj.c
> > index 90dff13..99e1491 100644
> > --- a/src/mesa/main/pipelineobj.c
> > +++ b/src/mesa/main/pipelineobj.c
> > @@ -646,7 +646,7 @@ _mesa_GetProgramPipelineiv(GLuint pipeline,
> > GLenum pname, GLint *params)
> >         return;
> >      case GL_VALIDATE_STATUS:
> >         /* If pipeline is not bound, return initial value 0. */
> > -      *params = (ctx->_Shader->Name != pipe->Name) ? 0 : pipe
> > ->Validated;
> > +      *params = pipe->Validated;
> 
> I agree with the added check below but I feel worried with this
> change 
> as the test is testing explicitly if pipeline was bound before the
> call, 
> here it just happens to be empty as well.


Hi,

Please take a look at Khronos bug 12176 the spec was updated with this
text to reflect this test so I'm very confident this is all the test is
expecting us to do.

> 
> I guess my main worry is that we don't seem to be implementing
> following:
> 
> "If pipeline is a name that has been generated (without subsequent 
> deletion) by GenProgramPipelines, but refers to a program pipeline 
> object that has not been previously bound, the GL first creates a new
> state vector in the same manner as when BindProgramPipeline creates a
> new program pipeline object."

That's just saying that a new object is created when we call
 validateProgramPipeline() it doesn't say anything about how to
validate the new object, the spec addition does.

Tim


> 
> So we don't realy create 'new state vector' for a new program
> pipeline 
> object but try to return same values as one would have?
> 
> 
> >         return;
> >      case GL_VERTEX_SHADER:
> >         *params = pipe->CurrentProgram[MESA_SHADER_VERTEX]
> > @@ -858,6 +858,29 @@ _mesa_validate_program_pipeline(struct
> > gl_context* ctx,
> >         }
> >      }
> > 
> > +   /* Section 11.1.3.11 (Validation) of the OpenGL 4.5 spec says:
> > +    *
> > +    *    "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:
> > +    *
> > +    *       ...
> > +    *
> > +    *       - There is no current program object specified by
> > UseProgram,
> > +    *         there is a current program pipeline object, and that
> > object is
> > +    *         empty (no executable code is installed for any
> > stage).
> > +    */
> > +   bool program_empty = true;
> > +   for (i = 0; i < MESA_SHADER_STAGES; i++) {
> > +      if (pipe->CurrentProgram[i]) {
> > +         program_empty = false;
> > +         break;
> > +      }
> > +   }
> > +   if(program_empty) {
> > +      goto err;
> > +   }
> > +
> >      /* Section 2.11.11 (Shader Execution), subheading
> > "Validation," of the
> >       * OpenGL 4.1 spec says:
> >       *
> > 
> 
> // Tapani