On 10/05/2015 10:21 AM, Ilia Mirkin wrote:
On Mon, Oct 5, 2015 at 3:14 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.
Don't you have this same problem with output attributes as well? Is
the situation here that the packed varying pass adds its own varyings
on top of the usual ones, and so things were getting (essentially)
double-counted?
As far as I see the problem is only with active attribute variables,
glGetActiveAttrib used to take in to account all resources with input
type (as it was just calling _mesa_program_resource_find_index). Now it
explicitly checks that varyings are not counted as active attribute
variables.
For querying outputs via program resource queries I don't see any
regressions on the current tests so I'm not convinced there is such problem.
For fragment shader outputs there is a problem but I consider it
separate from this. Mesa lowers fragment output array as series of
variables like 'gl_out_FragData0, gl_out_FragData1 ..' while user
expects there to be 'array[0], array[1] ..' and query for array[0] does
not match gl_out_FragData0. This one I was planning to tackle next.
Patch fixes the issue by adding additional check to is_active_attrib
and iterating only active attribs (not any inputs) 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.
v2: simplify and optimize with help of is_packed_varying bit
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 | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 7189676..6d54e61 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -108,7 +108,8 @@ is_active_attrib(const ir_variable *var)
switch (var->data.mode) {
case ir_var_shader_in:
- return var->data.location != -1;
+ return var->data.location != -1 &&
+ !var->data.is_packed_varying;
case ir_var_system_value:
/* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
@@ -153,12 +154,18 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint
desired_index,
return;
}
- struct gl_program_resource *res =
- _mesa_program_resource_find_index(shProg, GL_PROGRAM_INPUT,
- desired_index);
+ 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(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;
}
@@ -270,7 +277,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(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