Ian Romanick wrote: > On Thu, Sep 26, 2002 at 07:20:58AM +0100, Keith Whitwell wrote: > >>Ian Romanick wrote: >> >>>- Do we really need the &3 in radeon_vtxfmt_c.c: >>> >>> static void radeon_MultiTexCoord1fARB( GLenum target, GLfloat s ) >>> { >>>- GLfloat *dest = vb.texcoordptr[(target - GL_TEXTURE0_ARB)&1]; >>>+ GLfloat *dest = vb.texcoordptr[(target - GL_TEXTURE0_ARB)&3]; >>> dest[0] = s; >>> dest[1] = 0; >>> } >>> >>> If we don't need the mask, then the AND instructions should be removed >>> from the assembly stubs in radeon_vtxtmp_x86.S as well. >>> >>We definitely need something here. This code must not crash for bogus values >>of target, but the behaviour is otherwise undefined. In the above code you'd >>want to set up a bogus value for texcoordptr[3] to point to some temporary >>storage, or anywhere at all, really. An alternative is a guard like: >> >>if (target - GL_TEXTURE_0_ARB < 3) >> >>which is slightly more work when looking at the codegen templates. >> > > I've made some changes (and a discovery) here, and I should have a new > version of the patch out for people to review either later today or > tomorrow. My discovery is that the '- GL_TEXTURE0' is useless. The value > for GL_TEXTURE0 is 0x84C0. The low order 5 bits are all 0. For any of the > possible valid values for target, subtracting GL_TEXTURE0 is the same as > masking with 0x1F. Masking with 0x1F followed by a mask with 0x03 (or > 0x01) is redundant. > > My vote is to change the 2-TMU version (in all 6 places) to: > > static void radeon_MultiTexCoord1fARB( GLenum target, GLfloat s ) > { > GLfloat *dest = vb.texcoordptr[target & 1]; > dest[0] = s; > dest[1] = 0; > }
Neat! Keith ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel