A bunch of other nits below...

On 04/05/2013 03:26 PM, gregory wrote:
* Extend gl_shader_state as pipeline object state
* Add a new container gl_pipeline_shader_state that contains
   binding point of the previous object
* Update mesa init/free shader state due to the extension of
   the attibute
* Add an init/free pipeline function for the context
* Implement GenProgramPipeline/DeleteProgramPipeline/IsProgramPipeline.
  I based my work on the VAO code.
---
  src/mesa/main/context.c     |    3 +
  src/mesa/main/mtypes.h      |   26 +++++
  src/mesa/main/pipelineobj.c |  233 ++++++++++++++++++++++++++++++++++++++++++-
  src/mesa/main/pipelineobj.h |   25 +++++
  src/mesa/main/shaderapi.c   |   10 +-
  src/mesa/main/shaderapi.h   |    3 +
  6 files changed, 298 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 0539934..d089827 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -106,6 +106,7 @@
  #include "macros.h"
  #include "matrix.h"
  #include "multisample.h"
+#include "pipelineobj.h"
  #include "pixel.h"
  #include "pixelstore.h"
  #include "points.h"
@@ -762,6 +763,7 @@ init_attrib_groups(struct gl_context *ctx)
     _mesa_init_lighting( ctx );
     _mesa_init_matrix( ctx );
     _mesa_init_multisample( ctx );
+   _mesa_init_pipeline( ctx );
     _mesa_init_pixel( ctx );
     _mesa_init_pixelstore( ctx );
     _mesa_init_point( ctx );
@@ -1167,6 +1169,7 @@ _mesa_free_context_data( struct gl_context *ctx )
     _mesa_free_texture_data( ctx );
     _mesa_free_matrix_data( ctx );
     _mesa_free_viewport_data( ctx );
+   _mesa_free_pipeline_data(ctx);
     _mesa_free_program_data(ctx);
     _mesa_free_shader_state(ctx);
     _mesa_free_queryobj_data(ctx);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index bacfc85..7c147d2 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2380,9 +2380,19 @@ struct gl_shader_program

  /**
   * Context state for GLSL vertex/fragment shaders.
+ * Extended to support pipeline object
   */
  struct gl_shader_state
  {
+   /** Name of the pipeline object as received from glGenProgramPipelines.
+    * It would be 0 for shaders without separate shader objects.
+    */
+   GLuint Name;
+
+   GLint RefCount;
+
+   _glthread_Mutex Mutex;
+
     /**
      * Programs used for rendering
      *

Previously, gl_shader_state was just used as a field in gl_context to encapsulate shader bindings. Now it looks like it's going to be a new type of object (which there may be many). Below, there's lots of new code like:

static inline struct gl_shader_state *
lookup_pipeline_object(struct gl_context *ctx, GLuint id)

So the name should probably be changed from gl_shader_state to gl_pipeline_object.



@@ -2404,8 +2414,23 @@ struct gl_shader_state
     struct gl_shader_program *ActiveProgram;

     GLbitfield Flags;                    /**<  Mask of GLSL_x flags */
+
+   GLboolean ValidationStatus;          /**<  Pipeline Validation status */

How about just "Validated"?



+
+   GLboolean EverBound;                 /**<  Has the pipeline object been 
created */
  };

+/**
+ * Context state for GLSL pipeline shaders.
+ */
+struct gl_pipeline_shader_state
+{
+   /** Currently bound pipeline object. See _mesa_BindProgramPipeline() */
+   struct gl_shader_state *PipelineObj;
+
+   /** Pipeline objects */
+   struct _mesa_HashTable *Objects;
+};

  /**
   * Compiler options for a single GLSL shaders type
@@ -3509,6 +3534,7 @@ struct gl_context
     struct gl_geometry_program_state GeometryProgram;
     struct gl_ati_fragment_shader_state ATIFragmentShader;

+   struct gl_pipeline_shader_state Pipeline; /**<  GLSL pipeline shader object 
state */
     struct gl_shader_state Shader; /**<  GLSL shader object state */
     struct gl_shader_compiler_options ShaderCompilerOptions[MESA_SHADER_TYPES];

diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index ae5e2fb..e50416c 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -54,6 +54,170 @@
  #include "../glsl/glsl_parser_extras.h"
  #include "../glsl/ir_uniform.h"

+/**
+ * Delete a pipeline object.
+ */
+void
+_mesa_delete_pipeline_object( struct gl_context *ctx, struct gl_shader_state 
*obj )

You can remove the spaces after ( and before ). Many more instances of this below.


+{
+   (void) ctx;

That line's not appropriate since ctx is used.


+   _mesa_reference_shader_program(ctx,&obj->_CurrentFragmentProgram, NULL);
+   _mesa_reference_shader_program(ctx,&obj->CurrentFragmentProgram, NULL);
+   _mesa_reference_shader_program(ctx,&obj->CurrentVertexProgram, NULL);
+   _mesa_reference_shader_program(ctx,&obj->CurrentGeometryProgram, NULL);
+   _mesa_reference_shader_program(ctx,&obj->ActiveProgram, NULL);
+   _glthread_DESTROY_MUTEX(obj->Mutex);
+   free(obj);
+}
+
+/**
+ * Allocate and initialize a new pipeline object.
+ *

Can remove blank comment line.



+ */
+static struct gl_shader_state *
+_mesa_new_pipeline_object( struct gl_context *ctx, GLuint name )
+{
+   struct gl_shader_state *obj = CALLOC_STRUCT(gl_shader_state);
+   if (obj) {
+      obj->Name = name;
+      _glthread_INIT_MUTEX(obj->Mutex);
+      obj->RefCount = 1;
+      obj->Flags = get_shader_flags();
+   }
+
+   return obj;
+}
+
+/**
+ * Initialize pipeline object state for given context.
+ */
+void
+_mesa_init_pipeline(struct gl_context *ctx)
+{
+   ctx->Pipeline.Objects = _mesa_NewHashTable();
+
+   ctx->Pipeline.PipelineObj = NULL;
+}
+
+
+/**
+ * Callback for deleting a pipeline object.  Called by _mesa_HashDeleteAll().
+ */
+static void
+delete_pipelineobj_cb(GLuint id, void *data, void *userData)
+{
+   struct gl_shader_state *PipelineObj = (struct gl_shader_state *) data;
+   struct gl_context *ctx = (struct gl_context *) userData;
+   _mesa_delete_pipeline_object(ctx, PipelineObj);
+}
+
+
+/**
+ * Free pipeline state for given context.
+ */
+void
+_mesa_free_pipeline_data(struct gl_context *ctx)
+{
+   _mesa_HashDeleteAll(ctx->Pipeline.Objects, delete_pipelineobj_cb, ctx);
+   _mesa_DeleteHashTable(ctx->Pipeline.Objects);
+}
+
+/**
+ * Look up the pipeline object for the given ID.
+ *
+ * \returns
+ * Either a pointer to the pipeline object with the specified ID or \c NULL for
+ * a non-existent ID.  The spec defines ID 0 as being technically
+ * non-existent.
+ */
+static inline struct gl_shader_state *
+lookup_pipeline_object(struct gl_context *ctx, GLuint id)
+{
+   if (id == 0)
+      return NULL;
+   else
+      return (struct gl_shader_state *)
+         _mesa_HashLookup(ctx->Pipeline.Objects, id);
+}
+
+/**
+ * Add the given pipeline object to the pipeline object pool.
+ */
+static void
+save_pipeline_object( struct gl_context *ctx, struct gl_shader_state *obj )
+{
+   if (obj->Name>  0) {
+      _mesa_HashInsert(ctx->Pipeline.Objects, obj->Name, obj);
+   }
+}
+
+/**
+ * Remove the given pipeline object from the pipeline object pool.
+ * Do not deallocate the pipeline object though.
+ */
+static void
+remove_pipeline_object( struct gl_context *ctx, struct gl_shader_state *obj )
+{
+   if (obj->Name>  0) {
+      _mesa_HashRemove(ctx->Pipeline.Objects, obj->Name);
+   }
+}
+
+/**
+ * Set ptr to pipelineObj w/ reference counting.
+ * Note: this should only be called from the _mesa_reference_pipeline_object()
+ * inline function.
+ */
+void
+_mesa_reference_pipeline_object_(struct gl_context *ctx,
+                                 struct gl_shader_state **ptr,
+                                 struct gl_shader_state *PipelineObj)

