Bug 21872

Created attachment 26111 [details]
Valid image for TestRGBATexture

"git bisect" led me to this commit:
------------------------------------------------------------------------------
commit ad053d90f01852ee27e36a21402543562bf46ad6
Author: Brian <brian.paul@tungstengraphics.com>
Date:   Wed Oct 17 14:30:18 2007 -0600

    Replace repeat_remainder() with a simpler macro that just casts args to unsigned.
------------------------------------------------------------------------------

In master I re-implemented the macro by restoring repeat_remainder() and forwarding the macro to it.  That fixes the problem.

I've put the patch in-line below, but it removes the optimization attempted by the original commit.  The patch cannot be used as-is because it does not update the comment.

diff --git a/src/mesa/swrast/s_texfilter.c b/src/mesa/swrast/s_texfilter.c
index 0067d3e..546b421 100644
--- a/src/mesa/swrast/s_texfilter.c
+++ b/src/mesa/swrast/s_texfilter.c
@@ -138,8 +138,15 @@ lerp_rgba_3d(GLfloat result[4], GLfloat a, GLfloat b, GLfloat c,
  * If A is a signed integer, A % B doesn't give the right value for A < 0
  * (in terms of texture repeat).  Just casting to unsigned fixes that.
  */
-#define REMAINDER(A, B) ((unsigned) (A) % (unsigned) (B))
-
+static INLINE GLint
+repeat_remainder(GLint a, GLint b)
+{
+   if (a >= 0)
+      return a % b;
+   else
+      return (a + 1) % b + b - 1;
+}
+#define REMAINDER(A, B) repeat_remainder(A, B)
 
 /**
  * Used to compute texel locations for linear sampling.

Hmmm, I could have sworn I thoroughly tested that macro.

Here's a new version to try which avoids the conditional.  The idea is to add a bias to make negative values positive.  It'll fail for very large positive/negative A values but it shouldn't be an issue in practice.

#define REMAINDER(A, B) (((A) + (B) * 1024) % (B))

Let me know if that works for you.

Yes, that works.

Taking the range shift to an extreme, we can handle almost the entire range with the code below.  However, it adds an extra divide instruction since B is not known at compile time, so yours is probably better.

/**
 * If A is a signed integer, A % B doesn't give the right value for A < 0
 * (in terms of texture repeat).  We make A positive by adding a large
 * multiple of B to shift the entire range of GLint into that of GLuint
 * (except possibly the most negative partial block).
 */
#define OFFSET(B) ((((GLuint)0x10000000u)/(B))*(B))
#define REMAINDER(A, B) (((A) + OFFSET(B)) % (B))

I've committed my fix to Mesa, commit ed7f4b42307bff4633689d6781cd3643f10041e5
Thanks for debugging this, Brad.

Mass version move, cvs -> git