[Mesa-dev] [PATCH 02/12] sso: Add pipeline container/state
Brian Paul
brianp at vmware.com
Sat Apr 6 07:44:51 PDT 2013
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);
More information about the mesa-dev
mailing list