On Mon, Sep 28, 2015 at 5:12 AM, Tapani Pälli <tapani.pa...@intel.com> wrote:
> Commit 4639cea2921669527eb43dcb49724c05afb27e8e added packed varyings
> as part of program resource list. We need to take this to account with
> all functions dealing with active input attributes.
>
> Patch fixes the issue by adding additional check to is_active_attrib
> and iterating active attribs explicitly in GetActiveAttrib function.
>
> I've tested that fix works for Assault Android Cactus (demo version)
> and does not cause Piglit or CTS regressions in glGetProgramiv tests.
>
> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92122
> ---
> src/mesa/main/shader_query.cpp | 44
> +++++++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 99d9e10..d023504 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -101,15 +101,34 @@ _mesa_BindAttribLocation(GLhandleARB program, GLuint
> index,
> */
> }
>
> +/* Function checks if given input variable is packed varying by checking
> + * if there also exists output variable with the same name.
> + */
> +static bool
> +is_packed_varying(struct gl_shader_program *shProg, const ir_variable *input)
> +{
> + assert(input->data.mode == ir_var_shader_in);
No statements before variable decls in core code.
> + unsigned array_index = 0;
> + struct gl_program_resource *out =
> + _mesa_program_resource_find_name(shProg, GL_PROGRAM_OUTPUT,
> input->name,
> + &array_index);
This is slow, right? I.e. it's a loop?
> + if (out)
> + return true;
> + return false;
> +}
> +
> static bool
> -is_active_attrib(const ir_variable *var)
> +is_active_attrib(struct gl_shader_program *shProg,
> + const ir_variable *var)
> {
> if (!var)
> return false;
>
> switch (var->data.mode) {
> case ir_var_shader_in:
> - return var->data.location != -1;
> + return (var->data.location != -1 &&
> + var->data.location_frac == 0 &&
> + !is_packed_varying(shProg, var));
>
> case ir_var_system_value:
> /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
> @@ -154,19 +173,25 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint
> desired_index,
> return;
> }
>
> - struct gl_program_resource *res =
> - _mesa_program_resource_find_index(shProg, GL_PROGRAM_INPUT,
> - desired_index);
You appear to replace this function, which is O(n), with a loop around
what is essentially this same function, which is O(n^2). Could you
write a more targeted helper which achieves what you want with a
single scan of the program resource list?
> + struct gl_program_resource *res = shProg->ProgramResourceList;
> + unsigned index = -1;
> + for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++) {
> + if (res->Type != GL_PROGRAM_INPUT ||
> + !is_active_attrib(shProg, RESOURCE_VAR(res)))
> + continue;
> + if (++index == desired_index)
> + break;
> + }
>
> /* User asked for index that does not exist. */
> - if (!res) {
> + if (!res || index != desired_index) {
> _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
> return;
> }
>
> const ir_variable *const var = RESOURCE_VAR(res);
>
> - if (!is_active_attrib(var))
> + if (!is_active_attrib(shProg, var))
> return;
>
> const char *var_name = var->name;
> @@ -252,7 +277,7 @@ _mesa_count_active_attribs(struct gl_shader_program
> *shProg)
> for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
> if (res->Type == GL_PROGRAM_INPUT &&
> res->StageReferences & (1 << MESA_SHADER_VERTEX) &&
> - is_active_attrib(RESOURCE_VAR(res)))
> + is_active_attrib(shProg, RESOURCE_VAR(res)))
> count++;
> }
> return count;
> @@ -271,7 +296,8 @@ _mesa_longest_attribute_name_length(struct
> gl_shader_program *shProg)
> size_t longest = 0;
> for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
> if (res->Type == GL_PROGRAM_INPUT &&
> - res->StageReferences & (1 << MESA_SHADER_VERTEX)) {
> + res->StageReferences & (1 << MESA_SHADER_VERTEX) &&
> + is_active_attrib(shProg, RESOURCE_VAR(res))) {
>
> const size_t length = strlen(RESOURCE_VAR(res)->name);
> if (length >= longest)
> --
> 2.4.3
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
