[Mesa-dev] [PATCH 2/2] mesa: move GL_INVALID_OPERATION error to rendering call

[Tapani Pälli]

Yes, nice cleanup

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 12/06/2015 06:36 AM, Timothy Arceri wrote:
> The validation api doesn't trigger this error so just move it to the
> code called during rendering.
>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>   src/mesa/main/context.c     |  7 ++++---
>   src/mesa/main/pipelineobj.c | 30 +++++++++---------------------
>   src/mesa/main/pipelineobj.h |  3 ++-
>   3 files changed, 15 insertions(+), 25 deletions(-)
>
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index be542dd..b3fc3bd 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -2031,9 +2031,10 @@ _mesa_valid_to_render(struct gl_context *ctx, const char *where)
>
>      /* A pipeline object is bound */
>      if (ctx->_Shader->Name && !ctx->_Shader->Validated) {
> -      /* Error message will be printed inside _mesa_validate_program_pipeline.
> -       */
> -      if (!_mesa_validate_program_pipeline(ctx, ctx->_Shader, GL_TRUE)) {
> +      if (!_mesa_validate_program_pipeline(ctx, ctx->_Shader)) {
> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "glValidateProgramPipeline failed to validate the "
> +                     "pipeline");
>            return GL_FALSE;
>         }
>      }
> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> index d8c9ded..5eda4e5 100644
> --- a/src/mesa/main/pipelineobj.c
> +++ b/src/mesa/main/pipelineobj.c
> @@ -757,8 +757,7 @@ program_stages_interleaved_illegally(const struct gl_pipeline_object *pipe)
>
>   extern GLboolean
>   _mesa_validate_program_pipeline(struct gl_context* ctx,
> -                                struct gl_pipeline_object *pipe,
> -                                GLboolean IsBound)
> +                                struct gl_pipeline_object *pipe)
>   {
>      unsigned i;
>      bool program_empty = true;
> @@ -789,7 +788,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>       */
>      for (i = 0; i < MESA_SHADER_STAGES; i++) {
>         if (!program_stages_all_active(pipe, pipe->CurrentProgram[i])) {
> -         goto err;
> +         return GL_FALSE;
>         }
>      }
>
> @@ -810,7 +809,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>            ralloc_strdup(pipe,
>                          "Program is active for multiple shader stages with an "
>                          "intervening stage provided by another program");
> -      goto err;
> +      return GL_FALSE;
>      }
>
>      /* Section 2.11.11 (Shader Execution), subheading "Validation," of the
> @@ -831,7 +830,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>              pipe->CurrentProgram[MESA_SHADER_TESS_CTRL] ||
>              pipe->CurrentProgram[MESA_SHADER_TESS_EVAL])) {
>         pipe->InfoLog = ralloc_strdup(pipe, "Program lacks a vertex shader");
> -      goto err;
> +      return GL_FALSE;
>      }
>
>      /* Section 2.11.11 (Shader Execution), subheading "Validation," of the
> @@ -854,7 +853,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>                                            "Program %d was relinked without "
>                                            "PROGRAM_SEPARABLE state",
>                                            pipe->CurrentProgram[i]->Name);
> -         goto err;
> +         return GL_FALSE;
>         }
>      }
>
> @@ -878,7 +877,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>      }
>
>      if (program_empty) {
> -      goto err;
> +      return GL_FALSE;
>      }
>
>      /* Section 2.11.11 (Shader Execution), subheading "Validation," of the
> @@ -896,7 +895,7 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>       *           maximum number of texture image units allowed."
>       */
>      if (!_mesa_sampler_uniforms_pipeline_are_valid(pipe))
> -      goto err;
> +      return GL_FALSE;
>
>      /* Validate inputs against outputs, this cannot be done during linking
>       * since programs have been linked separately from each other.
> @@ -911,17 +910,10 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>       * OpenGL ES 3.1 specification has the same text.
>       */
>      if (!_mesa_validate_pipeline_io(pipe))
> -      goto err;
> +      return GL_FALSE;
>
>      pipe->Validated = GL_TRUE;
>      return GL_TRUE;
> -
> -err:
> -   if (IsBound)
> -      _mesa_error(ctx, GL_INVALID_OPERATION,
> -                  "glValidateProgramPipeline failed to validate the pipeline");
> -
> -   return GL_FALSE;
>   }
>
>   /**
> @@ -943,11 +935,7 @@ _mesa_ValidateProgramPipeline(GLuint pipeline)
>         return;
>      }
>
> -   /* ValidateProgramPipeline should not throw errors when pipeline validation
> -    * fails and should instead only update the validation status. We pass
> -    * false for IsBound to avoid an error being thrown.
> -    */
> -   _mesa_validate_program_pipeline(ctx, pipe, false);
> +   _mesa_validate_program_pipeline(ctx, pipe);
>   }
>
>   void GLAPIENTRY
> diff --git a/src/mesa/main/pipelineobj.h b/src/mesa/main/pipelineobj.h
> index 6dee775..fbcb765 100644
> --- a/src/mesa/main/pipelineobj.h
> +++ b/src/mesa/main/pipelineobj.h
> @@ -67,7 +67,8 @@ _mesa_bind_pipeline(struct gl_context *ctx,
>                       struct gl_pipeline_object *pipe);
>
>   extern GLboolean
> -_mesa_validate_program_pipeline(struct gl_context * ctx, struct gl_pipeline_object *pipe, GLboolean IsBound);
> +_mesa_validate_program_pipeline(struct gl_context * ctx,
> +                                struct gl_pipeline_object *pipe);
>
>
>   extern void GLAPIENTRY
>