Bug 85376 - Dolphin emulator has bad colors
Summary: Dolphin emulator has bad colors
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r600 (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-23 17:48 UTC by ghallberg+bugzilla
Modified: 2014-11-23 03:00 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Fifo-file which displays the discoloration (120.06 KB, text/plain)
2014-10-23 17:48 UTC, ghallberg+bugzilla
Details
Apitrace of the faulty fifo (875.06 KB, application/octet-stream)
2014-11-10 17:04 UTC, ghallberg+bugzilla
Details
Result of R600_DEBUG=ps,fs apitrace replay dolphin-emu.2.trace (122.29 KB, text/plain)
2014-11-10 17:08 UTC, ghallberg+bugzilla
Details
apitrace with MESA_EXTENSION_OVERRIDE=-GL_ARB_buffer_storage (508.73 KB, application/octet-stream)
2014-11-10 19:25 UTC, ghallberg+bugzilla
Details
rsps_edit6 (121.70 KB, text/plain)
2014-11-10 19:58 UTC, ghallberg+bugzilla
Details
psfs4 (74.48 KB, text/plain)
2014-11-10 20:01 UTC, ghallberg+bugzilla
Details
psfs results of edit7 (71.05 KB, text/plain)
2014-11-11 17:23 UTC, ghallberg+bugzilla
Details
psfs results of edit9 (68.54 KB, text/plain)
2014-11-11 19:05 UTC, ghallberg+bugzilla
Details
Proposed fix (2.29 KB, text/plain)
2014-11-11 21:31 UTC, Glenn Kennard
Details
psfs output on edit9 with glennks patch (138.52 KB, text/plain)
2014-11-12 21:15 UTC, ghallberg+bugzilla
Details
patch to hopefully fix (2.11 KB, patch)
2014-11-18 00:38 UTC, Dave Airlie
Details | Splinter Review
Apitrace of dolphin crashing while running the fifo posted above (151.61 KB, application/octet-stream)
2014-11-22 07:51 UTC, ghallberg+bugzilla
Details
gdb output from crash in dolphin (11.31 KB, text/plain)
2014-11-22 07:52 UTC, ghallberg+bugzilla
Details

Description ghallberg+bugzilla 2014-10-23 17:48:05 UTC
Created attachment 108308 [details]
Fifo-file which displays the discoloration

When using recent versions of dolphin-emulator with open source amd drivers, the graphics are tinted blue.

An example can be seen in this link: http://imgur.com/eLqLMdW

The example image was created when running the attached fifo-file in dolphin.

This is using the latest version of dolphin from the git-repo: 4.0.r3771.ce3a039-1 built with gcc 4.9.1-2 from the arch repositories

mesa is the 10.3.1 from the arch repositories and seems to be built with clang.

I'm running on an AMD A6-6400K APU.

I've tested the same versions of the software on my intel-based laptop, and the discoloration is not present there.
Comment 1 ghallberg+bugzilla 2014-11-10 16:10:16 UTC
Here's a discussion thread on the dolphin forums:

https://forums.dolphin-emu.org/Thread-everything-is-the-wrong-color-on-linux-with-open-source-amd-drivers?pid=340104#pid340104

And the change in dolphin which probably caused the behaviour to change: https://github.com/dolphin-emu/dolphin/commit/a9a8c730748b8c8a2e83feb4c38626361cf9daa1
Comment 2 ghallberg+bugzilla 2014-11-10 17:04:16 UTC
Created attachment 109230 [details]
Apitrace of the faulty fifo
Comment 3 ghallberg+bugzilla 2014-11-10 17:08:42 UTC
Created attachment 109231 [details]
Result of  R600_DEBUG=ps,fs apitrace replay dolphin-emu.2.trace
Comment 4 Markus Wick 2014-11-10 17:48:41 UTC
The affected shader seems to be the #22 one, but it's hard to guess what's wrong with the shader http://pastie.org/private/ccwhydvtgzw0rc1t8xdeoq

As abstract of this dolphin commit, it's basicly a rewrite of our fragment shaders with integer math, so I doubt it's useful to debug in this direction :/

About this shader, I see two uncommon features: We write to gl_FragDepth and we use a bitshift with an offset from an integer ubo. Maybe I'm lucky ...

About the issue itself, it only happens on *some* GPUs, so maybe a kind of shader compilation issue?
Comment 5 ghallberg+bugzilla 2014-11-10 19:25:05 UTC
Created attachment 109239 [details]
apitrace with MESA_EXTENSION_OVERRIDE=-GL_ARB_buffer_storage
Comment 6 ghallberg+bugzilla 2014-11-10 19:58:06 UTC
Created attachment 109245 [details]
rsps_edit6
Comment 7 ghallberg+bugzilla 2014-11-10 20:01:14 UTC
Created attachment 109247 [details]
psfs4
Comment 8 ghallberg+bugzilla 2014-11-11 17:23:35 UTC
Created attachment 109289 [details]
psfs results of edit7
Comment 9 ghallberg+bugzilla 2014-11-11 19:05:35 UTC
Created attachment 109303 [details]
psfs results of edit9
Comment 10 Ilia Mirkin 2014-11-11 20:33:50 UTC
Some observations (from IRC, edited):

In shader 21 (from attachment 109303 [details]), we see the following:

<imirkin_> MULLO_INT          R5.x,  [0x00000046 9.80909e-44].x, PV.x
<imirkin_> MULLO_INT          R5.y,  [0x00000078 1.68156e-43].x, PV.x
<imirkin_> that seems wrong
<imirkin_> the second PV.x uses the newly computed R5.x, no?
<imirkin_> instead of the previous R5.x before the mul happened
<imirkin_> so the 2nd and 3rd components get the compounded multiplication factor of .x and the respective .y and .z factor
<tstellar> Starting with the R5.x value returned by the ADD_INT, the caluclation is: R5.y = ((R5.x *46) * 78); R5.z = (R5.x * 46) * 0xc8
<imirkin_>   6: UMUL TEMP[1].xyz, IMM[0].yzww, TEMP[1].xxxx
<imirkin_> i'm guessing the thing that splits it up into 3 instructions forgets that it's overwriting the source
<imirkin_> cayman_mul_int_instr
<imirkin_> cayman-specific, and doesn't do the tmp register dance

vs the "regular" umul implementation (op2_trans) which will use a temp register if the dst mask has multiple dests. I wonder if all the cayman_* emit functions need this treatment.
Comment 11 Glenn Kennard 2014-11-11 21:31:36 UTC
Created attachment 109308 [details]
Proposed fix

Does the attached patch fix the issue?
Comment 12 ghallberg+bugzilla 2014-11-12 21:15:14 UTC
Created attachment 109371 [details]
psfs output on edit9 with glennks patch
Comment 13 ghallberg+bugzilla 2014-11-14 18:06:12 UTC
The attached patch did not help :(
Comment 14 Dave Airlie 2014-11-18 00:38:19 UTC
Created attachment 109650 [details] [review]
patch to hopefully fix
Comment 15 Matthew Woehlke 2014-11-20 02:49:08 UTC
Not sure if this is fixed or not, but I was able to isolate the GLSL where things go sideways.

This is broken:

    prev.rgb = (prev.rgb * (256 - ifog) + cfogcolor.rgb * ifog) >> 8;

The above apparently should have the same effect as the following, which works correctly:

    prev.r = (prev.r * (256 - ifog) + cfogcolor.r * ifog) >> 8;
    prev.g = (prev.g * (256 - ifog) + cfogcolor.g * ifog) >> 8;
    prev.b = (prev.b * (256 - ifog) + cfogcolor.b * ifog) >> 8;

So something is going sideways distributing the above operations across vector components. Seems reasonable to suspect code generation.

I tried splitting just the bitshift step with no effect, so my guess is that at least multiplying an int3 by an int is doing something wrong.
Comment 16 ghallberg+bugzilla 2014-11-22 07:49:41 UTC
After some patches, dolphin is no segfaulting instead of showing the wong colors. I'm uploading a gdb-log and an apitrace.
Comment 17 ghallberg+bugzilla 2014-11-22 07:51:20 UTC
Created attachment 109838 [details]
Apitrace of dolphin crashing while running the fifo posted above
Comment 18 ghallberg+bugzilla 2014-11-22 07:52:07 UTC
Created attachment 109839 [details]
gdb output from crash in dolphin
Comment 19 ghallberg+bugzilla 2014-11-22 15:11:10 UTC
The crash was due to bad build options on my part. Fix seems to be good!
Comment 20 Michel Dänzer 2014-11-23 03:00:13 UTC
Resolving per comment #19 (you can do this yourself BTW :).


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.