[Mesa-dev] [PATCH] mesa: renumber shader indices according to their placement in pipeline
Jose Fonseca
jfonseca at vmware.com
Mon Jul 1 02:12:37 PDT 2013
----- Original Message -----
> See my explanation in mtypes.h.
>
> v2: don't do this in gallium
Thanks for the update, Marek.
I think that the "These MUST have the same values as gallium's PIPE_SHADER_*" comment in gl_shader_type enum can be removed now.
Otherwise looks good AFAICT (but note I'm not very familiar with several of the components touched here.)
Jose
> ---
> src/glsl/linker.cpp | 16 ++++++++--------
> src/mesa/drivers/dri/i965/brw_shader.cpp | 8 ++------
> src/mesa/main/mtypes.h | 8 ++++++--
> src/mesa/main/shaderobj.h | 4 ++--
> src/mesa/program/ir_to_mesa.cpp | 10 +++-------
> src/mesa/program/program.h | 2 +-
> src/mesa/state_tracker/st_context.c | 5 -----
> src/mesa/state_tracker/st_extensions.c | 12 +++++++-----
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 +++-------
> 9 files changed, 32 insertions(+), 43 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index c168e47..61405ea 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1514,31 +1514,31 @@ static bool
> check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
> {
> static const char *const shader_names[MESA_SHADER_TYPES] = {
> - "vertex", "fragment", "geometry"
> + "vertex", "geometry", "fragment"
> };
>
> const unsigned max_samplers[MESA_SHADER_TYPES] = {
> ctx->Const.VertexProgram.MaxTextureImageUnits,
> - ctx->Const.FragmentProgram.MaxTextureImageUnits,
> - ctx->Const.GeometryProgram.MaxTextureImageUnits
> + ctx->Const.GeometryProgram.MaxTextureImageUnits,
> + ctx->Const.FragmentProgram.MaxTextureImageUnits
> };
>
> const unsigned max_default_uniform_components[MESA_SHADER_TYPES] = {
> ctx->Const.VertexProgram.MaxUniformComponents,
> - ctx->Const.FragmentProgram.MaxUniformComponents,
> - ctx->Const.GeometryProgram.MaxUniformComponents
> + ctx->Const.GeometryProgram.MaxUniformComponents,
> + ctx->Const.FragmentProgram.MaxUniformComponents
> };
>
> const unsigned max_combined_uniform_components[MESA_SHADER_TYPES] = {
> ctx->Const.VertexProgram.MaxCombinedUniformComponents,
> - ctx->Const.FragmentProgram.MaxCombinedUniformComponents,
> - ctx->Const.GeometryProgram.MaxCombinedUniformComponents
> + ctx->Const.GeometryProgram.MaxCombinedUniformComponents,
> + ctx->Const.FragmentProgram.MaxCombinedUniformComponents
> };
>
> const unsigned max_uniform_blocks[MESA_SHADER_TYPES] = {
> ctx->Const.VertexProgram.MaxUniformBlocks,
> - ctx->Const.FragmentProgram.MaxUniformBlocks,
> ctx->Const.GeometryProgram.MaxUniformBlocks,
> + ctx->Const.FragmentProgram.MaxUniformBlocks
> };
>
> for (unsigned i = 0; i < MESA_SHADER_TYPES; i++) {
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 12986cc..584b4c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -117,17 +117,13 @@ brw_link_shader(struct gl_context *ctx, struct
> gl_shader_program *shProg)
> for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
> struct brw_shader *shader =
> (struct brw_shader *)shProg->_LinkedShaders[stage];
> - static const GLenum targets[] = {
> - GL_VERTEX_PROGRAM_ARB,
> - GL_FRAGMENT_PROGRAM_ARB,
> - GL_GEOMETRY_PROGRAM_NV
> - };
>
> if (!shader)
> continue;
>
> struct gl_program *prog =
> - ctx->Driver.NewProgram(ctx, targets[stage], shader->base.Name);
> + ctx->Driver.NewProgram(ctx, _mesa_program_index_to_target(stage),
> + shader->base.Name);
> if (!prog)
> return false;
> prog->Parameters = _mesa_new_parameter_list();
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 1f62e2c..cab619b 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2174,12 +2174,16 @@ struct gl_shader
> /**
> * Shader stages. Note that these will become 5 with tessellation.
> * These MUST have the same values as gallium's PIPE_SHADER_*
> + *
> + * The order must match how shaders are ordered in the pipeline.
> + * The GLSL linker assumes that if i<j, then the j-th shader is
> + * executed later than the i-th shader.
> */
> typedef enum
> {
> MESA_SHADER_VERTEX = 0,
> - MESA_SHADER_FRAGMENT = 1,
> - MESA_SHADER_GEOMETRY = 2,
> + MESA_SHADER_GEOMETRY = 1,
> + MESA_SHADER_FRAGMENT = 2,
> MESA_SHADER_TYPES = 3
> } gl_shader_type;
>
> diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h
> index 5ce85cf..de1c9fc 100644
> --- a/src/mesa/main/shaderobj.h
> +++ b/src/mesa/main/shaderobj.h
> @@ -123,8 +123,8 @@ _mesa_shader_index_to_type(GLuint i)
> {
> static const GLenum enums[MESA_SHADER_TYPES] = {
> GL_VERTEX_SHADER,
> - GL_FRAGMENT_SHADER,
> - GL_GEOMETRY_SHADER ,
> + GL_GEOMETRY_SHADER,
> + GL_FRAGMENT_SHADER
> };
> if (i >= MESA_SHADER_TYPES)
> return 0;
> diff --git a/src/mesa/program/ir_to_mesa.cpp
> b/src/mesa/program/ir_to_mesa.cpp
> index 35a9b84..4af1c82 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3073,12 +3073,6 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
> linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i]);
>
> if (linked_prog) {
> - static const GLenum targets[] = {
> - GL_VERTEX_PROGRAM_ARB,
> - GL_FRAGMENT_PROGRAM_ARB,
> - GL_GEOMETRY_PROGRAM_NV
> - };
> -
> if (i == MESA_SHADER_VERTEX) {
> ((struct gl_vertex_program *)linked_prog)->UsesClipDistance
> = prog->Vert.UsesClipDistance;
> @@ -3086,7 +3080,9 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
>
> _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program,
> linked_prog);
> - if (!ctx->Driver.ProgramStringNotify(ctx, targets[i], linked_prog))
> {
> + if (!ctx->Driver.ProgramStringNotify(ctx,
> +
> _mesa_program_index_to_target(i),
> + linked_prog)) {
> return GL_FALSE;
> }
> }
> diff --git a/src/mesa/program/program.h b/src/mesa/program/program.h
> index ecda59b..f289838 100644
> --- a/src/mesa/program/program.h
> +++ b/src/mesa/program/program.h
> @@ -205,8 +205,8 @@ _mesa_program_index_to_target(GLuint i)
> {
> static const GLenum enums[MESA_SHADER_TYPES] = {
> GL_VERTEX_PROGRAM_ARB,
> - GL_FRAGMENT_PROGRAM_ARB,
> GL_GEOMETRY_PROGRAM_NV,
> + GL_FRAGMENT_PROGRAM_ARB
> };
> if(i >= MESA_SHADER_TYPES)
> return 0;
> diff --git a/src/mesa/state_tracker/st_context.c
> b/src/mesa/state_tracker/st_context.c
> index 7d18c25..80393cf 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -221,11 +221,6 @@ struct st_context *st_create_context(gl_api api, struct
> pipe_context *pipe,
> struct gl_context *shareCtx = share ? share->ctx : NULL;
> struct dd_function_table funcs;
>
> - /* Sanity checks */
> - STATIC_ASSERT(MESA_SHADER_VERTEX == PIPE_SHADER_VERTEX);
> - STATIC_ASSERT(MESA_SHADER_FRAGMENT == PIPE_SHADER_FRAGMENT);
> - STATIC_ASSERT(MESA_SHADER_GEOMETRY == PIPE_SHADER_GEOMETRY);
> -
> memset(&funcs, 0, sizeof(funcs));
> st_init_driver_functions(&funcs);
>
> diff --git a/src/mesa/state_tracker/st_extensions.c
> b/src/mesa/state_tracker/st_extensions.c
> index 83e8e2e..c0ec06a 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -68,7 +68,7 @@ void st_init_limits(struct st_context *st)
> {
> struct pipe_screen *screen = st->pipe->screen;
> struct gl_constants *c = &st->ctx->Const;
> - gl_shader_type sh;
> + unsigned sh;
> boolean can_ubo = TRUE;
>
> c->MaxTextureLevels
> @@ -149,23 +149,25 @@ void st_init_limits(struct st_context *st)
> can_ubo = FALSE;
> }
>
> - for (sh = 0; sh < MESA_SHADER_TYPES; ++sh) {
> - struct gl_shader_compiler_options *options =
> - &st->ctx->ShaderCompilerOptions[sh];
> + for (sh = 0; sh < PIPE_SHADER_TYPES; ++sh) {
> + struct gl_shader_compiler_options *options;
> struct gl_program_constants *pc;
>
> switch (sh) {
> case PIPE_SHADER_FRAGMENT:
> pc = &c->FragmentProgram;
> + options = &st->ctx->ShaderCompilerOptions[MESA_SHADER_FRAGMENT];
> break;
> case PIPE_SHADER_VERTEX:
> pc = &c->VertexProgram;
> + options = &st->ctx->ShaderCompilerOptions[MESA_SHADER_VERTEX];
> break;
> case PIPE_SHADER_GEOMETRY:
> pc = &c->GeometryProgram;
> + options = &st->ctx->ShaderCompilerOptions[MESA_SHADER_GEOMETRY];
> break;
> default:
> - assert(0);
> + /* compute shader, etc. */
> continue;
> }
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 32bc2b3..64e0a8a 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -5271,15 +5271,11 @@ st_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
> linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i]);
>
> if (linked_prog) {
> - static const GLenum targets[] = {
> - GL_VERTEX_PROGRAM_ARB,
> - GL_FRAGMENT_PROGRAM_ARB,
> - GL_GEOMETRY_PROGRAM_NV
> - };
> -
> _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program,
> linked_prog);
> - if (!ctx->Driver.ProgramStringNotify(ctx, targets[i], linked_prog))
> {
> + if (!ctx->Driver.ProgramStringNotify(ctx,
> +
> _mesa_program_index_to_target(i),
> + linked_prog)) {
> _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program,
> NULL);
> _mesa_reference_program(ctx, &linked_prog, NULL);
> --
> 1.8.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list