Bug 91173

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.

Also reproduced with radeonsi hd7870/mesa-git/llvm-svn

reproduced with i965 (IVB)

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.

(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.

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.

(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.

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>

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?

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.

(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

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.

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.

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.

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 */

The second issue is fixed for real by

http://patchwork.freedesktop.org/patch/53442/

(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 :)

Both of the patches are now upstream. They missed the last 10.5.x release, but should be in 10.6.2.