Bug 31923 - [GLSL 1.20] allowing inconsistent centroid declaration between two vertex shaders
[GLSL 1.20] allowing inconsistent centroid declaration between two vertex sha...
Status: VERIFIED FIXED
Product: Mesa
Classification: Unclassified
Component: glsl-compiler
7.10
All Linux (All)
: medium minor
Assigned To: Ian Romanick
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-25 17:43 UTC by Gordon Jin
Modified: 2011-02-11 22:09 UTC (History)
1 user (show)

See Also:


Attachments
test case for piglit (5.42 KB, patch)
2010-11-25 17:43 UTC, Gordon Jin
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Gordon Jin 2010-11-25 17:43:40 UTC
Created attachment 40576 [details] [review]
test case for piglit

This is similar to bug#30261, just s/invariant/centroid.

GLSL 1.20 section 4.3.6 says:
The type and presence of the centroid and invariant qualifiers of varying variables with the same name declared in linked vertex and fragments shaders must match, otherwise the link command will fail.

The mesa glsl compiler fails link correctly for one vertex shader and one
fragment shader.

But it links (incorrectly) successfully for two vertex shaders. The attached case fails with below message:
Program should have failed linking, but it was (incorrectly) successful.
PIGLIT: {'result': 'fail' }

Tested on Piketon (i965) with mesa master.
Comment 1 Vinson Lee 2010-11-25 21:15:46 UTC
I've committed the attached test case (attachment id=40576) to piglit.
Comment 2 Chad Versace 2011-01-10 15:32:43 UTC
I just pushed a fix for this bug.

http://cgit.freedesktop.org/mesa/mesa/commit/?id=61428dd2ab66017f80dc4f3b0793e741d93a6d47
Comment 3 Ian Romanick 2011-01-10 17:26:10 UTC
I had a nearly identical fix too (which is why I assigned the bug to myself), but I'm not convinced that the test is correct.  Both AMD and NVIDIA fail this test on Linux.  In some sense it doesn't matter what the spec says when the majority of implementers do something different.

We can leave the code as-is for now, but I suspect we may revert this patch.
Comment 4 Ian Romanick 2011-01-10 17:45:11 UTC
Leave open until the fix is cherry picked back to the stable branch(es) and the other issues are resolved.
Comment 5 Chad Versace 2011-01-10 18:25:24 UTC
(In reply to comment #3)
> We can leave the code as-is for now, but I suspect we may revert this patch.

In the future, I'll first post bugfix patches here for discussion.
Comment 6 Ian Romanick 2011-01-27 12:30:48 UTC
Fixed by 5f9c50053cb83180a51313ed9130d0e36db5642c (7.9) and 45be27d09b0e581f376de02b1c12a63e17387f7c (7.10).
Comment 7 Gordon Jin 2011-02-11 22:09:26 UTC
verified