Bug 29823 - GetUniform[if]v busted
GetUniform[if]v busted
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Mesa core
7.10
Other All
: medium critical
Assigned To: Ian Romanick
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-26 13:32 UTC by Vladimir Vukicevic
Modified: 2011-03-02 07:54 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
special-case code for float arrays (579 bytes, patch)
2010-08-27 08:03 UTC, Brian Paul
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Vukicevic 2010-08-26 13:32:16 UTC
(Mesa 7.8.2, reproduced in 7.7.x as well.)

Given a shader that includes a uniform such as:

uniform float u_array[4];

void main()
{
... something that uses u_array[0..3];
}

glGetUniformfv with the location of "u_array" or "u_array[0]" (they're identical locations, as per the spec) does something very strange that ends up scribbling the stack.  For that uniform, the gl_program_parameter struct looks like:

Name: "u_array"
Type: PROGRAM_UNIFORM
DataType: 0x1406
Size: 0x10
Used: 0x01
Initialized: 0x01
Flags: 0

The Size of 0x10 seems to have somehow ended up as the size in bytes of the uniform storage.  This causes a problem in get_uniform_rows_cols, where it does:

if (p->Size <= 4) {
  *rows = 1;
  *cols = p->Size;
} else {
  *rows = p->Size / 4 + 1;
  if (p->Size % 4 == 0)
    *cols = 4;
  else
    *cols = p->Size % 4;
}

We end up with rows = 5 and cols = 4, which causes _mesa_get_uniformfv to attempt to write 4*5 values, which is invalid.

I'm not sure where the bogus Size = 16 is coming from...
Comment 1 Brian Paul 2010-08-27 08:03:00 UTC
All GLSL data is stored in float[4] vectors.  We try to pack individual floats into these vectors but we can't do that for arrays.  So a GLSL array of K floats array really occupies K float[4] vectors.  So in this case a GLSL float[4] array occupies four float[4] vectors which totals 16 (0x10) floats.

The get_uniform_rows_cols() function isn't correctly figuring this out.

I'm attaching a patch/hack to work around this.  Let me know if it helps.

Have you tested with Mesa git/master with the new GLSL compiler?
Comment 2 Brian Paul 2010-08-27 08:03:24 UTC
Created attachment 38223 [details] [review]
special-case code for float arrays
Comment 3 Vladimir Vukicevic 2010-08-27 08:54:36 UTC
Thanks Brian -- will test this, and also working on testing git master now, as per suggestion on IRC yesterday.  I have to switch some code to using surfaceless EGL contexts instead of OSMesa, but that's a better thing going forward anyway.
Comment 4 Vladimir Vukicevic 2010-08-27 13:22:05 UTC
Oh hm, just tested it, and it's not quite correct -- for an array of floats, it will write the entire array, instead of just the 1 component (because u_foo == u_foo[0] as far as getUniformLocation is concerned, and getUniform is supposed to just return the specific array component in that case, not the entire array).

If I force the rows/cols to 1, then it works for array index [0], but u_foo[1] returns the same value as u_foo[0].  split_location_offset in _mesa_get_uniformfv is returning an offset of 1 for u_foo[1], but the function doesn't use the offset at all it looks like.

(Still working on testing with mesa from git via egl.)
Comment 5 Vladimir Vukicevic 2010-08-27 14:17:45 UTC
Got things working with mesa from git master, via EGL -- this works correctly there.
Comment 6 Vladimir Vukicevic 2010-08-27 14:21:23 UTC
Argh, ignore that, I had the wrong EGL library in place (ANGLE, not Mesa).
Comment 7 Vladimir Vukicevic 2010-08-27 22:11:30 UTC
Ok, just got mesa from git master working -- it has the same problem with what looks like the same cause (including offset being ignored if the workaround patch is used).
Comment 8 Brian Paul 2010-08-30 07:06:37 UTC
Can you provide a simple glut test program for this?  Then if I can find some time I'll try to debug further.
Comment 9 Marek Olšák 2011-02-06 06:49:17 UTC
Is this still an issue with current Mesa git?
Comment 10 Ian Romanick 2011-02-07 13:44:26 UTC
This should be fixed in master by the commit below.  I'm leaving the bug open until the fix is cherry-picked back to the stable branches.

commit 20d278a7ff0ce66e5c4ac437e1fbe52c31a1ecb3
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Thu Jan 27 12:24:27 2011 -0800

    mesa: glGetUniform only returns a single element of an array
    
    Also return it as the correct type.  Previously the whole array would
    be returned and each element would be expanded to a vec4.
    
    Fixes piglit test getuniform-01 and bugzilla #29823.
Comment 11 Marek Olšák 2011-03-02 07:54:34 UTC
(In reply to comment #10)
> This should be fixed in master by the commit below.  I'm leaving the bug open
> until the fix is cherry-picked back to the stable branches.

Already cherry-picked, closing..