Created attachment 26110 [details] Test Image for test TestTextureRGBA Hello, 6 VTK tests that rely on non-power-of-two texture support fails on some versions of Mesa: Mesa 7.0.4: OK Mesa 7.4.2: broken Mesa git: broken The 6 tests passed with proprietary nVidia drivers 180.44 with GeForce 6 and Quadro FX 3600M. The following 2 tests use a 100x19 RGBA texture: TestTextureRGBA TestTextureRGBADepthPeeling The following 4 tests use a 200x200 RGB texture: TestTStripsColorsTCoords TestTStripsNormalsColorsTCoords TestTStripsNormalsTCoords TestTStripsTCoords Here is a build using Mesa GIT, with the failing tests listed: http://www.cdash.org/CDash/viewTest.php?onlyfailed&buildid=338063 You can click on the failed button to get a detail report of the test, notably a comparison between the test image and the valid image For example, if you click on TestTextureRGBA, you get: http://www.cdash.org/CDash/testDetails.php?test=23560070&build=338063 For example, for test TestTextureRGBA, GL_TEXTURE_MIN_FILTER and GL_TEXTURE_MAG_FILTER are set to GL_LINEAR, GL_TEXTURE_WRAP_S and GL_TEXTURE_WRAP_T to GL_REPEAT.
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
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.