Am Do, den 16.12.2004 schrieb Brian Paul um 16:45:
> Felix Kühling wrote:
> > Hi,
> >
> > I noticed some strange rendering artifacts with the Savage that are
> > caused by very large texture coordinates on GL_REPEAT'ed textures. Very
> > large means that it gets noticeable with texture coordinates >255 or
> > <-256. A real world example that exhibits this problem is Torcs with the
> > "Spring" track. Right at the start the effect can be seen very nicely.
> > The track before the start line shows artifacts, directly after the
> > start line everything looks fine.
> >
> > My question is, should I consider such problems an application bug or
> > would it be wise to implement a workaround? I was thinking of
> > implementing a TNL pipeline stage that normalizes texture coordinates.
>
> That could be tricky. If you're thinking of doing something like t' =
> t MOD maxValue, that'll often cause incorrect interpolation.
The trick is that you have to change (add or subtract) all texture
coordinates in one primitive (e.g. triangle) in the same way, that is,
the relative differences between texcoords must not be changed.
I've already hacked up something and the result looks good. It computes
the max and min coordinates per direction and then subtracts
floor((max+min)/2 + 0.5)
from all texture coordinates in the vertex buffer. This is done only for
texture coordinates for which the wrapping mode is GL_REPEAT. I havn't
measured the performance, but I didn't notice a major difference. I
guess the Savage has other bottle-necks. ;-)
A patch is attached for anyone who wants to see the gory details.
>
>
> > I was also wondering if other hardware has similar problems. I'm
> > attaching a small test program that demonstrates the effect and a
> > screenshot of what I get on my ProSavageDDR. With software rendering the
> > output is almost correct. Compile with
> >
> > cc -lGL -lGLU -lglut teximage.c -o teximage
>
> If the hardware implements texcoord interpolation with fixed point you
> can imagine how the bits are used.
>
> If the largest texture dimension is 2048, you'd need at least 11 bits
> to address all texels.
>
> Then you need some sub-texel bits for accurate interpolation and for
> computing the weighting for linear sampling. You probably need 10
> bits there.
>
> Allocate another bit for the sign.
>
> Finally, whatever bits are left in the interpolator will limit the
> maximum coordinate range. If the interpolator is 32 bits, then 32 -
> 11 - 10 - 1 = 10. So the max coordinate would be 2^10 or 1024.
>
> Maybe the savage hardware has a narrow interpolator, or allocates the
> bits differently.
When you increase the texture coordinate offset you can see the
artifacts get worse with every power of two. So you can literally take
away bits available for the interpolation on the Savage by making
texture coordinates bigger. :)
>
> I believe one of the differences between "pro" and "consumer" cards is
> the accuracy and range of interpolators.
Or maybe more expensive hardware does the normalization per
hardware-primitive before it starts interpolating.
>
> -Brian
--
| Felix Kühling <[EMAIL PROTECTED]> http://fxk.de.vu |
| PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 |
? core.2742
? core.2751
? debugfallback.diff
? depend
? diff-20041215
? savage_mesa_20041019.diff
? savage_texnorm.diff
? savagestate.diff
? savagetris.diff
? throttle_and_front.diff
Index: savage_xmesa.c
===================================================================
RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/savage/savage_xmesa.c,v
retrieving revision 1.18
diff -u -r1.18 savage_xmesa.c
--- savage_xmesa.c 15 Dec 2004 17:45:23 -0000 1.18
+++ savage_xmesa.c 16 Dec 2004 21:16:10 -0000
@@ -109,6 +109,22 @@
NULL
};
+extern const struct tnl_pipeline_stage _savage_texnorm_stage;
+
+static const struct tnl_pipeline_stage *savage_pipeline[] = {
+
+ &_tnl_vertex_transform_stage,
+ &_tnl_normal_transform_stage,
+ &_tnl_lighting_stage,
+ &_tnl_fog_coordinate_stage,
+ &_tnl_texgen_stage,
+ &_tnl_texture_transform_stage,
+ &_savage_texnorm_stage,
+ &_tnl_render_stage,
+ 0,
+};
+
+
/* this is first function called in dirver*/
static GLboolean
@@ -455,7 +471,7 @@
/* Install the customized pipeline:
*/
-#if 0
+#if 1
_tnl_destroy_pipeline( ctx );
_tnl_install_pipeline( ctx, savage_pipeline );
#endif
Index: savagetris.c
===================================================================
RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/savage/savagetris.c,v
retrieving revision 1.14
diff -u -r1.14 savagetris.c
--- savagetris.c 14 Dec 2004 22:37:46 -0000 1.14
+++ savagetris.c 16 Dec 2004 21:16:11 -0000
@@ -911,3 +911,189 @@
SAVAGE_CONTEXT(ctx)->verts = (char *)tnl->clipspace.vertex_buf;
}
+
+
+/***
+ * Pipeline stage for texture coordinate normalization
+ * This should probably go somewhere else.
+ ***/
+struct texnorm_stage_data {
+ GLvector4f texcoord[MAX_TEXTURE_UNITS];
+};
+
+#define TEXNORM_STAGE_DATA(stage) ((struct texnorm_stage_data *)stage->privatePtr)
+
+
+static GLboolean run_texnorm_stage( GLcontext *ctx,
+ struct tnl_pipeline_stage *stage )
+{
+ struct texnorm_stage_data *store = TEXNORM_STAGE_DATA(stage);
+ savageContextPtr imesa = SAVAGE_CONTEXT(ctx);
+ TNLcontext *tnl = TNL_CONTEXT(ctx);
+ struct vertex_buffer *VB = &tnl->vb;
+ GLuint i;
+
+ if (imesa->Fallback)
+ return GL_TRUE;
+
+ for (i = 0 ; i < ctx->Const.MaxTextureUnits ; i++) {
+ if (!(stage->inputs & stage->changed_inputs & VERT_BIT_TEX(i)))
+ continue;
+
+ GLuint reallyEnabled = ctx->Texture.Unit[i]._ReallyEnabled;
+ struct gl_texture_object *texObj = ctx->Texture.Unit[i]._Current;
+ GLboolean normalizeS = (texObj->WrapS == GL_REPEAT);
+ GLboolean normalizeT = (reallyEnabled & TEXTURE_2D_BIT) &&
+ (texObj->WrapT == GL_REPEAT);
+ GLfloat *in = (GLfloat *)VB->TexCoordPtr[i]->data;
+ GLint instride = VB->TexCoordPtr[i]->stride;
+ GLfloat (*out)[4] = store->texcoord[i].data;
+ GLint j;
+
+ if (normalizeS && normalizeT) {
+ /* determine extreme values in S and T */
+ GLfloat minS = in[0], maxS = in[0], minT = in[1], maxT = in[1];
+ GLfloat correctionS, correctionT;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ for (j = 1; j < VB->Count; ++j) {
+ if (in[0] < minS) minS = in[0];
+ else if (in[0] > maxS) maxS = in[0];
+ if (in[1] < minT) minT = in[1];
+ else if (in[1] > maxT) maxT = in[1];
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ correctionS = -floor((minS + maxS) * 0.5 + 0.5);
+ correctionT = -floor((minT + maxT) * 0.5 + 0.5);
+ in = (GLfloat *)VB->TexCoordPtr[i]->data;
+ for (j = 0; j < VB->Count; ++j) {
+ out[j][0] = in[0] + correctionS;
+ out[j][1] = in[1] + correctionT;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ } else if (normalizeS) {
+ /* determine extreme values in S */
+ GLfloat minS = in[0], maxS = in[0];
+ GLfloat correctionS;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ for (j = 1; j < VB->Count; ++j) {
+ if (in[0] < minS) minS = in[0];
+ else if (in[0] > maxS) maxS = in[0];
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ correctionS = -floor((minS + maxS) * 0.5 + 0.5);
+ in = (GLfloat *)VB->TexCoordPtr[i]->data;
+ if (reallyEnabled & TEXTURE_2D_BIT) {
+ for (j = 0; j < VB->Count; ++j) {
+ out[j][0] = in[0] + correctionS;
+ out[j][1] = in[1];
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ } else {
+ for (j = 0; j < VB->Count; ++j) {
+ out[j][0] = in[0] + correctionS;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ }
+ } else if (normalizeT) {
+ /* determine extreme values in T */
+ GLfloat minT = in[1], maxT = in[1];
+ GLfloat correctionT;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ for (j = 1; j < VB->Count; ++j) {
+ if (in[1] < minT) minT = in[1];
+ else if (in[1] > maxT) maxT = in[1];
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ correctionT = -floor((minT + maxT) * 0.5 + 0.5);
+ in = (GLfloat *)VB->TexCoordPtr[i]->data;
+ for (j = 0; j < VB->Count; ++j) {
+ out[j][0] = in[0];
+ out[j][1] = in[1] + correctionT;
+ in = (GLfloat *)((GLubyte *)in + instride);
+ }
+ }
+
+ if (normalizeS || normalizeT)
+ VB->AttribPtr[VERT_ATTRIB_TEX0+i] = VB->TexCoordPtr[i] = &store->texcoord[i];
+ }
+
+ return GL_TRUE;
+}
+
+
+/* Called the first time stage->run() is invoked.
+ */
+static GLboolean alloc_texnorm_data( GLcontext *ctx,
+ struct tnl_pipeline_stage *stage )
+{
+ struct vertex_buffer *VB = &TNL_CONTEXT(ctx)->vb;
+ struct texnorm_stage_data *store;
+ GLuint i;
+
+ stage->privatePtr = CALLOC(sizeof(*store));
+ store = TEXNORM_STAGE_DATA(stage);
+ if (!store)
+ return GL_FALSE;
+
+ for (i = 0 ; i < ctx->Const.MaxTextureUnits ; i++)
+ _mesa_vector4f_alloc( &store->texcoord[i], 0, VB->Size, 32 );
+
+ /* Now run the stage.
+ */
+ stage->run = run_texnorm_stage;
+ return stage->run( ctx, stage );
+}
+
+
+static void check_texnorm( GLcontext *ctx,
+ struct tnl_pipeline_stage *stage )
+{
+ GLuint flags = 0;
+
+ if (((ctx->Texture.Unit[0]._ReallyEnabled & (TEXTURE_1D_BIT|TEXTURE_2D_BIT)) &&
+ (ctx->Texture.Unit[0]._Current->WrapS == GL_REPEAT)) ||
+ ((ctx->Texture.Unit[0]._ReallyEnabled & TEXTURE_2D_BIT) &&
+ (ctx->Texture.Unit[0]._Current->WrapT == GL_REPEAT)))
+ flags |= VERT_BIT_TEX0;
+
+ if (((ctx->Texture.Unit[1]._ReallyEnabled & (TEXTURE_1D_BIT|TEXTURE_2D_BIT)) &&
+ (ctx->Texture.Unit[1]._Current->WrapS == GL_REPEAT)) ||
+ ((ctx->Texture.Unit[1]._ReallyEnabled & TEXTURE_2D_BIT) &&
+ (ctx->Texture.Unit[1]._Current->WrapT == GL_REPEAT)))
+ flags |= VERT_BIT_TEX1;
+
+ stage->inputs = flags;
+ stage->outputs = flags;
+ stage->active = (flags != 0);
+}
+
+
+static void free_texnorm_data( struct tnl_pipeline_stage *stage )
+{
+ struct texnorm_stage_data *store = TEXNORM_STAGE_DATA(stage);
+ GLuint i;
+
+ if (store) {
+ for (i = 0 ; i < MAX_TEXTURE_UNITS ; i++)
+ if (store->texcoord[i].data)
+ _mesa_vector4f_free( &store->texcoord[i] );
+ FREE( store );
+ stage->privatePtr = 0;
+ }
+}
+
+
+const struct tnl_pipeline_stage _savage_texnorm_stage =
+{
+ "savage texture coordinate normalization stage", /* name */
+ _NEW_TEXTURE, /* check_state */
+ _NEW_TEXTURE, /* run_state */
+ GL_TRUE, /* active? */
+ 0, /* inputs */
+ 0, /* outputs */
+ 0, /* changed_inputs */
+ NULL, /* private data */
+ free_texnorm_data, /* destructor */
+ check_texnorm, /* check */
+ alloc_texnorm_data, /* run -- initially set to init */
+};
