Bug 35849

Summary: when sampling textures from both fragment and vertex shaders the vertex texture has the incorrect texture bound
Product: Mesa Reporter: Julian Adams <joolsa>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
i915 platform: i915 features:
Attachments: Adds a test demonstrating the bug to piglit
log of testcase run with shader dumping enabled
updated version of the patch to add a testcase demonstrating the bug to piglit

Description Julian Adams 2011-03-31 13:34:23 UTC
Created attachment 45105 [details] [review]
Adds a test demonstrating the bug to piglit

tested from git commit 6584d0cd4fa3a3255a4c0962f31338601df705cb on master

I've found a problem when sampling textures in both the vertex and the fragment shader of the same program. Under some circumstances the vertex shader samples the texture bound to the fragment shader. I've attached a patch with a piglit test demonstrating this.
Comment 1 Julian Adams 2011-03-31 13:42:21 UTC
Created attachment 45107 [details]
log of testcase run with shader dumping enabled

To me the Mesa IR looks odd: both the fragment program and the vertex program sample from texture[0]. In the testcase I posted there is code which is commented out from the vertex shader. Uncommenting this makes the tests pass, although it simply declares another texture and uses it, writing to a interpolator which is ignored in the fragment shader. It has the side effect of making the texture lookup we want in the vertex shader have a different index in the Mesa IR.
Comment 2 Brian Paul 2011-04-12 17:22:17 UTC
The test program has a few minor issues.

1. "@file fbo-drawbuffers-blend-add.c" is wrong
2. There's a mix of tab and space indention that's inconsistent.  Either use spaces everywhere or tabs everywhere.
Comment 3 Brian Paul 2011-04-12 17:28:22 UTC
I'd also replace "slot" with "unit".
Comment 4 Brian Paul 2011-04-12 17:44:06 UTC
It looks like the logic in update_textures() in st_atom_texture.c is wrong.  Basically, we need to loop over the vertex/geometry/fragment shader types and do validation for each texture referenced by each of those shader types.

I'll try to fix this when I get a little time.
Comment 5 Julian Adams 2011-04-13 13:02:24 UTC
Created attachment 45588 [details] [review]
updated version of the patch to add a testcase demonstrating the bug to piglit

this (hopefully) addresses the whitespace issues mentioned in comment 2 and replaces slot with unit as per comment 3.
Comment 6 Brian Paul 2011-04-13 13:23:05 UTC
Thanks.  I've committed the piglit patch with some additional fixes (put declarations before code, etc).
Comment 7 Dave Airlie 2011-05-15 13:49:56 UTC
I've hopefully fixed this in master for gallium drivers, by doing what Brian suggested.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.