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

[Samuel Pitoiset]

On 07/25/2017 03:08 PM, Brian Paul wrote:
> 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?

I don't use it either, but I remember someone else attempted to remove 
it (maybe Emil?), without success.

> 
> -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)
>>>
>>
>>
>