From b2bddaf7a000bf9830a7947b18d8e31fb25353ae Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Fri, 28 Jun 2013 14:33:54 -0700 Subject: [PATCH] mesa/sso: replace Shader binding point with _Shader 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... V2: formating improvement V3 (idr): * Build fix. The original patch added calls to _mesa_use_shader_program with 4 parameters, but the fourth parameter isn't added to that function until a much later patch. Just drop that parameter for now. * Trivial reformatting. * Updated comment of gl_context::_Shader v4 (idr): Reformat spec quotations to look like spec quotations. Update comments describing what gl_context::_Shader can point to. Bot suggested by Eric. Reviewed-by: Ian Romanick Reviewed-by: Eric Anholt --- src/mesa/main/mtypes.h | 23 +++++++++++ src/mesa/main/pipelineobj.c | 8 ++++ src/mesa/main/shaderapi.c | 97 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c6d90c5..f9317a5 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2817,6 +2817,9 @@ struct gl_pipeline_shader_state /** Currently bound pipeline object. See _mesa_BindProgramPipeline() */ struct gl_pipeline_object *Current; + /* Default Object to ensure that _Shader is never NULL */ + struct gl_pipeline_object *Default; + /** Pipeline objects */ struct _mesa_HashTable *Objects; }; @@ -4123,6 +4126,26 @@ struct gl_context struct gl_pipeline_shader_state Pipeline; /**< GLSL pipeline shader object state */ struct gl_pipeline_object Shader; /**< GLSL shader object state */ + + /** + * Current active shader pipeline state + * + * Almost all internal users want ::_Shader instead of ::Shader. The + * exceptions are bits of legacy GLSL API that do not know about separate + * shader objects. + * + * If a program is active via \c glUseProgram, this will point to + * \c ::Shader. + * + * If a program pipeline is active via \c glBindProgramPipeline, this will + * point to \c ::Pipeline.Current. + * + * If neither a program nor a program pipeline is active, this will point to + * \c ::Pipeline.Default. This ensures that \c ::_Shader will never be + * \c NULL. + */ + struct gl_pipeline_object *_Shader; + struct gl_shader_compiler_options ShaderCompilerOptions[MESA_SHADER_STAGES]; struct gl_query_state Query; /**< occlusion, timer queries */ diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c index 27012df..849c781 100644 --- a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c @@ -94,6 +94,10 @@ _mesa_init_pipeline(struct gl_context *ctx) ctx->Pipeline.Objects = _mesa_NewHashTable(); ctx->Pipeline.Current = NULL; + + /* Install a default Pipeline */ + ctx->Pipeline.Default = _mesa_new_pipeline_object(ctx, 0); + _mesa_reference_pipeline_object(ctx, &ctx->_Shader, ctx->Pipeline.Default); } @@ -117,6 +121,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 5060cbb..f0d345a 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -44,6 +44,7 @@ #include "main/hash.h" #include "main/hash_table.h" #include "main/mtypes.h" +#include "main/pipelineobj.h" #include "main/shaderapi.h" #include "main/shaderobj.h" #include "main/transformfeedback.h" @@ -144,6 +145,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); mtx_destroy(&ctx->Shader.Mutex); } @@ -1541,7 +1544,31 @@ _mesa_UseProgram(GLhandleARB program) shProg = NULL; } - _mesa_use_program(ctx, shProg); + /* The "Dependencies on EXT_separate_shader_objects" section of the + * ARB_separate_shader_object spec says: + * + * "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.Current) { + _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name); + } + } } @@ -1815,7 +1842,41 @@ _mesa_UseShaderProgramEXT(GLenum type, GLuint program) } } - _mesa_use_shader_program(ctx, type, shProg); + /* The "Dependencies on EXT_separate_shader_objects" section of the + * ARB_separate_shader_object spec says: + * + * "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); + } else { + /* Must be done first: detach the progam */ + _mesa_use_shader_program(ctx, type, shProg); + + /* Nothing remains current */ + if (!ctx->Shader.CurrentVertexProgram && + !ctx->Shader.CurrentGeometryProgram && + !ctx->Shader.CurrentFragmentProgram && + !ctx->Shader.ActiveProgram) { + + /* 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.Current) { + _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name); + } + } + } } @@ -1830,7 +1891,37 @@ _mesa_ActiveProgramEXT(GLuint program) ? _mesa_lookup_shader_program_err(ctx, program, "glActiveProgramEXT") : NULL; - _mesa_active_program(ctx, shProg, "glActiveProgramEXT"); + /* The "Dependencies on EXT_separate_shader_objects" section of the + * ARB_separate_shader_object spec says: + * + * "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 && !ctx->Shader.CurrentGeometryProgram && + !ctx->Shader.CurrentFragmentProgram && !ctx->Shader.ActiveProgram) { + + /* 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.Current) { + _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name); + } + } + } + return; } -- 2.7.4