[Mesa-dev] [PATCH 04/12] sso: implement ActiveShaderProgram & GetProgramPipelineiv

[gregory hainaut]

On Fri, 3 May 2013 11:56:39 -0700
Matt Turner <mattst88 at gmail.com> wrote:

> On Fri, May 3, 2013 at 10:44 AM, Gregory Hainaut
> <gregory.hainaut at gmail.com> wrote:
> > V2:
> > * Rename object
> > * Formatting improvement
> > ---
> >  src/mesa/main/pipelineobj.c |   77 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >
> > diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> > index d81bd0e..ffbeeae 100644
> > --- a/src/mesa/main/pipelineobj.c
> > +++ b/src/mesa/main/pipelineobj.c
> > @@ -231,6 +231,30 @@ _mesa_UseProgramStages(GLuint pipeline, GLbitfield stages, GLuint program)
> >  void GLAPIENTRY
> >  _mesa_ActiveShaderProgram(GLuint pipeline, GLuint program)
> >  {
> > +   GET_CURRENT_CONTEXT(ctx);
> > +   struct gl_shader_program *shProg = (program != 0)
> > +      ? _mesa_lookup_shader_program_err(ctx, program, "glActiveShaderProgram(program)")
> > +      : NULL;
> > +
> > +   struct gl_pipeline_object *pipe = lookup_pipeline_object(ctx, pipeline);
> > +
> > +   if (!pipe) {
> > +      _mesa_error(ctx, GL_INVALID_OPERATION, "glActiveShaderProgram(pipeline)");
> > +      return;
> > +   }
> > +
> > +   /* Object is created by any Pipeline call but glGenProgramPipelines,
> > +    * glIsProgramPipeline and GetProgramPipelineInfoLog
> > +    */
> > +   pipe->EverBound = GL_TRUE;
> > +
> > +   if ((shProg != NULL) && !shProg->LinkStatus) {
> > +      _mesa_error(ctx, GL_INVALID_OPERATION,
> > +            "glActiveShaderProgram(program %u not linked)", shProg->Name);
> > +      return;
> > +   }
> > +
> > +   _mesa_reference_shader_program(ctx, &pipe->ActiveProgram, shProg);
> >  }
> >
> >  /**
> > @@ -348,6 +372,59 @@ _mesa_IsProgramPipeline(GLuint pipeline)
> >  void GLAPIENTRY
> >  _mesa_GetProgramPipelineiv(GLuint pipeline, GLenum pname, GLint *params)
> >  {
> > +   GET_CURRENT_CONTEXT(ctx);
> > +   struct gl_pipeline_object *pipe = lookup_pipeline_object(ctx, pipeline);
> > +
> > +   /* Are geometry shaders available in this context?
> > +    */
> > +   const bool has_gs = _mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_geometry_shader4;
> 
> The geometry shaders in GL 3.2 aren't exactly the same as in
> ARB_geometry_shader4, so it's conceivable that we could support 3.2
> without ARB_geometry_shader4. Probably should be changed to
> 
> const bool has_gs = _mesa_is_desktop_gl(ctx) && (ctx->Version >= 32 ||
> ctx->Extensions.ARB_geometry_shader4);
> 
> > +
> > +   if (!pipe) {
> > +      _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramPipelineiv(pipeline)");
> > +      return;
> > +   }
> > +
> > +   /* Object is created by any Pipeline call but glGenProgramPipelines,
> > +    * glIsProgramPipeline and GetProgramPipelineInfoLog
> > +    */
> > +   pipe->EverBound = GL_TRUE;
> > +
> > +   switch (pname) {
> > +   case GL_ACTIVE_PROGRAM:
> > +      *params = pipe->ActiveProgram ? pipe->ActiveProgram->Name : 0;
> > +      return;
> > +   case GL_INFO_LOG_LENGTH:
> > +      // TODO
> > +      *params = 0;
> > +      return;
> > +   case GL_VALIDATE_STATUS:
> > +      *params = pipe->ValidationStatus;
> > +      return;
> > +   case GL_VERTEX_SHADER:
> > +      *params = pipe->CurrentVertexProgram ? pipe->CurrentVertexProgram->Name : 0;
> > +      return;
> > +   case GL_TESS_EVALUATION_SHADER:
> > +      /* NOT YET SUPPORTED */
> > +      break;
> > +   case GL_TESS_CONTROL_SHADER:
> > +      /* NOT YET SUPPORTED */
> > +      break;
> > +   case GL_GEOMETRY_SHADER:
> > +      if (!has_gs) break;
> > +      *params = pipe->CurrentGeometryProgram ? pipe->CurrentGeometryProgram->Name : 0;;
> > +      return;
> > +   case GL_FRAGMENT_SHADER:
> > +      *params = pipe->CurrentFragmentProgram ? pipe->CurrentFragmentProgram->Name : 0;;
> 
> Double ; at the ends of these two lines.
> 
> > +      return;
> > +   case GL_COMPUTE_SHADER:
> > +      /* NOT YET SUPPORTED */
> > +      break;
> 
> GL_COMPUTE_SHADER isn't valid, even in GL 4.3 where compute shaders
> are part of core.
>
Feel like they forget to update the spec when they add this new stage. I will remove it. Thanks.
 
> > +   default:
> > +      break;
> > +   }
> 
> The spec says
> """
> Dependencies on ARB_geometry_shader4, EXT_geometry_shader4, NV_geometry_-
> shader4, and/or OpenGL version 3.2
> 
>     If none of ARB_geometry_shader4, EXT_geometry_shader4, NV_geometry_-
>     shader4, or OpenGL 3.2 are supported by the implementation, ignore
>     all references to geometry shaders and generate an INVALID_ENUM
>     error when UseProgramStages is called with GEOMETRY_SHADER_BIT
>     set in <stages>.
> """
> 
> Similar wording appears for tesselation So I think if a shader stage
> isn't supported that this function should return INVALID_ENUM.

When tesselation (geometry) isn't supported the switch statement will break and report
the GL_INVALID_ENUM below. I think we are good.
Technically the tessalation case is useless, I add it to ease the future
tesselation implementation. I can remove it, if you prefer.
 
> > +
> > +   _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramPipelineiv(pname=%s)",
> > +         _mesa_lookup_enum_by_nr(pname));
> >  }
> >
> >  /**
> > --