Bug 44333 - [bisected] Color distortion with xbmc mediaplayer
[bisected] Color distortion with xbmc mediaplayer
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965
git
x86-64 (AMD64) Linux (All)
: medium major
Assigned To: Kenneth Graunke
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-30 14:53 UTC by alanw
Modified: 2012-02-15 18:15 UTC (History)
2 users (show)

See Also:


Attachments
Screenshot (43.42 KB, image/jpeg)
2011-12-30 14:53 UTC, alanw
Details
YUV to RGB shader (3.49 KB, text/plain)
2012-01-03 09:07 UTC, elupus
Details
Possible fix for the optimization (1.57 KB, patch)
2012-01-17 09:57 UTC, Kenneth Graunke
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description alanw 2011-12-30 14:53:53 UTC
Created attachment 54988 [details]
Screenshot

I have ugly color distortion with xbmc mediaplayer (www.xbmc.org)
As I know xbmc uses glsl for video rendering.

This distortion was not an issue with mesa 7.11. It is only introduced with pre 7.12 builds. The bug is been in the code for a long time (weeks, months).

Hardware: Intel Sandy Bridge HD3000
Opsys: Ubuntu Oneiric x64. Updated from the xorg-edgers ppa.
Kernel: 3.2-rc7
Xorg 7.12 git 22 12 2011
SNA was disabled.

Other users also reported this problem.
Comment 1 Ian Romanick 2012-01-02 20:16:36 UTC
Any chance of a bisect?  That would help narrow down the problem.
Comment 2 alanw 2012-01-03 04:00:13 UTC
Yes. I'll make a few compiles and try to narrow down the cause as much as possible. I'll get back to you.
Comment 3 elupus 2012-01-03 09:06:43 UTC
Was just about to report this. It seem to be a miss compilation of the GLSL shader. Just dropping color output from one of the source textures resolves the issue.

The shader takes three textures put them in a vector and does a matrix multiplication to convert yuv to rgb. (simplified as below)

  vec4 yuv, rgb;
  yuv.rgba = vec4( texture2D(m_sampY, m_cordY).r
                 , texture2D(m_sampU, m_cordU).g
                 , texture2D(m_sampV, m_cordV).a
                 , 1.0 );

  rgb   = m_yuvmat * yuv;
  rgb.a = gl_Color.a;
  gl_FragColor = rgb;


Only one of the elements in the rgba ends up wrong after matrix multiplication. Writing the sampled data from the texture directly to output does not yield the error.

I'll attached the shader.
Comment 4 elupus 2012-01-03 09:07:16 UTC
Created attachment 55089 [details]
YUV to RGB shader
Comment 5 elupus 2012-01-03 09:11:25 UTC
Forgot to say that only define set for that shader is XBMC_YV12, the others are not.
Comment 6 alanw 2012-01-04 16:57:14 UTC
I found the problematic commit, after a long search:

It is called: i965: Avoid generating MOVs for most ir_assignment handling.
And is here: http://cgit.freedesktop.org/mesa/mesa/commit/?id=dc7f449d1ac53a66e6efb56ccf2a5953418a26ca

With a revert patch for current master, everything is fine again:
http://pastebin.com/cLpMCXwg

Please check if there is another solution than ti revert it!

Thank you! Regards, Alan
Comment 7 Kenneth Graunke 2012-01-17 09:57:01 UTC
Created attachment 55689 [details] [review]
Possible fix for the optimization

Does the attached patch (against Mesa master) solve the issue for you?  It's kind of a stab in the dark, but worth a try...
Comment 8 alanw 2012-01-17 16:24:54 UTC
Hi Kenneth !

Unfortunately this patch did not help.
Comment 9 Kenneth Graunke 2012-01-17 22:37:42 UTC
Alright, thanks.
Comment 10 elupus 2012-02-12 12:19:51 UTC
Shameless bump. Issue still remain. It's a bit annoying to not being able to use GLSL shaders in xbmc on intel hardware now that they have started become quite usable otherwise.

Can i get you guys some other logs to figure out why it get's misscompiled?
Comment 11 Kenneth Graunke 2012-02-12 17:45:11 UTC
Actually, I was finally able to reproduce this.  For some reason, XBMC appears to crash the Fedora 16 X server, but my Archlinux system works.  Now that I can see the issue first-hand, I should be able to track it down.  Sorry for the delay.
Comment 12 Kenneth Graunke 2012-02-12 23:32:48 UTC
Obvious two-line fix posted to the mailing list:
http://lists.freedesktop.org/archives/mesa-dev/2012-February/018969.html

With the patch, my cartoons are no longer toxic green. :)

I should be able to get this into Mesa 8.0.1, which is due out shortly.
Comment 13 elupus 2012-02-13 00:36:54 UTC
Awesome to hear!
Comment 14 alanw 2012-02-14 01:05:11 UTC
Thank you for the fix Kenneth,

Cheers !
Comment 15 Kenneth Graunke 2012-02-15 18:15:55 UTC
Turns out that was a bit overzealous.

The bug is fixed in master via the following commits:

commit 4b274068204c7f0bacaa4639f24feb433353b861
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Tue Feb 14 12:43:22 2012 -0800

    i965/fs: Take # of components into account in try_rewrite_rhs_to_dst.
    
    Commit dc7f449d1ac53a66e6efb56ccf2a5953418a26ca introduced a new method
    for avoiding MOVs: try to rewrite the destination of the instruction
    that produced the RHS so it writes into the LHS.
    
    Unfortunately, this is not safe for swizzled texturing operations, as
    they return a set of four contiguous registers.  Consider the following:
    
    (assign (x)
            (var_ref vec_ctor_x)
            (swiz x (tex vec4 (var_ref m_sampY) (var_ref m_cordY) 0 1 ())))
    
    In this case, the source and destination registers are equal, since
    reg_offset is 0 for both.  Yet, this is only a partial move: the texture
    operation generates four registers, and the LHS only covers one.
    
    Fixes color distortion in XBMC when using GLSL shaders.
    
    NOTE: This is a candidate for the 8.0 branch (with the previous commit).
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44333
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>

commit 8ab02b511882857a09fceed0e93bf4a0b25c17b2
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Tue Feb 14 12:43:21 2012 -0800

    i965/fs: Add a new fs_inst::regs_written function.
    
    Certain instructions write more than one register.  Texturing, for
    example, returns 4 registers.  (We set rlen to 4 even for TXS and float
    shadow sampling.)  Some math functions return 2.  Most return 1.
    
    The next commit introduces a use of this function.
    
    NOTE: This is a candidate for the 8.0 branch (dependency of a fix).
    
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>

I've also cherry-picked these to the 8.0 branch, so they'll show up in Mesa 8.0.1 which should be coming out in a day or two.

Thanks for the bug report and the great help!  The bisect and shader analysis were both very helpful.