Re: [Mesa-dev] [PATCH 05/12] sso: replace Shader binding point with _Shader

Brian Paul Sat, 06 Apr 2013 07:52:43 -0700

One more comment for now below. I may not get to review the rest fora few days.


-Brian


On 04/05/2013 03:27 PM, gregory wrote:

To avoid NULL pointer check a default pipeline object is installed in _Shader 
when no
program is current

The spec say that UseProgram/UseShaderProgramEXT/ActiveProgramEXT got an higher
priority over the pipeline object. When default program is uninstall, the 
pipeline is
used if any was bound.

Note: A careful rename need to be done now...
---
  src/mesa/main/mtypes.h      |    4 ++
  src/mesa/main/pipelineobj.c |    8 ++++
  src/mesa/main/shaderapi.c   |   91 +++++++++++++++++++++++++++++++++++++++++--
  3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 2445574..dc54f3d 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2433,6 +2433,9 @@ struct gl_pipeline_shader_state
     /** Currently bound pipeline object. See _mesa_BindProgramPipeline() */
     struct gl_shader_state *PipelineObj;

+   /* Default Object to ensure that _Shader is never NULL */
+   struct gl_shader_state *Default;
+
     /** Pipeline objects */
     struct _mesa_HashTable *Objects;
  };
@@ -3541,6 +3544,7 @@ struct gl_context

     struct gl_pipeline_shader_state Pipeline; /**<  GLSL pipeline shader 
object state */
     struct gl_shader_state Shader; /**<  GLSL shader object state */
+   struct gl_shader_state *_Shader; /**<  Points to ::Shader or 
::Pipeline.PipelineObj or ::Pipeline.Default */

If gl_shader_state gets renamed to gl_pipeline_object then this fieldwould probably be better name CurrentPipeline, or similar.

     struct gl_shader_compiler_options ShaderCompilerOptions[MESA_SHADER_TYPES];

     struct gl_query_state Query;  /**<  occlusion, timer queries */
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 7a56c67..c805cdf 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -97,6 +97,10 @@ _mesa_init_pipeline(struct gl_context *ctx)
     ctx->Pipeline.Objects = _mesa_NewHashTable();

     ctx->Pipeline.PipelineObj = NULL;
+
+   /* Install a default Pipeline */
+   ctx->Pipeline.Default = _mesa_new_pipeline_object(ctx, 0);
+   _mesa_reference_pipeline_object(ctx,&ctx->_Shader, ctx->Pipeline.Default);
  }


@@ -120,6 +124,10 @@ _mesa_free_pipeline_data(struct gl_context *ctx)
  {
     _mesa_HashDeleteAll(ctx->Pipeline.Objects, delete_pipelineobj_cb, ctx);
     _mesa_DeleteHashTable(ctx->Pipeline.Objects);
+
+   _mesa_reference_pipeline_object(ctx,&ctx->_Shader, NULL);
+   _mesa_delete_pipeline_object(ctx, ctx->Pipeline.Default);
+
  }

  /**
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index a86a429..1092287 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -43,6 +43,7 @@
  #include "main/hash.h"
  #include "main/mfeatures.h"
  #include "main/mtypes.h"
+#include "main/pipelineobj.h"
  #include "main/shaderapi.h"
  #include "main/shaderobj.h"
  #include "main/transformfeedback.h"
@@ -138,6 +139,8 @@ _mesa_free_shader_state(struct gl_context *ctx)
     _mesa_reference_shader_program(ctx,&ctx->Shader.ActiveProgram, NULL);

     /* Extended for ARB_separate_shader_objects */
+   _mesa_reference_pipeline_object(ctx,&ctx->_Shader, NULL);
+
     assert(ctx->Shader.RefCount == 1);
     _glthread_DESTROY_MUTEX(ctx->Shader.Mutex);
  }
@@ -1453,7 +1456,29 @@ _mesa_UseProgram(GLhandleARB program)
        shProg = NULL;
     }

