On 10/02/2015 07:46 PM, Ilia Mirkin wrote:
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.
ok
+ 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?
It is agreeably slow, alternatively we could add new flag to ir_variable
to make things quick.
+ 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?
OK, I can simplify this. At this point I think it would be fine to
revert the patches adding packed varyings to resource list. I can then
send these changes together when adding them back.
+ 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
