Bug 89224

Created attachment 113658 [details]
Unigine Valley Mesa-10.4

Attached JPEG image with Unigine Valley incorrect rendering.

Checked with Mesa-10.5rc1. The issue is reproducible.

I found quick fix, but not sure it is completely correct:

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
index 6769310..2ed9ce1 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
@@ -47,6 +47,7 @@ is_direct_copy(vec4_instruction *inst)
    return (inst->opcode == BRW_OPCODE_MOV &&
           !inst->predicate &&
           inst->dst.file == GRF &&
+           !inst->saturate &&
           !inst->dst.reladdr &&
           !inst->src[0].reladdr &&
           inst->dst.type == inst->src[0].type);

Abdiel, can you look at this?  It would be good if a fix for this could land in time for the 10.5 release.

Created attachment 113683 [details] [review]
Valley patch for mesa-10.4

This patch looks more accurately.

I'll look into this.

(In reply to Andrey Sudnik from comment #5)
> Created attachment 113683 [details] [review] [review]
> Valley patch for mesa-10.4
> 
> This patch looks more accurately.

I wrote a small vertex shader to reproduce this:

in vec4 vertex;
out vec4 color;

uniform vec4 a, b;

void main() {
	gl_Position = vertex;
	color.xyz = clamp(a.xyz, 0.0, 1.0);
	color.w = a.w;
}

Without your patch, we simply drop the saturate in copy propagation. I need to come up with a way of testing this other than inspecting the disassembly.

I've sent your patch to the mailing list for review. Thanks!

Thanks for the patch. I've committed it as

commit 0dfec59a2785cf7a87ee5128889ecebe810b611b
Author: Andrey Sudnik <andrey.sudnik@intel.com>
Date:   Thu Mar 5 11:16:49 2015 -0800

    i965/vec4: Don't lose the saturate modifier in copy propagation.
    
    Cc: 10.4, 10.5 <mesa-stable@lists.freedesktop.org>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89224
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

It should end up in the 10.4.6 and 10.5.0 releases.

Thank you Matt!