On 01/06/2016 03:40 AM, Timothy Arceri wrote:
Even if re-linking fails rendering shouldn't fail as the previous
succesfully linked program will still be available. It also shouldn't
be possible to have an unlinked program as part of the current rendering
state.

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

Which is the last one, after this change this horrible test from depths of hell starts to finally pass!

It would be cool if Ian and Brian could comment a bit here why these checks were originally done and if they can be safely removed, was is just to play 'extra safe'?


This change should improve performance on CPU limited benchmarks as noted
in commit d6c6b186cf308f.

 From Section 7.3 (Program Objects) of the OpenGL 4.5 spec:

    "If a program object that is active for any shader stage is re-linked
     unsuccessfully, the link status will be set to FALSE, but any existing
     executables and associated state will remain part of the current rendering
     state until a subsequent call to UseProgram, UseProgramStages, or
     BindProgramPipeline removes them from use. If such a program is attached to
     any program pipeline object, the existing executables and associated state
     will remain part of the program pipeline object until a subsequent call to
     UseProgramStages removes them from use. An unsuccessfully linked program 
may
     not be made part of the current rendering state by UseProgram or added to
     program pipeline objects by UseProgramStages until it is successfully
     re-linked."

    "void UseProgram(uint program);

    ...

    An INVALID_OPERATION error is generated if program has not been linked, or
    was last linked unsuccessfully.  The current rendering state is not 
modified."

V2: apply the rule to both core and compat.

Cc: Tapani Pälli <[email protected]>
Cc: Ian Romanick <[email protected]>
Cc: Brian Paul <[email protected]>
---
  src/mesa/main/context.c | 63 +++----------------------------------------------
  1 file changed, 3 insertions(+), 60 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index be983d4..f3fd01f 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1930,31 +1930,6 @@ _mesa_check_blend_func_error(struct gl_context *ctx)
     return GL_TRUE;
  }

-static bool
-shader_linked_or_absent(struct gl_context *ctx,
-                        const struct gl_shader_program *shProg,
-                        bool *shader_present, const char *where)
-{
-   if (shProg) {
-      *shader_present = true;
-
-      if (!shProg->LinkStatus) {
-         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(shader not linked)", 
where);
-         return false;
-      }
-#if 0 /* not normally enabled */
-      {
-         char errMsg[100];
-         if (!_mesa_validate_shader_program(ctx, shProg, errMsg)) {
-            _mesa_warning(ctx, "Shader program %u is invalid: %s",
-                          shProg->Name, errMsg);
-         }
-      }
-#endif
-   }
-
-   return true;
-}

  /**
   * Prior to drawing anything with glBegin, glDrawArrays, etc. this function
@@ -1967,54 +1942,22 @@ shader_linked_or_absent(struct gl_context *ctx,
  GLboolean
  _mesa_valid_to_render(struct gl_context *ctx, const char *where)
  {
-   unsigned i;
-
     /* This depends on having up to date derived state (shaders) */
     if (ctx->NewState)
        _mesa_update_state(ctx);

-   if (ctx->API == API_OPENGL_CORE || ctx->API == API_OPENGLES2) {
-      bool from_glsl_shader[MESA_SHADER_COMPUTE] = { false };
-
-      for (i = 0; i < MESA_SHADER_COMPUTE; i++) {
-         if (!shader_linked_or_absent(ctx, ctx->_Shader->CurrentProgram[i],
-                                      &from_glsl_shader[i], where))
-            return GL_FALSE;
-      }
-
-      /* In OpenGL Core Profile and OpenGL ES 2.0 / 3.0, there are no assembly
-       * shaders.  Don't check state related to those.
-       */
-   } else {
-      bool has_vertex_shader = false;
-      bool has_fragment_shader = false;
-
-      /* In OpenGL Compatibility Profile, there is only vertex shader and
-       * fragment shader.  We take this path also for API_OPENGLES because
-       * optimizing that path would make the other (more common) paths
-       * slightly slower.
-       */
-      if (!shader_linked_or_absent(ctx,
-                                   
ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX],
-                                   &has_vertex_shader, where))
-         return GL_FALSE;
-
-      if (!shader_linked_or_absent(ctx,
-                                   
ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT],
-                                   &has_fragment_shader, where))
-         return GL_FALSE;
-
+   if (ctx->API == API_OPENGL_COMPAT) {
        /* Any shader stages that are not supplied by the GLSL shader and have
         * assembly shaders enabled must now be validated.
         */
-      if (!has_vertex_shader
+      if (!ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX]
            && ctx->VertexProgram.Enabled && !ctx->VertexProgram._Enabled) {
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "%s(vertex program not valid)", where);
           return GL_FALSE;
        }

-      if (!has_fragment_shader) {
+      if (!ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT]) {
           if (ctx->FragmentProgram.Enabled && !ctx->FragmentProgram._Enabled) {
              _mesa_error(ctx, GL_INVALID_OPERATION,
                          "%s(fragment program not valid)", where);

_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to