[Mesa-dev] [PATCH 03/15] mesa: Move _mesa_GetAttribLocationARB to shader_query.cpp
Kenneth Graunke
kenneth at whitecape.org
Thu Sep 29 14:13:46 PDT 2011
On 09/29/2011 10:51 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> This allows querying the linked shader itself rather than the Mesa IR.
> This is the first step towards removing gl_program::Attributes.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
NAK. Comments below.
> ---
> src/mesa/main/shader_query.cpp | 78 ++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/shaderapi.c | 41 ---------------------
> src/mesa/sources.mak | 3 +-
> 3 files changed, 80 insertions(+), 42 deletions(-)
> create mode 100644 src/mesa/main/shader_query.cpp
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> new file mode 100644
> index 0000000..b8c4b6a
> --- /dev/null
> +++ b/src/mesa/main/shader_query.cpp
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright © 2011 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file shader_query.cpp
> + * C-to-C++ bridge functions to query GLSL shader data
> + *
> + * \author Ian Romanick <ian.d.romanick at intel.com>
> + */
> +
> +#include "main/core.h"
> +#include "glsl_symbol_table.h"
> +#include "ir.h"
> +#include "shaderobj.h"
> +
> +extern "C" {
> +#include "shaderapi.h"
> +}
> +
> +GLint GLAPIENTRY
> +_mesa_GetAttribLocationARB(GLhandleARB program, const GLcharARB * name)
> +{
> + GET_CURRENT_CONTEXT(ctx);
> + struct gl_shader_program *const shProg =
> + _mesa_lookup_shader_program_err(ctx, program, "glGetAttribLocation");
> +
> + if (!shProg) {
> + return -1;
> + }
> +
> + if (!shProg->LinkStatus) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glGetAttribLocation(program not linked)");
> + return -1;
> + }
> +
> + if (!name)
> + return -1;
> +
> + struct gl_shader *const vs = shProg->_LinkedShaders[MESA_SHADER_VERTEX];
> + if (vs != NULL) {
> + const ir_variable *const var = vs->symbols->get_variable(name);
You pointed out the other day that the ir_variables for unused
attributes get removed from the IR post-linking and even freed, but
_don't_ get removed from the symbol table...since we actually have no
mechanism to do that.
(That's actually pretty broken in its own right - post-linking, we
should just delete the whole symbol table and NULL out the pointer, to
prevent people from doing stuff like this and hitting bugs.)
As such, you can't look things up this way: you'll get locations for
non-existent attributes. Your later patch to GetActiveAttribARB uses a
different mechanism that (presumably) works; you probably just want to
refactor that out and use it here too.
> +
> + /* The extra check against VERT_ATTRIB_GENERIC0 is because
> + * glGetAttribLocation cannot be used on "conventional" attributes.
> + *
> + * From page 95 of the OpenGL 3.0 spec:
> + *
> + * "If name is not an active attribute, if name is a conventional
> + * attribute, or if an error occurs, -1 will be returned."
> + */
> + if (var != NULL
> + && var->mode == ir_var_in
> + && var->location >= VERT_ATTRIB_GENERIC0)
> + return var->location - VERT_ATTRIB_GENERIC0;
> + }
> + return -1;
> +}
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 3ce14d1..49e65e8 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -324,39 +324,6 @@ attach_shader(struct gl_context *ctx, GLuint program, GLuint shader)
> }
>
>
> -static GLint
> -get_attrib_location(struct gl_context *ctx, GLuint program, const GLchar *name)
> -{
> - struct gl_shader_program *shProg
> - = _mesa_lookup_shader_program_err(ctx, program, "glGetAttribLocation");
> -
> - if (!shProg) {
> - return -1;
> - }
> -
> - if (!shProg->LinkStatus) {
> - _mesa_error(ctx, GL_INVALID_OPERATION,
> - "glGetAttribLocation(program not linked)");
> - return -1;
> - }
> -
> - if (!name)
> - return -1;
> -
> - if (shProg->VertexProgram) {
> - const struct gl_program_parameter_list *attribs =
> - shProg->VertexProgram->Base.Attributes;
> - if (attribs) {
> - GLint i = _mesa_lookup_parameter_index(attribs, -1, name);
> - if (i >= 0) {
> - return attribs->Parameters[i].StateIndexes[0];
> - }
> - }
> - }
> - return -1;
> -}
> -
> -
> static void
> bind_attrib_location(struct gl_context *ctx, GLuint program, GLuint index,
> const GLchar *name)
> @@ -1316,14 +1283,6 @@ _mesa_GetAttachedShaders(GLuint program, GLsizei maxCount,
> }
>
>
> -GLint GLAPIENTRY
> -_mesa_GetAttribLocationARB(GLhandleARB program, const GLcharARB * name)
> -{
> - GET_CURRENT_CONTEXT(ctx);
> - return get_attrib_location(ctx, program, name);
> -}
> -
> -
> /* GL_EXT_gpu_shader4, GL3 */
> GLint GLAPIENTRY
> _mesa_GetFragDataLocation(GLuint program, const GLchar *name)
> diff --git a/src/mesa/sources.mak b/src/mesa/sources.mak
> index da5d90e..e473b49 100644
> --- a/src/mesa/sources.mak
> +++ b/src/mesa/sources.mak
> @@ -104,7 +104,8 @@ MAIN_SOURCES = \
> $(MAIN_ES_SOURCES)
>
> MAIN_CXX_SOURCES = \
> - main/ff_fragment_shader.cpp
> + main/ff_fragment_shader.cpp \
> + main/shader_query.cpp
>
> MATH_SOURCES = \
> math/m_debug_clip.c \
More information about the mesa-dev
mailing list