[Mesa-dev] [PATCH 05/12] sso: replace Shader binding point with _Shader
gregory hainaut
gregory.hainaut at gmail.com
Wed Apr 10 23:45:20 PDT 2013
On Sat, 06 Apr 2013 08:52:35 -0600
Brian Paul <brianp at vmware.com> wrote:
> One more comment for now below. I may not get to review the rest for
> a few days.
>
> -Brian
Thanks very much for the review. I redid properly my patch and rebase
my work on a more recent mesa version. I want to do a piglit non regression test
first and double check any typo. Then I will probabaly post the V2 friday
except if you prefer to finish the first review first.
FYI, I choose to keep the shorter "Current" so we got the basic "Pipeline.Current"
Cheers,
Gregory
>
> 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 field
> would 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;
> > }
> >
>
More information about the mesa-dev
mailing list