Mesa (master): meta: Fix saving the program pipeline state

Ian Romanick idr at kemper.freedesktop.org
Fri May 2 14:31:43 UTC 2014


Module: Mesa
Branch: master
Commit: ca21cffebd063354291d561eadc2ded8795a5333
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=ca21cffebd063354291d561eadc2ded8795a5333

Author: Ian Romanick <ian.d.romanick at intel.com>
Date:   Tue Mar 25 18:34:31 2014 -0700

meta: Fix saving the program pipeline state

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.

v3: Fairly significant re-write.  I think it's much cleaner now, and it
avoids a bug with some meta ops that use shaders (reported by Chia-I).

v4: Check Pipeline.Current against NULL instead of Pipeline.Default.
Suggested by Chia-I.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-by: Chia-I Wu <olv at lunarg.com>

---

 src/mesa/drivers/common/meta.c |   86 +++++++++++++++++++++++++---------------
 src/mesa/drivers/common/meta.h |    1 -
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index ab86f9c..d391059 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -577,19 +577,21 @@ _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);
+         if (ctx->Pipeline.Current) {
+            _mesa_reference_pipeline_object(ctx, &save->Pipeline,
+                                            ctx->Pipeline.Current);
+            _mesa_BindProgramPipeline(0);
       }
 
-      for (i = 0; i < MESA_SHADER_STAGES; i++) {
+      /* Save the shader state from ctx->Shader (instead of ctx->_Shader) so
+       * that we don't have to worry about the current pipeline state.
+       */
+      for (i = 0; i <= MESA_SHADER_FRAGMENT; i++) {
          _mesa_reference_shader_program(ctx, &save->Shader[i],
-                                     ctx->_Shader->CurrentProgram[i]);
+                                        ctx->Shader.CurrentProgram[i]);
       }
       _mesa_reference_shader_program(ctx, &save->ActiveShader,
-                                     ctx->_Shader->ActiveProgram);
+                                     ctx->Shader.ActiveProgram);
 
       _mesa_UseProgram(0);
    }
@@ -908,6 +910,14 @@ _mesa_meta_end(struct gl_context *ctx)
    }
 
    if (state & MESA_META_SHADER) {
+      static const GLenum targets[] = {
+         GL_VERTEX_SHADER,
+         GL_GEOMETRY_SHADER,
+         GL_FRAGMENT_SHADER,
+      };
+
+      bool any_shader;
+
       if (ctx->Extensions.ARB_vertex_program) {
          _mesa_set_enable(ctx, GL_VERTEX_PROGRAM_ARB,
                           save->VertexProgramEnabled);
@@ -929,37 +939,47 @@ _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);
-      }
+      any_shader = false;
+      for (i = 0; i <= MESA_SHADER_FRAGMENT; i++) {
+         /* It is safe to call _mesa_use_shader_program even if the extension
+          * necessary for that program state is not supported.  In that case,
+          * the saved program object must be NULL and the currently bound
+          * program object must be NULL.  _mesa_use_shader_program is a no-op
+          * in that case.
+          */
+         _mesa_use_shader_program(ctx, targets[i],
+                                  save->Shader[i],
+                                  &ctx->Shader);
+
+         /* Do this *before* killing the reference. :)
+          */
+         if (save->Shader[i] != NULL)
+            any_shader = true;
 
-      if (ctx->Extensions.ARB_vertex_shader) {
-	 _mesa_use_shader_program(ctx, GL_VERTEX_SHADER,
-                                  save->Shader[MESA_SHADER_VERTEX],
-                                  ctx->_Shader);
+         _mesa_reference_shader_program(ctx, &save->Shader[i], NULL);
       }
 
-      if (_mesa_has_geometry_shaders(ctx))
-	 _mesa_use_shader_program(ctx, GL_GEOMETRY_SHADER_ARB,
-                                  save->Shader[MESA_SHADER_GEOMETRY],
-                                  ctx->_Shader);
+      _mesa_reference_shader_program(ctx, &ctx->Shader.ActiveProgram,
+                                     save->ActiveShader);
+      _mesa_reference_shader_program(ctx, &save->ActiveShader, NULL);
 
-      if (ctx->Extensions.ARB_fragment_shader)
-	 _mesa_use_shader_program(ctx, GL_FRAGMENT_SHADER,
-                                  save->Shader[MESA_SHADER_FRAGMENT],
-                                  ctx->_Shader);
+      /* If there were any stages set with programs, use ctx->Shader as the
+       * current shader state.  Otherwise, use Pipeline.Default.  The pipeline
+       * hasn't been restored yet, and that may modify ctx->_Shader further.
+       */
+      if (any_shader)
+         _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
+                                         &ctx->Shader);
+      else
+         _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
+                                         ctx->Pipeline.Default);
 
-      _mesa_reference_shader_program(ctx, &ctx->_Shader->ActiveProgram,
-				     save->ActiveShader);
+      if (save->Pipeline) {
+         assert(ctx->Extensions.ARB_separate_shader_objects);
+         _mesa_bind_pipeline(ctx, save->Pipeline);
 
-      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);
+         _mesa_reference_pipeline_object(ctx, &save->Pipeline, 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 */




More information about the mesa-commit mailing list