Bug 91173 - Oddworld: Stranger's Wrath HD: disfigured models in wrong colors
Summary: Oddworld: Stranger's Wrath HD: disfigured models in wrong colors
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: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-01 05:32 UTC by Béla Gyebrószki
Modified: 2015-07-02 17:39 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
bisect log for color issue (2.56 KB, text/plain)
2015-07-01 18:42 UTC, Ilia Mirkin
Details
screenshot with HEAD + color patch (1.07 MB, image/png)
2015-07-01 19:34 UTC, Ilia Mirkin
Details
tgsi shader generated by that program (2.73 KB, text/plain)
2015-07-01 21:23 UTC, Ilia Mirkin
Details

Description Béla Gyebrószki 2015-07-01 05:32:26 UTC
The game runs in Wine and I have no such problems with the Nvidia binary drivers.
With nouveau the game renders menus and videos properly, but all in-game character models have strange colors and they are oddly disfigured.
Example video showing the problem:
https://drive.google.com/open?id=0B-tTbLKBl-tOdDRydldfM3FqWW8

The problem exists with disabled shader optimization (NV50_PROG_OPTIMIZE=0) and with the software renderer too.

Trace file (uncompressed 300 MB):
https://drive.google.com/open?id=0B-tTbLKBl-tOQTRPVVdldG1PZEU
The problem is first visible in frame #503.