We don't capitialize the first letter of function parameter names.

s/PipelineObj/obj/


+{
+   assert(*ptr != PipelineObj);
+
+   if (*ptr) {
+      /* Unreference the old pipeline object */
+      GLboolean deleteFlag = GL_FALSE;
+      struct gl_shader_state *oldObj = *ptr;
+
+      _glthread_LOCK_MUTEX(oldObj->Mutex);
+      ASSERT(oldObj->RefCount>  0);
+      oldObj->RefCount--;
+#if 0
+      printf("PipelineObj %p %d DECR to %d\n",
+            (void *) oldObj, oldObj->Name, oldObj->RefCount);
+#endif
+      deleteFlag = (oldObj->RefCount == 0);
+      _glthread_UNLOCK_MUTEX(oldObj->Mutex);
+
+      if (deleteFlag) {
+         _mesa_delete_pipeline_object(ctx, oldObj);
+      }
+
+      *ptr = NULL;
+   }
+   ASSERT(!*ptr);
+
+   if (PipelineObj) {
+      /* reference new pipeline object */
+      _glthread_LOCK_MUTEX(PipelineObj->Mutex);
+      if (PipelineObj->RefCount == 0) {
+         /* this pipeline's being deleted (look just above) */
+         /* Not sure this can every really happen.  Warn if it does. */
+         _mesa_problem(NULL, "referencing deleted pipeline object");
+         *ptr = NULL;
+      }
+      else {
+         PipelineObj->RefCount++;
+#if 0
+         printf("PipelineObj %p %d INCR to %d\n",
+               (void *) PipelineObj, PipelineObj->Name, PipelineObj->RefCount);
+#endif
+         *ptr = PipelineObj;
+      }
+      _glthread_UNLOCK_MUTEX(PipelineObj->Mutex);
+   }
+}

  /**
   * Bound program to severals stages of the pipeline
@@ -88,6 +252,37 @@ _mesa_BindProgramPipeline (GLuint pipeline)
  void GLAPIENTRY
  _mesa_DeleteProgramPipelines (GLsizei n, const GLuint *pipelines)
  {
+   GET_CURRENT_CONTEXT(ctx);
+   GLsizei i;
+
+   if (n<  0) {
+      _mesa_error(ctx, GL_INVALID_VALUE, "glDeleteProgramPipelines(n<0)");
+      return;
+   }
+
+   for (i = 0; i<  n; i++) {
+      struct gl_shader_state *obj = lookup_pipeline_object(ctx, pipelines[i]);
+
+      if ( obj != NULL ) {

Remove spaces: "if (obj != NULL)", or just "if (obj) {"


+         ASSERT( obj->Name == pipelines[i] );
+
+         /* If the pipeline object is currently bound, the spec says "If an 
object that is
+          * currently bound is deleted, the binding for that object
+          * reverts to zero and no program pipeline object becomes current."
+          */
+         if ( obj == ctx->Pipeline.PipelineObj ) {
+            _mesa_BindProgramPipeline(0);
+         }
+
+         /* The ID is immediately freed for re-use */
+         remove_pipeline_object(ctx, obj);
+
+         /* Unreference the pipeline object.
+          * If refcount hits zero, the object will be deleted.
+          */
+         _mesa_reference_pipeline_object(ctx,&obj, NULL);
+      }
+   }
  }

  /**
@@ -98,6 +293,36 @@ _mesa_DeleteProgramPipelines (GLsizei n, const GLuint 
*pipelines)
  void GLAPIENTRY
  _mesa_GenProgramPipelines (GLsizei n, GLuint *pipelines)
  {
+   GET_CURRENT_CONTEXT(ctx);
+
+   GLuint first;
+   GLint i;
+
+   if (n<  0) {
+      _mesa_error(ctx, GL_INVALID_VALUE, "glGenProgramPipelines(n<0)");
+      return;
+   }
+
+   if (!pipelines) {
+      return;
+   }
+
+   first = _mesa_HashFindFreeKeyBlock(ctx->Pipeline.Objects, n);
+
+   for (i = 0; i<  n; i++) {
+      struct gl_shader_state *obj = NULL;

you don't need the "= NULL" initialization here.


+      GLuint name = first + i;
+
+      obj = _mesa_new_pipeline_object(ctx, name);
+      if (!obj) {
+         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGenProgramPipelines");
+         return;
+      }
+
+      save_pipeline_object(ctx, obj);
+      pipelines[i] = first + i;
+   }
+
  }

  /**
@@ -110,7 +335,13 @@ _mesa_GenProgramPipelines (GLsizei n, GLuint *pipelines)
  GLboolean GLAPIENTRY
  _mesa_IsProgramPipeline (GLuint pipeline)
  {
-   return GL_FALSE;
+   GET_CURRENT_CONTEXT(ctx);
+
+   struct gl_shader_state *obj = lookup_pipeline_object(ctx, pipeline);
+   if (obj == NULL)
+      return GL_FALSE;
+
+   return obj->EverBound;
  }

  /**
diff --git a/src/mesa/main/pipelineobj.h b/src/mesa/main/pipelineobj.h
index 05faadc..b0ee233 100644
--- a/src/mesa/main/pipelineobj.h
+++ b/src/mesa/main/pipelineobj.h
@@ -37,6 +37,31 @@ extern "C" {

  struct _glapi_table;
  struct gl_context;
+struct gl_shader_state;
+
+extern void
+_mesa_delete_pipeline_object( struct gl_context *ctx, struct gl_shader_state 
*obj );
+
+extern void
+_mesa_init_pipeline(struct gl_context *ctx);
+
+extern void
+_mesa_free_pipeline_data(struct gl_context *ctx);
+
+extern void
+_mesa_reference_pipeline_object_(struct gl_context *ctx,
+                                 struct gl_shader_state **ptr,
+                                 struct gl_shader_state *PipelineObj);

s/PipelineObj/obj/


+
+static inline void
+_mesa_reference_pipeline_object(struct gl_context *ctx,
+                                struct gl_shader_state **ptr,
+                                struct gl_shader_state *PipelineObj)
+{
+   if (*ptr != PipelineObj)
+      _mesa_reference_pipeline_object_(ctx, ptr, PipelineObj);
+}
+

  extern void GLAPIENTRY
  _mesa_UseProgramStages (GLuint pipeline, GLbitfield stages, GLuint program);
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 07caf2f..95ad5ea 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -61,7 +61,7 @@
  /**
   * Return mask of GLSL_x flags by examining the MESA_GLSL env var.
   */
