Bug 29684 - [glsl] wine shaders break with mesa GLSL
Summary: [glsl] wine shaders break with mesa GLSL
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-19 22:55 UTC by Rubén Fernández
Modified: 2010-08-22 16:34 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Log for the game "The Abbey" using MESA_GLSL=dump (44.18 KB, application/octet-stream)
2010-08-19 22:56 UTC, Rubén Fernández
Details
Log for the game "Dark Corners of the Earth" using MESA_GLSL=dump (84.99 KB, application/octet-stream)
2010-08-19 22:57 UTC, Rubén Fernández
Details
Log for the game "Doctor Who: the Adventure Games" using MESA_GLSL=dump (36.87 KB, application/octet-stream)
2010-08-19 22:58 UTC, Rubén Fernández
Details
Log for the game "Portal" using MESA_GLSL=dump (45.15 KB, application/octet-stream)
2010-08-19 22:58 UTC, Rubén Fernández
Details
output with MESA_GLSL=dump (29.15 KB, application/octet-stream)
2010-08-22 08:28 UTC, Marc Dietrich
Details

Description Rubén Fernández 2010-08-19 22:55:36 UTC
Every wine program or game using shaders that I have tried, fail to render using the new GLSL compiler (all of them black screens); all of them run correctly using either the proprietary driver or disabling GLSL on wine (which makes wine use assembly shaders); most of them didn't work with the old GLSL compiler either, so not much of a regression.

Graphics Card: ATI Technologies Inc RV505 [Radeon X1550 64-bit]
CPU: Intel Core Duo 1.8 Ghz, 2.5 GB RAM
Linux kernel 2.6.34, libdrm 2.4.21

Using MESA_GLSL=dump shows that many shaders fail to compile; the error messages seem to be always one of these:

Cannot access field `xy' of non-structure / non-vector.
(caused by this line of code: "gl_TexCoord[0].xy = clamp(OUT[0].xy, -FLT_MAX, FLT_MAX);"

and,

vector index must be < 4
(caused by the line "gl_Position.xyzw = clamp(OUT[10].xyzw, -FLT_MAX, FLT_MAX);")
Comment 1 Rubén Fernández 2010-08-19 22:56:33 UTC
Created attachment 38001 [details]
Log for the game "The Abbey" using MESA_GLSL=dump
Comment 2 Rubén Fernández 2010-08-19 22:57:11 UTC
Created attachment 38002 [details]
Log for the game "Dark Corners of the Earth" using MESA_GLSL=dump
Comment 3 Rubén Fernández 2010-08-19 22:58:00 UTC
Created attachment 38003 [details]
Log for the game "Doctor Who: the Adventure Games" using MESA_GLSL=dump
Comment 4 Rubén Fernández 2010-08-19 22:58:30 UTC
Created attachment 38004 [details]
Log for the game "Portal" using MESA_GLSL=dump
Comment 5 Pavel Ondračka 2010-08-21 05:10:32 UTC
Wine d3d9 tests are another way to reproduce this.
Comment 6 Henri Verbeet 2010-08-21 06:07:25 UTC
For what it's worth, this happens because of the array parameter in the function declaration "void order_ps_input(in vec4 OUT[12])" being mishandled by the GLSL compiler, as mentioned earlier on IRC to Eric and Ian. "void order_ps_input(in vec4[12] OUT)" does work as expected. That's also an easy workaround if you don't mind patching Wine source.
Comment 7 Sven Arvidsson 2010-08-21 14:22:19 UTC
Another test case is rthdribl, which worked before the glsl2 merge, but now segfaults. I'm not really sure how to get a proper backtrace from Wine though.

http://www.daionet.gr.jp/~masa/rthdribl/
Comment 8 Kenneth Graunke 2010-08-21 15:40:25 UTC
(In reply to comment #6)
> For what it's worth, this happens because of the array parameter in the
> function declaration "void order_ps_input(in vec4 OUT[12])" being mishandled by
> the GLSL compiler, as mentioned earlier on IRC to Eric and Ian. "void
> order_ps_input(in vec4[12] OUT)" does work as expected. That's also an easy
> workaround if you don't mind patching Wine source.

Thanks, Henri - good catch!  I've added piglit tests array-19.vert, array-20.vert, and array-21.vert to test this, and fixed them in:

commit e511a35fc53fb75a2401d8a94c0c35634175c575
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Sat Aug 21 15:30:34 2010 -0700

    glsl: Handle array declarations in function parameters.
    
    The 'vec4[12] foo' style already worked, but the 'vec4 foo[12]' style
    did not.  Also, 'vec4[] foo' was wrongly accepted.
    
    Fixes piglit test cases array-19.vert and array-21.vert.
    
    May fix fd.o bug #29684 (or at least part of it).

I haven't tested with wine, though.
Comment 9 Rubén Fernández 2010-08-21 17:10:02 UTC
(In reply to comment #8)
> I haven't tested with wine, though.

I did, and it works now.

As far as I see, wine+glsl now works with the new GLSL compiler just as it did with the old one; can't find any compilation errors or regressions.

That doesn't mean it's bug free, though; but the bugs I found also happened with the old compiler (which suggests that they are bugs in the radeon driver, rather than the compiler); but I shall report them as new bugs.
Comment 10 Marc Dietrich 2010-08-22 08:26:47 UTC
mmh - still broken here, but works when "UseGLSL = disabled" in wine.
Comment 11 Marc Dietrich 2010-08-22 08:28:46 UTC
Created attachment 38060 [details]
output with MESA_GLSL=dump
Comment 12 Rubén Fernández 2010-08-22 09:41:32 UTC
(In reply to comment #10)
> mmh - still broken here, but works when "UseGLSL = disabled" in wine.

Your output log indicates no compilation errors - which means that this particular bug is fixed. I know there are still GLSL bugs in wine, but I think they should be reported as separate bugs, as I'll do soon.
Comment 13 Marc Dietrich 2010-08-22 11:26:51 UTC
ok - I didn't read the whole bug report (just the symptoms). sorry, closing again.
Comment 14 Rubén Fernández 2010-08-22 16:34:32 UTC
(In reply to comment #13)
> ok - I didn't read the whole bug report (just the symptoms). sorry, closing
> again.

You can now follow / contribute to improving GLSL with this new bug report:
https://bugs.freedesktop.org/show_bug.cgi?id=29741


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.