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;
}
--
Smile! http://antwrp.gsfc.nasa.gov/apod/ap990315.html
-------------------------------------------------------
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
