[Mesa-dev] [PATCH 05/21] meta: Fix saving the program pipeline state

[Chia-I Wu]

On Thu, May 1, 2014 at 12:11 AM, Ian Romanick <idr at freedesktop.org> wrote:
> On 04/29/2014 08:43 PM, Chia-I Wu wrote:
>> On Wed, Apr 30, 2014 at 8:52 AM, Ian Romanick <idr at freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> This code was broken in some odd ways before.  Too much state was being
>>> saved, it was being restored in the wrong order, and in the wrong way.
>>> The biggest problem was that the pipeline object was restored before
>>> restoring the programs attached to the default pipeline.
>>>
>>> Fixes a regression in the glean texgen test.
>>>
>>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>>> Reviewed-by: Eric Anholt <eric at anholt.net>
>>> ---
>>>  src/mesa/drivers/common/meta.c | 34 ++++++++++++++++++++--------------
>>>  src/mesa/drivers/common/meta.h |  1 -
>>>  2 files changed, 20 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
>>> index ac27abb..92bc185 100644
>>> --- a/src/mesa/drivers/common/meta.c
>>> +++ b/src/mesa/drivers/common/meta.c
>>> @@ -577,11 +577,15 @@ _mesa_meta_begin(struct gl_context *ctx, GLbitfield state)
>>>        }
>>>
>>>        if (ctx->Extensions.ARB_separate_shader_objects) {
>>> -         /* Warning it must be done before _mesa_UseProgram call */
>>> -         _mesa_reference_pipeline_object(ctx, &save->_Shader, ctx->_Shader);
>>> -         _mesa_reference_pipeline_object(ctx, &save->Pipeline,
>>> -                                         ctx->Pipeline.Current);
>>> -         _mesa_BindProgramPipeline(0);
>>> +         /* Warning: This must be done before saving the current programs.
>>> +          * Otherwise the programs attached to the pipeline will be saved
>>> +          * instead of the programs attached to the default pipeline.
>>> +          */
>>> +         if (ctx->Pipeline.Current != ctx->Pipeline.Default) {
>>> +            _mesa_reference_pipeline_object(ctx, &save->Pipeline,
>>> +                                            ctx->Pipeline.Current);
>>> +            _mesa_BindProgramPipeline(0);
>>> +         }
>>>        }
>>>
>>>        for (i = 0; i < MESA_SHADER_STAGES; i++) {
>>> @@ -929,14 +933,6 @@ _mesa_meta_end(struct gl_context *ctx)
>>>                            save->ATIFragmentShaderEnabled);
>>>        }
>>>
>>> -      /* Warning it must be done before _mesa_use_shader_program call */
>>> -      if (ctx->Extensions.ARB_separate_shader_objects) {
>>> -         _mesa_reference_pipeline_object(ctx, &ctx->_Shader, save->_Shader);
>>> -         _mesa_reference_pipeline_object(ctx, &ctx->Pipeline.Current,
>>> -                                         save->Pipeline);
>>> -         _mesa_reference_pipeline_object(ctx, &save->Pipeline, NULL);
>>> -      }
>>> -
>>>        if (ctx->Extensions.ARB_vertex_shader) {
>>>          _mesa_use_shader_program(ctx, GL_VERTEX_SHADER,
>>>                                    save->Shader[MESA_SHADER_VERTEX],
>>> @@ -956,10 +952,20 @@ _mesa_meta_end(struct gl_context *ctx)
>>>        _mesa_reference_shader_program(ctx, &ctx->_Shader->ActiveProgram,
>>>                                      save->ActiveShader);
>>>
>>> +      /* Warning: This must be done after _mesa_use_shader_program call.
>>> +       * Otherwise the programs will be restored to the pipeline object
>>> +       * instead of to the default pipeline.
>>> +       */
>>> +      if (save->Pipeline) {
>>> +         assert(ctx->Extensions.ARB_separate_shader_objects);
>>> +         _mesa_bind_pipeline(ctx, save->Pipeline);
>> This issue does not appear to be fixed
>>
>>   http://lists.freedesktop.org/archives/mesa-dev/2014-April/057999.html
>>
>> The attached change to piglit triggers it.
>
> What should the result have been with that patch?  I tried that with my
> current sso6 branch, and
> arb_separate_shader_objects-rendezvous_by_location passes with or
> without this patch.
The patch should not affect the result, but I got this error after applying it

$ ./bin/arb_separate_shader_object-rendezvous_by_location -auto
Probe color at (0,0)
  Expected: 0.000000 1.000000 0.000000 1.000000
  Observed: 1.000000 1.000000 1.000000 1.000000
PIGLIT: {'result': 'fail' }

I am also on sso6 (except that I merged it to master).

>
>>> +
>>> +         _mesa_reference_pipeline_object(ctx, &save->Pipeline, NULL);
>>> +      }
>>> +
>>>        for (i = 0; i < MESA_SHADER_STAGES; i++)
>>>           _mesa_reference_shader_program(ctx, &save->Shader[i], NULL);
>>>        _mesa_reference_shader_program(ctx, &save->ActiveShader, NULL);
>>> -      _mesa_reference_pipeline_object(ctx, &save->_Shader, NULL);
>>>     }
>>>
>>>     if (state & MESA_META_STENCIL_TEST) {
>>> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
>>> index fde4f9a..0a34792 100644
>>> --- a/src/mesa/drivers/common/meta.h
>>> +++ b/src/mesa/drivers/common/meta.h
>>> @@ -121,7 +121,6 @@ struct save_state
>>>     GLboolean ATIFragmentShaderEnabled;
>>>     struct gl_shader_program *Shader[MESA_SHADER_STAGES];
>>>     struct gl_shader_program *ActiveShader;
>>> -   struct gl_pipeline_object   *_Shader;
>>>     struct gl_pipeline_object   *Pipeline;
>>>
>>>     /** MESA_META_STENCIL_TEST */
>>> --
>>> 1.8.1.4
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 
olv at LunarG.com