Re: [Mesa-dev] [PATCH 05/21] meta: Fix saving the program pipeline state
On 04/30/2014 06:59 PM, Chia-I Wu wrote:
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.00 1.00 0.00 1.00
Observed: 1.00 1.00 1.00 1.00
PIGLIT: {'result': 'fail' }
I am also on sso6 (except that I merged it to master).
I wasn't hitting because I wasn't hitting meta for the clear path. With
meta blocked out, I get the same failure. I should have a fix out
momentarilly... just one last piglit run...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/21] meta: Fix saving the program pipeline state
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.
+
+ _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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/21] meta: Fix saving the program pipeline state
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.00 1.00 0.00 1.00
Observed: 1.00 1.00 1.00 1.00
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