-   _mesa_use_program(ctx, shProg);
+   /*
+    * The executable code for an individual shader stage is taken from the
+    * current program for that stage.  If there is a current program object
+    * for any shader stage or for uniform updates established by UseProgram,
+    * UseShaderProgramEXT, or ActiveProgramEXT, the current program for that
+    * stage (if any) is considered current.  Otherwise, if there is a bound
+    * program pipeline object ...
+    */
+   if (program) {
+      /* Attach shader state to the binding point */
+      _mesa_reference_pipeline_object(ctx,&ctx->_Shader,&ctx->Shader);
+      /* Update the program */
+      _mesa_use_program(ctx, shProg);
+   } else {
+      /* Must be done first: detach the progam */
+      _mesa_use_program(ctx, shProg);
+      /* Unattach shader_state binding point */
+      _mesa_reference_pipeline_object(ctx,&ctx->_Shader, 
ctx->Pipeline.Default);
+      /* If a pipeline was bound, rebind it */
+      if (ctx->Pipeline.PipelineObj) {
+         _mesa_BindProgramPipeline (ctx->Pipeline.PipelineObj->Name);
+      }
+   }
  }


@@ -1769,7 +1794,37 @@ _mesa_UseShaderProgramEXT(GLenum type, GLuint program)
        }
     }

-   _mesa_use_shader_program(ctx, type, shProg);
+   /*
+    * The executable code for an individual shader stage is taken from the
+    * current program for that stage.  If there is a current program object
+    * for any shader stage or for uniform updates established by UseProgram,
+    * UseShaderProgramEXT, or ActiveProgramEXT, the current program for that
+    * stage (if any) is considered current.  Otherwise, if there is a bound
+    * program pipeline object ...
+    */
+   if (program) {
+      /* Attach shader state to the binding point */
+      _mesa_reference_pipeline_object(ctx,&ctx->_Shader,&ctx->Shader);
+      /* Update the program */
+      _mesa_use_shader_program(ctx, type, shProg,&ctx->Shader);
+   } else {
+      /* Must be done first: detach the progam */
+      _mesa_use_shader_program(ctx, type, shProg,&ctx->Shader);
+
+      /* Nothing remains current */
+      if (  ctx->Shader.CurrentVertexProgram   == NULL&&
+            ctx->Shader.CurrentGeometryProgram == NULL&&
+            ctx->Shader.CurrentFragmentProgram == NULL&&
+            ctx->Shader.ActiveProgram          == NULL) {
+
+         /* Unattach shader_state binding point */
+         _mesa_reference_pipeline_object(ctx,&ctx->_Shader, 
ctx->Pipeline.Default);
+         /* If a pipeline was bound, rebind it */
+         if (ctx->Pipeline.PipelineObj) {
+            _mesa_BindProgramPipeline (ctx->Pipeline.PipelineObj->Name);
+         }
+      }
+   }
  }


@@ -1784,7 +1839,37 @@ _mesa_ActiveProgramEXT(GLuint program)
        ? _mesa_lookup_shader_program_err(ctx, program, "glActiveProgramEXT")
        : NULL;

-   _mesa_active_program(ctx, shProg, "glActiveProgramEXT");
+   /*
+    * The executable code for an individual shader stage is taken from the
+    * current program for that stage.  If there is a current program object
+    * for any shader stage or for uniform updates established by UseProgram,
+    * UseShaderProgramEXT, or ActiveProgramEXT, the current program for that
+    * stage (if any) is considered current.  Otherwise, if there is a bound
+    * program pipeline object ...
+    */
+   if(shProg != NULL) {
+      /* Attach shader state to the binding point */
+      _mesa_reference_pipeline_object(ctx,&ctx->_Shader,&ctx->Shader);
+      _mesa_active_program(ctx, shProg, "glActiveProgramEXT");
+   } else {
+      /* Must be done first: unset the current active progam */
+      _mesa_active_program(ctx, shProg, "glActiveProgramEXT");
+
+      /* Nothing remains current */
+      if (  ctx->Shader.CurrentVertexProgram   == NULL&&
+            ctx->Shader.CurrentGeometryProgram == NULL&&
+            ctx->Shader.CurrentFragmentProgram == NULL&&
+            ctx->Shader.ActiveProgram          == NULL) {
+
+         /* Unattach shader_state binding point */
+         _mesa_reference_pipeline_object(ctx,&ctx->_Shader, 
ctx->Pipeline.Default);
+         /* If a pipeline was bound, rebind it */
+         if (ctx->Pipeline.PipelineObj) {
+            _mesa_BindProgramPipeline (ctx->Pipeline.PipelineObj->Name);
+         }
+      }
+   }
+
     return;
  }


_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] sso: replace Shader binding point with _Shader

Reply via email to