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

[Samuel Pitoiset]

On 07/25/2017 04:54 PM, Nicolai Hähnle wrote:
> On 25.07.2017 15:44, Ilia Mirkin wrote:
>> On Tue, Jul 25, 2017 at 9:09 AM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>>
>>> 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.
>>
>> I've used it on rare occasions, although it's a lot less helpful than
>> it might be. I think ultimately it's done in the wrong place. If we
>> want such a thing, it should hook into the dispatch logic and
>> automated.
> 
> Yeah, with all those no_error changes, it seems we're repeating this 
> discussion every few months ;)
> 
> I'd be willing to review patches that move this into auto-generated 
> dispatch logic.

Yeah, that seems better. I will think about this. :)

Samuel.

> 
> Cheers,
> Nicolai