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

[Nicolai Hähnle]

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.

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.