Fedora 22 32-bit
VGA compatible controller: NVIDIA Corporation G92 [GeForce GTS 250] (rev a2) (prog-if 00 [VGA controller])
Mesa 10.6-branchpoint-773-g1de93f9
Kernel 4.0.6-300.fc22.i686+PAE
Xorg 1.17.2
libdrm-2.4.61-3.fc22.i686
Comment 1 Ilia Mirkin 2015-07-01 07:59:14 UTC
Reproduced on nvc0. Might be nice to verify on a radeon or i965, but most likely a core or at least gallium bug if it repros on swrast as well.
Comment 2 Laurent carlier 2015-07-01 08:36:27 UTC
Also reproduced with radeonsi hd7870/mesa-git/llvm-svn
Comment 3 Tapani Pälli 2015-07-01 09:29:26 UTC
reproduced with i965 (IVB)
Comment 4 Ilia Mirkin 2015-07-01 14:58:33 UTC
Béla (or anyone else): would probably also be good to check that the *trace* replays OK with the nvidia blob drivers. Sometimes wine will do different things there since it has more support for NV_* extensions, and more features are exposed in compat contexts.
Comment 5 Béla Gyebrószki 2015-07-01 15:12:34 UTC
(In reply to Ilia Mirkin from comment #4)
> Béla (or anyone else): would probably also be good to check that the *trace*
> replays OK with the nvidia blob drivers. Sometimes wine will do different
> things there since it has more support for NV_* extensions, and more
> features are exposed in compat contexts.

I replayed the trace with Nvidia binary drivers 340.76 and it rendered models correctly.
Comment 6 Ilia Mirkin 2015-07-01 17:47:26 UTC
The plot thickens... it actually renders fine on HSW (i965) on mesa 10.3.7, but fails with (approx) HEAD. Actually I'm not 100% sure if it's fine, but it's definitely nowhere near as messed up.
Comment 7 Béla Gyebrószki 2015-07-01 18:10:45 UTC
(In reply to Ilia Mirkin from comment #6)
> The plot thickens... it actually renders fine on HSW (i965) on mesa 10.3.7,
> but fails with (approx) HEAD. Actually I'm not 100% sure if it's fine, but
> it's definitely nowhere near as messed up.

I compiled 10.3.7 and can confirm that the color of the models is correct with that Mesa version, but the problem with disfigured models is present in 10.3.7 too.
Comment 8 Ilia Mirkin 2015-07-01 18:42:00 UTC
Created attachment 116848 [details]
bisect log for color issue

OK, so I bisected the color issue to

$ git bisect good
4b249d2eed686384d6d7c36f3232360891d5eeda is the first bad commit
commit 4b249d2eed686384d6d7c36f3232360891d5eeda
Author: Iago Toral Quiroga <itoral@igalia.com>
Date:   Fri Feb 13 10:23:26 2015 +0100

    mesa: Handle transferOps in texstore_rgba
    
    In the recent rewrite of the format conversion code we did not handle this.
    This patch adds the missing support.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89068
    Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
    Cc: "10.5" <mesa-stable@lists.freedesktop.org>
Comment 9 Ilia Mirkin 2015-07-01 19:20:27 UTC
Fix for the color issue available at: http://patchwork.freedesktop.org/patch/53425/

Béla, any chance you could supply a screenshot of what the models are supposed to look like? They don't appear to look *that* wrong... theoretically. Or perhaps they're extra-wrong on nv50 and actually fine on nvc0/etc?
Comment 10 Ilia Mirkin 2015-07-01 19:34:26 UTC
Created attachment 116850 [details]
screenshot with HEAD + color patch

OK, so I guess the model is a little wrong :) Looks like a clipping issue? Or... something.
Comment 11 Béla Gyebrószki 2015-07-01 19:53:22 UTC
(In reply to Ilia Mirkin from comment #9)
> Fix for the color issue available at:
> http://patchwork.freedesktop.org/patch/53425/
> 
> Béla, any chance you could supply a screenshot of what the models are
> supposed to look like? They don't appear to look *that* wrong...
> theoretically. Or perhaps they're extra-wrong on nv50 and actually fine on
> nvc0/etc?

This video shows how models look like with Mesa + your patch applied (colors are OK, but models are still seriously broken):
https://drive.google.com/open?id=0B-tTbLKBl-tOS1p4TEpFTVQ5dGs

This is with Nvidia blob 340.76:
https://drive.google.com/open?id=0B-tTbLKBl-tON09LVi1ISFdZVE0
Comment 12 Ilia Mirkin 2015-07-01 20:23:56 UTC
The problem draw appears to be the one at call 838845. The previous one 838747 still doesn't show any bits of the model.

It uses a vertex program (ARB_fp) with indirect constbuf accesses... I'm assuming it's a vertex program issue since the rasterized geometry seems to be quite off.

TEMP R0;
TEMP R1;
TEMP R2;
TEMP R3;
TEMP R4;
TEMP R5;
ADDRESS A0;
MAD R0.xy, vertex.attrib[1], c[7].y, c[7].x;
ADD R5.x, R0.y, c[0];
ARL A0.x, R5.x;
MOV R0.zw, c[A0.x + 9];
ADD R4.w, R0.x, c[0].x;
MOV R1.x, c[0].w;
ADD R0.x, c[A0.x + 9].y, R1;
FLR R0.y, R0.x;
FRC R0.x, c[A0.x + 9].y;
ARL A0.x, R4.w;
MUL R1.y, R0, c[174].x;
MAD R0.y, R0.x, c[0], -c[0].z;
ADD R0.x, R1.y, -c[0].z;
DP3 R1.y, R0, R0;
ADD R1.x, R1, c[A0.x + 9].y;
RSQ R2.x, R1.y;
FLR R1.y, R1.x;
MOV R1.zw, c[A0.x + 9];
FRC R1.x, c[A0.x + 9].y;
MUL R2.y, R1, c[174].x;
MAD R1.y, R1.x, c[0], -c[0].z;
ADD R1.x, R2.y, -c[0].z;
DP3 R2.w, R1, R1;
RSQ R2.w, R2.w;
MUL R3.xyz, R2.w, R1;
MUL R2.xyz, R2.x, R0;
ARL A0.x, R5.x;
MUL R4.xyz, R2.zxyw, c[A0.x + 10].yzxw;
MAD R2.xyz, R2.yzxw, c[A0.x + 10].zxyw, -R4;
ARL A0.x, R4.w;
MUL R4.xyz, R3.zxyw, c[A0.x + 10].yzxw;
MUL R1, vertex.attrib[1].z, R1;
MAD R3.xyz, R3.yzxw, c[A0.x + 10].zxyw, -R4;
MOV R3.w, c[A0.x + 9].x;
ARL A0.x, R5.x;
MOV R2.w, c[A0.x + 9].x;
ARL A0.x, R4.w;
MUL R4, vertex.attrib[1].z, c[A0.x + 10];
MUL R3, vertex.attrib[1].z, R3;
MAD R3, vertex.attrib[1].w, R2, R3;
MUL R2, vertex.attrib[0], c[5];
ADD R2, R2, c[8];
DP4 R5.z, R2, R3;
ARL A0.x, R5.x;
MAD R0, vertex.attrib[1].w, R0, R1;
MAD R3, vertex.attrib[1].w, c[A0.x + 10], R4;
DP4 R5.x, R2, R0;
DP4 R5.y, R2, R3;
MOV R5.w, R2;
DP4 R0.x, R5, c[2];
MOV result.position.y, -R0.x;
DP4 result.position.w, R5, c[4];
DP4 result.position.z, R5, c[3];
DP4 result.position.x, R5, c[1];
MOV result.color, vertex.attrib[3];
MOV result.color.secondary, vertex.attrib[4];
MUL result.texcoord[0], vertex.attrib[9], c[6];
MOV result.texcoord[1], vertex.attrib[10];
MOV result.texcoord[2], vertex.attrib[11];
MOV result.texcoord[3], vertex.attrib[12];
END

Have yet to analyze how this gets transformed to TGSI and whatever i965 uses.
Comment 13 Ilia Mirkin 2015-07-01 21:23:44 UTC
Created attachment 116854 [details]
tgsi shader generated by that program

Note that the headers of that program have stuff like

#var float4 c62 :  :  : 1 : 0
#var float4 c69 :  : c[1] : 2 : 1
#var float4 c70 :  : c[2] : 3 : 1
#var float4 c71 :  : c[3] : 4 : 1
#var float4 c72 :  : c[4] : 5 : 1
#var float4 c76 :  : c[5] : 6 : 1
#var float4 c78 :  :  : 7 : 0
#var float4 c79 :  : c[6] : 8 : 1
#var float4 c89 :  : c[7] : 9 : 1
#var float4 c95 :  : c[8] : 10 : 1
#var float4 array[0] :  : c[9] : 11 : 1
...
#var float4 array[164] :  : c[173] : 11 : 1
#const c[0] = 96 4 2 0.5
#const c[174] = 0.0009765625

I haven't the faintest clue what this means or how it changes the constbuf mapping situation. Perhaps not at all.
Comment 14 Ilia Mirkin 2015-07-01 21:30:44 UTC
Erm... ok...

MOV R0.zw, c[A0.x + 9];
MOV R1.x, c[0].w;
ADD R0.x, c[A0.x + 9].y, R1;
FLR R0.y, R0.x;

vs

  0: MAD TEMP[0].xy, IN[1], CONST[7].yyyy, CONST[7].xxxx
  3: MOV TEMP[0].zw, CONST[ADDR[0].x+9]
  7: FLR TEMP[0].y, CONST[0].wwww

Could be that I'm matching the wrong shaders. But this seems highly suspect. Need to see if there's a good way of dumping mesa ir... I wonder if it doesn't notice the write-mask on the MOV R0.zw and thinks that R0 contains the value it wants.
Comment 15 Ilia Mirkin 2015-07-01 21:36:12 UTC
The following patch fully fixes the problem for me (in addition to the color thing obviously). Still need to figure out exactly which opt is broken. Kinda tempted to just remove the whole thing...

diff --git a/src/mesa/program/prog_optimize.c b/src/mesa/program/prog_optimize.c
index f9e9035..8ebdaa8 100644
--- a/src/mesa/program/prog_optimize.c
+++ b/src/mesa/program/prog_optimize.c
@@ -1341,6 +1341,7 @@ void
 _mesa_optimize_program(struct gl_context *ctx, struct gl_program *program)
 {
    GLboolean any_change;
+   return;
 
    _mesa_simplify_cmp(program);
    /* Stop when no modifications were output */
Comment 16 Ilia Mirkin 2015-07-01 22:22:59 UTC
The second issue is fixed for real by

http://patchwork.freedesktop.org/patch/53442/
Comment 17 Béla Gyebrószki 2015-07-02 04:04:05 UTC
(In reply to Ilia Mirkin from comment #16)
> The second issue is fixed for real by
> 
> http://patchwork.freedesktop.org/patch/53442/

Confirming, the patch fixes the remaining issue in the game and models are rendered properly, thanks a lot :)
Comment 18 Ilia Mirkin 2015-07-02 17:39:11 UTC
Both of the patches are now upstream. They missed the last 10.5.x release, but should be in 10.6.2.


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.