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.


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...


@@ -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.


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?


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.


+ 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.




-------------------------------------------------------
This SF.Net email is sponsored by: INetU
Attention Web Developers & Consultants: Become An INetU Hosting Partner.
Refer Dedicated Servers. We Manage Them. You Get 10% Monthly Commission!
INetU Dedicated Managed Hosting http://www.inetu.net/partner/index.php
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to