Ian Romanick wrote: > Here is a patch that will enable the 3rd texture unit in the Radeon > R100/RV200 driver. I have tested it using multiarb on a Radeon "classic" > DDR and a Radeon M6 (AGP developer board). There are a few issues that I > came across that should be resolved before applying. > > - How should we calculate the maximum texture size? I changed to code based > on some older discussions on the matter. The problem was that the old > method could over-commit texture memory. My impression is that the driver > /must/ avoid the situation where there isn't enough memory to load all the > textures required for a given pass. If AGP texturing is enabled (that > patch should be ready tomorrow *grin*), should AGP size be taken into > consideration? As long as all the textures of a given size can fit > somewhere, it should be acceptable (though not optimal). > > - 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. > - In radeon_vtxfmt_x86.c in radeon_makeX86MultiTextureCoord2fvARB and > radeon_makeX86MultiTextureCoord2fARB, is the if-statement that masks > 'key' a fast path specifically for the case where texture unit 0 & 1 > are enabled? Does it need to be expanded for the 3rd unit? Looking at > the code in radeon_vtxtmp_x86.S, this just doesn't seem right. I'll have to take a look at all the affected codegen. > By the end of this week I hope to have two more Radeon patches (one to fix > one of the crashes on non-TCL hw, and one to enable AGP textures). I've > finally gotten back to cleaning up my old texture manager patch, so I hope > to get that out RSN as well. Please note that the r200 has taken a divergent path with respect to the use of the agp texture memory. In summary, there's a kernel-space manager for this memory heap which allows us to divide up that region of agp space without the fear that it will be 'stolen' by another context, as the existing texture management code allows. This allows me to implement the client agp allocator from GL_NV_vertex_array_range, and will allow a full implementation of that extension presently... ------------------------------------------------------- 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