On Thu, May 1, 2014 at 12:11 AM, Ian Romanick <i...@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 <i...@freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.roman...@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.roman...@intel.com>
>>> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>>> Reviewed-by: Eric Anholt <e...@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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
--
o...@lunarg.com
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
