Bugzilla – Bug 35849
when sampling textures from both fragment and vertex shaders the vertex texture has the incorrect texture bound
Last modified: 2011-05-15 13:49:56 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.
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. 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.
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.
I'd also replace "slot" with "unit".
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.
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.
Thanks. I've committed the piglit patch with some additional fixes (put declarations before code, etc).
I've hopefully fixed this in master for gallium drivers, by doing what Brian suggested.