[Mesa-dev] [PATCH 1/2] mesa: add bind_program_pipeline() helper

[Brian Paul]

On 07/25/2017 02:04 AM, Nicolai Hähnle wrote:
> On 21.07.2017 15:16, Samuel Pitoiset wrote:
>> To reduce code duplication.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/mesa/main/pipelineobj.c | 62
>> ++++++++++++++++++---------------------------
>>   1 file changed, 25 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>> index f40111108c..79d97c2211 100644
>> --- a/src/mesa/main/pipelineobj.c
>> +++ b/src/mesa/main/pipelineobj.c
>> @@ -429,44 +429,11 @@ _mesa_ActiveShaderProgram(GLuint pipeline,
>> GLuint program)
>>      _mesa_reference_shader_program(ctx, &pipe->ActiveProgram, shProg);
>>   }
>> -void GLAPIENTRY
>> -_mesa_BindProgramPipeline_no_error(GLuint pipeline)
>> -{
>> -   GET_CURRENT_CONTEXT(ctx);
>> -   struct gl_pipeline_object *newObj = NULL;
>> -
>> -   /* Rebinding the same pipeline object: no change.
>> -    */
>> -   if (ctx->_Shader->Name == pipeline)
>> -      return;
>> -
>> -   /* Get pointer to new pipeline object (newObj)
>> -    */
>> -   if (pipeline) {
>> -      /* non-default pipeline object */
>> -      newObj = _mesa_lookup_pipeline_object(ctx, pipeline);
>> -
>> -      /* Object is created by any Pipeline call but
>> glGenProgramPipelines,
>> -       * glIsProgramPipeline and GetProgramPipelineInfoLog
>> -       */
>> -      newObj->EverBound = GL_TRUE;
>> -   }
>> -
>> -   _mesa_bind_pipeline(ctx, newObj);
>> -}
>> -
>> -/**
>> - * Make program of the pipeline current
>> - */
>> -void GLAPIENTRY
>> -_mesa_BindProgramPipeline(GLuint pipeline)
>> +static ALWAYS_INLINE void
>> +bind_program_pipeline(struct gl_context *ctx, GLuint pipeline, bool
>> no_error)
>>   {
>> -   GET_CURRENT_CONTEXT(ctx);
>>      struct gl_pipeline_object *newObj = NULL;
>> -   if (MESA_VERBOSE & VERBOSE_API)
>> -      _mesa_debug(ctx, "glBindProgramPipeline(%u)\n", pipeline);
>
> Personally, I would leave the debug print here instead of moving it. In
> release builds it is compiled away anyway. An analogous comment applies
> to the second patch.

I think the whole MESA_VERBOSE stuff could be removed, actually. 
Nowadays we have apitrace to log/examine API calls.  I haven't used 
MESA_VERBOSE in years.  Anyone else?

-Brian


>
> Either way, series is
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
>
>> -
>>      /* Rebinding the same pipeline object: no change.
>>       */
>>      if (ctx->_Shader->Name == pipeline)
>> @@ -482,7 +449,7 @@ _mesa_BindProgramPipeline(GLuint pipeline)
>>       *         - by BindProgramPipeline if the current transform
>> feedback
>>       *           object is active and not paused;
>>       */
>> -   if (_mesa_is_xfb_active_and_unpaused(ctx)) {
>> +   if (!no_error && _mesa_is_xfb_active_and_unpaused(ctx)) {
>>         _mesa_error(ctx, GL_INVALID_OPERATION,
>>               "glBindProgramPipeline(transform feedback active)");
>>         return;
>> @@ -493,7 +460,7 @@ _mesa_BindProgramPipeline(GLuint pipeline)
>>      if (pipeline) {
>>         /* non-default pipeline object */
>>         newObj = _mesa_lookup_pipeline_object(ctx, pipeline);
>> -      if (!newObj) {
>> +      if (!no_error && !newObj) {
>>            _mesa_error(ctx, GL_INVALID_OPERATION,
>>                        "glBindProgramPipeline(non-gen name)");
>>            return;
>> @@ -508,6 +475,27 @@ _mesa_BindProgramPipeline(GLuint pipeline)
>>      _mesa_bind_pipeline(ctx, newObj);
>>   }
>> +void GLAPIENTRY
>> +_mesa_BindProgramPipeline_no_error(GLuint pipeline)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +   bind_program_pipeline(ctx, pipeline, true);
>> +}
>> +
>> +/**
>> + * Make program of the pipeline current
>> + */
>> +void GLAPIENTRY
>> +_mesa_BindProgramPipeline(GLuint pipeline)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +
>> +   if (MESA_VERBOSE & VERBOSE_API)
>> +      _mesa_debug(ctx, "glBindProgramPipeline(%u)\n", pipeline);
>> +
>> +   bind_program_pipeline(ctx, pipeline, false);
>> +}
>> +
>>   void
>>   _mesa_bind_pipeline(struct gl_context *ctx,
>>                       struct gl_pipeline_object *pipe)
>>
>
>