Ian,
thanks a lot for having a look at it.
Am 2003.06.27 03:19:00 +0200 schrieb(en) Ian Romanick:
> Andreas Stenglein wrote:
> > here is a patch that works at least for multiarb.c
> > It is against HEAD from 19 June 2003
> > (I cleaned it up a bit but its not ready for merge:
> > still some questions...)
>
> I've taken a quick peek at this patch, and I have a couple comments. I
> hope to be able to look at it in more detail in the next week or so, but
> I can't make any promises.
>
> > 1) could someone with an 8MB or 16MB Radeon check if the
> > resulting max_texturesize is big enough?
> > (just use mesa's glxinfo: glxinfo -l)
>
> I should be able to test this on an M6 w/8MB next. Keep in mind that
> the amount of available AGP memory also plays a role. If the card only
> has 1MB of available memory for textures, but there is 256MB of AGP
> memory, it probably won't be a limitation.
>
> > 2) could someone try it out with a game/demo that
> > makes use of the 3rd TMU?
>
> Other than multiarb, I think UT2k3 is the only option. Did anyone ever
> get that working on R100? I know that someone (Keith?) finally got it
> working on R200.
I got the ut2003 demo 2206 and it worked on the radeon 7500.
Some lighting/coloring is broken, (I saw some sort of that in
another program) and it tries to use GL_TEXTURE_CUBE_MAP.
Unfortunately it doesnt log which extensions it
would use if they were available.
And it doesnt log the implementation limits.
@daniel:
are there any switches to get more information?
what is being drawn with the 3rd TMU ?
would GL_NV_vertex_array_range really help so much or is
this info of the README.linux outdated?
>
> > 3) could someone with knowledge about the vfmt and codegen
> > stuff have a look on it? especially whether we need those
> > dummys and what should be done in the fast-path and
> > with vertex3f.
>
> That's where I'll try to focus my attention when I look at it deep. At
> first glance (see my notes below), it look pretty good in this respect.
>
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.c Wed
> > Jun 11 00:06:16 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.c Thu Jun 26
> > 17:15:36 2003
> > @@ -314,6 +317,8 @@
> > 12,
> > GL_FALSE );
> >
> > + /* FIXME: we should verify that we dont get limits below the minimum
> > requirements of OpenGL */
> > +
> > ctx->Const.MaxTextureMaxAnisotropy = 16.0;
> >
> > /* No wide points.
>
> Yes and no. If we persist with the current memory sizing scheme, we'll
> need to disable texture units if there is not enough memory to provide
> the required minimum texture size. My opinion, however, is that we
> should bag that scheme altogether. I think we'll leave that particular
> issue for a (slightly) later date...
ok
>
> > @@ -374,13 +379,15 @@
> >
> > _math_matrix_ctr( &rmesa->TexGenMatrix[0] );
> > _math_matrix_ctr( &rmesa->TexGenMatrix[1] );
> > + _math_matrix_ctr( &rmesa->TexGenMatrix[2] );
> > _math_matrix_ctr( &rmesa->tmpmat );
> > _math_matrix_set_identity( &rmesa->TexGenMatrix[0] );
> > _math_matrix_set_identity( &rmesa->TexGenMatrix[1] );
> > + _math_matrix_set_identity( &rmesa->TexGenMatrix[2] );
> > _math_matrix_set_identity( &rmesa->tmpmat );
> >
> > driInitExtensions( ctx, card_extensions, GL_TRUE );
> > - if( rmesa->dri.drmMinor >= 9 || getenv( "RADEON_RECTANGLE_FORCE_ENABLE")) /*
> > FIXME! a.s. */
> > + if( rmesa->dri.drmMinor >= 9)
> > _mesa_enable_extension( ctx, "GL_NV_texture_rectangle");
> > radeonInitDriverFuncs( ctx );
> > radeonInitIoctlFuncs( ctx );
>
> The various bits of texture-rectangle cleanup like this should be
> commited separately from the 3rd TMU stuff. It will make it easier to
> grok the log messages & roll stuff back if needed.
yes, right.. this getenv stuff more or less got merged by accident:
I didnt know if we should bump the version number, so I just included
some test and a "workaround".
I think this cleanup could just be committed.
OTOH I've found some use of MAX_TEXTURE_UNITS (the one defined in mesas config.h)
in the extra layer for NPOT textures (radeon_swtcl.c).
I think RADEON_MAX_TEXTURE_UNITS and maybe ctx->Const.MaxTextureUnits
can be used there. Only keith knows ...
>
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.h
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.h
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.h Wed
> > Jun 11 00:06:16 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_context.h Thu Jun 26
> > 17:22:54 2003
> > @@ -635,30 +636,37 @@
> > GLuint prim;
> > };
> >
> > +/* FIXME: do we really need add. 2 to prevent segfault if someone */
> > +/* specifies GL_TEXTURE3 (esp. for the codegen-path) ? */
> > +#define RADEON_MAX_VERTEX_SIZE 19 /* 17 + 2 */
> > +
>
> If you're going to use a define for this, which is a good idea, you
> should move the commentary about the chosen value out of radeon_vbinfo
> and put it next to the define.
>
> I've done some thinking about this particular fast-path. I believe that
> we should try to keep it as much as possible. I don't think there's any
> compelling reason not to. I would point out two things here.
>
> 1. The extra 2 elements (or MAX_TEXTURE_COORD_ELEMENTS elements, which
> is 2 right now, but will eventually grow to 3) is to give the appearance
> of having a power-of-two number of texture units. This allows some
> optimizations.
>
> 2. This is safe because the texture coordinates are always the last
> elements in a vertex. Therefore the actual vertex data is packed into
> the start of the buffer.
>
> > struct radeon_vbinfo {
> > GLint counter, initial_counter;
> > GLint *dmaptr;
> > void (*notify)( void );
> > GLint vertex_size;
> >
> > - /* A maximum total of 15 elements per vertex: 3 floats for position, 3
> > + /* A maximum total of 17 elements per vertex: 3 floats for position, 3
> > * floats for normal, 4 floats for color, 4 bytes for secondary color,
> > - * 2 floats for each texture unit (4 floats total).
> > + * 2 floats for each texture unit (6 floats total).
> > *
> > - * As soon as the 3rd TMU is supported or cube maps (or 3D textures) are
> > + * As soon as cube maps (or 3D textures) are
> > * supported, this value will grow.
> > *
> > * The position data is never actually stored here, so 3 elements could be
> > * trimmed out of the buffer.
> > */
> > - union { float f; int i; radeon_color_t color; } vertex[15];
> > + union { float f; int i; radeon_color_t color; } vertex[RADEON_MAX_VERTEX_SIZE];
> >
> > GLfloat *normalptr;
> > GLfloat *floatcolorptr;
> > radeon_color_t *colorptr;
> > GLfloat *floatspecptr;
> > radeon_color_t *specptr;
> > - GLfloat *texcoordptr[2];
> > + GLfloat *texcoordptr[4]; /* this should be RADEON_MAX_TEXTURE_UNITS but */
> > + /* the extra one is needed in radeon_vtxfmt_c.c
> > and */
> > + /* in the codegen-path if someone specifies
> > GL_TEXTURE3 */
> > + /* maybe we should just use mesas
> > MAX_TEXTURE_UNITS here */
>
> 4 should be used.
>
> > GLenum *prim; /* &ctx->Driver.CurrentExecPrimitive */
> > GLuint primflags;
>
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_tcl.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_tcl.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_tcl.c Fri May 2
> > 13:01:54 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_tcl.c Thu Jun 19
> > 14:02:42 2003
> > @@ -385,6 +385,16 @@
> > }
> > }
> >
> > + if (ctx->Texture.Unit[2]._ReallyEnabled) {
> > + if (ctx->Texture.Unit[2].TexGenEnabled) {
> > + if (rmesa->TexGenNeedNormals[2]) {
> > + inputs |= VERT_BIT_NORMAL;
> > + }
> > + } else {
> > + inputs |= VERT_BIT_TEX2;
> > + }
> > + }
> > +
>
> I don't remember, but could this be made into a loop?
you are right, yes, maybe. but I thought it isnt a problem for 3TMUs,
only for 6 (r200) ;)
so it could be good to have it here, too.
>
> > stage->inputs = inputs;
> > stage->active = 1;
> > }
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt.c Fri May 2
> > 13:01:56 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt.c Thu Jun 26
> > 17:55:47 2003
> > @@ -626,6 +657,11 @@
> > rmesa->vb.floatspecptr = ctx->Current.Attrib[VERT_ATTRIB_COLOR1];
> > rmesa->vb.texcoordptr[0] = ctx->Current.Attrib[VERT_ATTRIB_TEX0];
> > rmesa->vb.texcoordptr[1] = ctx->Current.Attrib[VERT_ATTRIB_TEX1];
> > + rmesa->vb.texcoordptr[2] = ctx->Current.Attrib[VERT_ATTRIB_TEX2];
> > + rmesa->vb.texcoordptr[3] = ctx->Current.Attrib[VERT_ATTRIB_TEX0]; /* dummy to
> > prevent segfault when someone */
> > + /*
> > specifies GL_TEXTURE3 */
> > + /*
> > question: could we use VERT_ATTRIB_TEX3, too without */
> > + /* the
> > risk of "broken" vtxfmt and codegen path ? */
> >
> > /* Run through and initialize the vertex components in the order
> > * the hardware understands:
>
> It doesn't matter what you use. The spec says that if an app tries to
> see the coord for a non-existant texture unit the results are undefined.
> Over-writting a different texture unit's coordinate or doing nothing
> are both equally valid.
>
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_c.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_c.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_c.c Fri
> > May 2 13:01:56 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_c.c Thu
> > Jun 26 17:57:22 2003
> > @@ -548,12 +548,16 @@
> > * with 0x1F. Masking with 0x1F and then masking with 0x01 is redundant, so
> > * the subtraction has been omitted.
> > */
> > +/* question: should we continue using this and make the texcoordptr 4 elements
> > + * or should we mask and verify that the index doesn't get bigger than
> > 2 ?
> > + * We have this issue in the codegen stuff, too
> > + */
>
> Yes (see my explanation above).
>
> > @@ -736,17 +740,20 @@
> >
> > #define ACTIVE_ST0 RADEON_CP_VC_FRMT_ST0
> > #define ACTIVE_ST1 RADEON_CP_VC_FRMT_ST1
> > -#define ACTIVE_ST_ALL (RADEON_CP_VC_FRMT_ST1|RADEON_CP_VC_FRMT_ST0)
> > +#define ACTIVE_ST2 RADEON_CP_VC_FRMT_ST2
> > +#define ACTIVE_ST_ALL
> > (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1|RADEON_CP_VC_FRMT_ST2)
> >
> > /* Each codegen function should be able to be fully specified by a
> > * subsetted version of rmesa->vb.vertex_format.
> > */
> > + /* question: this is strange... could someone explain ? */
> > #define MASK_NORM (ACTIVE_XYZW)
> > #define MASK_COLOR (MASK_NORM|ACTIVE_NORM)
> > #define MASK_SPEC (MASK_COLOR|ACTIVE_COLOR)
> > #define MASK_ST0 (MASK_SPEC|ACTIVE_SPEC)
> > #define MASK_ST1 (MASK_ST0|ACTIVE_ST0)
> > -#define MASK_ST_ALL (MASK_ST1|ACTIVE_ST1)
> > +#define MASK_ST2 (MASK_ST1|ACTIVE_ST1)
> > +#define MASK_ST_ALL (MASK_ST2|ACTIVE_ST2)
> > #define MASK_VERTEX (MASK_ST_ALL|ACTIVE_FPALPHA)
>
> You'll have to search through the list archives. I remember asking the
> same thing once upon a time, but I don't recall the correct technical
> answer.
>
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_sse.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_sse.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_sse.c Fri
> > May 2 13:01:56 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_sse.c Thu
> > Jun 26 18:58:10 2003
> > @@ -178,13 +178,21 @@
> > if (RADEON_DEBUG & DEBUG_CODEGEN)
> > fprintf(stderr, "%s 0x%08x\n", __FUNCTION__, key );
> >
> > - if ((key & (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1)) ==
> > - (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1)) {
> > - DFN ( _sse_MultiTexCoord2fv, rmesa->vb.dfn_cache.MultiTexCoord2fvARB );
> > - FIXUP(dfn->code, 18, 0xdeadbeef, (int)rmesa->vb.texcoordptr[0]);
> > - } else {
> > - DFN ( _sse_MultiTexCoord2fv_2, rmesa->vb.dfn_cache.MultiTexCoord2fvARB );
> > - FIXUP(dfn->code, 14, 0x0, (int)rmesa->vb.texcoordptr);
> > +/* question: should we just let the case for RADEON_CP_VC_FRMT_ST0 for the */
> > +/* default path? Maybe some programs just use glMultiTexCoord2f(v)ARB for */
> > +/* every TMU, as GL_TEXTURE0_ARB is also allowed */
>
> No. If some other set of texture units (say, 0 and 2 or just 1), are
> enabled, masking the unit with 3 won't put the data in the correct place
> in the vertex buffer. If just TMU 1 is enabled, it's coordinate needs
> to be at the 0th position. Just masking w/3 would put it at the 1st
> position, and the chip will get garbage.
what I tried to ask was, if it would be better to disable the fast-path for the
case RADEON_CP_VC_FRMT_ST0:
(as it was before.)
before TMU3-patch it was more or less:
switch (statement) {
case (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1):
fast-path(); break;
default:
default-path(); break;
}
>
> > + switch (key &
> > (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1|RADEON_CP_VC_FRMT_ST2))
> > + {
> > + case RADEON_CP_VC_FRMT_ST0:
> > + case (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1):
> > + case (RADEON_CP_VC_FRMT_ST0|RADEON_CP_VC_FRMT_ST1|RADEON_CP_VC_FRMT_ST2):
> > + DFN ( _sse_MultiTexCoord2fv, rmesa->vb.dfn_cache.MultiTexCoord2fvARB );
> > + FIXUP(dfn->code, 18, 0xdeadbeef, (int)rmesa->vb.texcoordptr[0]);
> > + break;
> > + default:
> > + DFN ( _sse_MultiTexCoord2fv_2, rmesa->vb.dfn_cache.MultiTexCoord2fvARB );
> > + FIXUP(dfn->code, 14, 0x0, (int)rmesa->vb.texcoordptr);
> > + break;
> > }
> > return dfn;
> > }
> > diff -ru trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_x86.c
> > tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_x86.c
> > --- trunk_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_x86.c Fri
> > May 2 13:01:56 2003
> > +++ tex3_20030619/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_vtxfmt_x86.c Thu
> > Jun 26 18:11:45 2003
> > @@ -75,7 +75,7 @@
> > if (RADEON_DEBUG & DEBUG_CODEGEN)
> > fprintf(stderr, "%s 0x%08x %d\n", __FUNCTION__, key, rmesa->vb.vertex_size
> > );
> >
> > - switch (rmesa->vb.vertex_size) {
> > + switch (rmesa->vb.vertex_size) { /* FIXME: do we need something
> > here for 3rd TMU? */
> > case 4: {
> >
> > DFN ( _x86_Vertex3f_4, rmesa->vb.dfn_cache.Vertex3f );
>
> I don't think we have to do anything special. It should just picked up
> by the default case. We may decide to put a fast path, but I don't
> expect so.
>
greetings,
Andreas
-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100006ave/direct;at.asp_061203_01/01
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