-static GLbitfield
+extern GLbitfield

remove 'extern'.


  get_shader_flags(void)

Put _mesa_ prefix on non-static functions.



  {
     GLbitfield flags = 0x0;
@@ -115,6 +115,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
        memcpy(&ctx->ShaderCompilerOptions[sh],&options, sizeof(options));

     ctx->Shader.Flags = get_shader_flags();
+
+   /* Extended for ARB_separate_shader_objects */
+   ctx->Shader.RefCount = 1;
+   _glthread_INIT_MUTEX(ctx->Shader.Mutex);
  }


@@ -132,6 +136,10 @@ _mesa_free_shader_state(struct gl_context *ctx)
     _mesa_reference_shader_program(ctx,&ctx->Shader._CurrentFragmentProgram,
                                  NULL);
     _mesa_reference_shader_program(ctx,&ctx->Shader.ActiveProgram, NULL);
+
+   /* Extended for ARB_separate_shader_objects */
+   assert(ctx->Shader.RefCount == 1);
+   _glthread_DESTROY_MUTEX(ctx->Shader.Mutex);
  }


diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
index e0bd77d..bf69597 100644
--- a/src/mesa/main/shaderapi.h
+++ b/src/mesa/main/shaderapi.h
@@ -39,6 +39,9 @@ struct _glapi_table;
  struct gl_context;
  struct gl_shader_program;

+extern GLbitfield
+get_shader_flags(void);
+
  extern void
  _mesa_copy_string(GLchar *dst, GLsizei maxLength,
                    GLsizei *length, const GLchar *src);

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

Reply via email to