now that the lighting bugs are finally mostly gone, I've just gone ahead and changed the lighting code a bit more... (patch against cvs, without the earlier colormat fix).
This patch causes the driver to no longer use the PREMULT lighting, instead it will use the SOURCE_MATERIAL stuff. Looking at r200_reg.h it looked to me like the chip can handle a lot what is currently done in software. So I changed that ;-)
What's new:
- no more bazillion calls of update_light_colors with lots of color multiplications, since the chip handles that now itself.
- two-side-lighting with different materials no longer causes a tcl fallback (so samples/fog now runs correctly, of course if you use tcl_mode=0 it is still hosed - it looks like the tcl fallback if both fog and two-side lighting are enabled is broken on both radeon and r200, but that's another topic. It was the starting point for this patch however, but I was unable to figure out what's wrong with the fallback. The tnl stuff is scary :-().
- removed the strange fallback if materials between begin / end were discovered. Couldn't figure out why it was there (the comment said the chip handles it just fine?), I thought maybe because material changes caused for instance lighting updates, which now no longer happens. Well so far it didn't lock up...
There are some things I'm unsure about. For one, the update_global_ambient now has an impossible if condition, but I have no idea if the function works correctly now or not - it could also work better than before, who knows ;-). There's also a lot of guesswork involved, but at first glance things seemed to look quite good - the patch passes the nwn and glxgears test ;-) (and some others too, but didn't test extensively yet).
Maybe I missed something important and this lighting code fails in some cases horribly, but if not I think this lighting code would be much nicer (it should likely also help TNL performance quite a bit, unless you run on a P5 10Ghz). The code is also quite a bit simpler than before IMHO, most of the code is just two large copy/paste if statements.
Comments?
Roland
btw from looking at the radeon_reg.h it seems the same changes could be done for the radeon, except the two-sided-lighting with materials which the chip doesn't seem to handle.
Index: r200_state.c =================================================================== RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/r200/r200_state.c,v retrieving revision 1.13 diff -u -r1.13 r200_state.c --- r200_state.c 27 Jan 2004 18:52:40 -0000 1.13 +++ r200_state.c 31 Jan 2004 01:47:57 -0000 @@ -786,6 +786,8 @@ float *fcmd = (float *)R200_DB_STATE( glt );
/* Need to do more if both emmissive & ambient are PREMULT: + * no idea what to do here. But this if condition will never happen currently, + * thus no dependencies on materials now */ if ((rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1] & ((3 << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | @@ -808,9 +810,6 @@ /* Update on change to * - light[p].colors * - light[p].enabled - * - material, - * - colormaterial enabled - * - colormaterial bitmask */ static void update_light_colors( GLcontext *ctx, GLuint p ) { @@ -821,102 +820,111 @@ if (l->Enabled) { r200ContextPtr rmesa = R200_CONTEXT(ctx); float *fcmd = (float *)R200_DB_STATE( lit[p] ); - GLuint bitmask = ctx->Light.ColorMaterialBitmask; - GLfloat (*mat)[4] = ctx->Light.Material.Attrib; COPY_4V( &fcmd[LIT_AMBIENT_RED], l->Ambient ); COPY_4V( &fcmd[LIT_DIFFUSE_RED], l->Diffuse ); COPY_4V( &fcmd[LIT_SPECULAR_RED], l->Specular ); - - if (!ctx->Light.ColorMaterialEnabled) - bitmask = 0; - - if ((bitmask & MAT_BIT_FRONT_AMBIENT) == 0) - SELF_SCALE_3V( &fcmd[LIT_AMBIENT_RED], mat[MAT_ATTRIB_FRONT_AMBIENT] ); - - if ((bitmask & MAT_BIT_FRONT_DIFFUSE) == 0) - SELF_SCALE_3V( &fcmd[LIT_DIFFUSE_RED], mat[MAT_ATTRIB_FRONT_DIFFUSE] ); - - if ((bitmask & MAT_BIT_FRONT_SPECULAR) == 0) - SELF_SCALE_3V( &fcmd[LIT_SPECULAR_RED], mat[MAT_ATTRIB_FRONT_SPECULAR] ); R200_DB_STATECHANGE( rmesa, &rmesa->hw.lit[p] ); } } -/* Also fallback for asym colormaterial mode in twoside lighting... - */ -static void check_twoside_fallback( GLcontext *ctx ) -{ - GLboolean fallback = GL_FALSE; - GLint i; +static void r200ColorMaterial( GLcontext *ctx, GLenum face, GLenum mode ) - if (ctx->Light.Enabled && ctx->Light.Model.TwoSide) { - if (ctx->Light.ColorMaterialEnabled && - (ctx->Light.ColorMaterialBitmask & BACK_MATERIAL_BITS) != - ((ctx->Light.ColorMaterialBitmask & FRONT_MATERIAL_BITS)<<1)) - fallback = GL_TRUE; - else { - for (i = MAT_ATTRIB_FRONT_AMBIENT; i < MAT_ATTRIB_FRONT_INDEXES; i+=2) - if (memcmp( ctx->Light.Material.Attrib[i], - ctx->Light.Material.Attrib[i+1], - sizeof(GLfloat)*4) != 0) { - fallback = GL_TRUE; - break; - } - } - } +{ + r200ContextPtr rmesa = R200_CONTEXT(ctx); + GLuint light_model_ctl1 = rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1]; - TCL_FALLBACK( ctx, R200_TCL_FALLBACK_LIGHT_TWOSIDE, fallback ); -} + light_model_ctl1 &= ~((0xf << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | + (0xf << R200_FRONT_AMBIENT_SOURCE_SHIFT) | + (0xf << R200_FRONT_DIFFUSE_SOURCE_SHIFT) | + (0xf << R200_FRONT_SPECULAR_SOURCE_SHIFT) | + (0xf << R200_BACK_EMISSIVE_SOURCE_SHIFT) | + (0xf << R200_BACK_AMBIENT_SOURCE_SHIFT) | + (0xf << R200_BACK_DIFFUSE_SOURCE_SHIFT) | + (0xf << R200_BACK_SPECULAR_SOURCE_SHIFT)); -static void r200ColorMaterial( GLcontext *ctx, GLenum face, GLenum mode ) -{ if (ctx->Light.ColorMaterialEnabled) { - r200ContextPtr rmesa = R200_CONTEXT(ctx); - GLuint light_model_ctl1 = rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1]; GLuint mask = ctx->Light.ColorMaterialBitmask; - /* Default to PREMULT: - */ - light_model_ctl1 &= ~((0xf << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | - (0xf << R200_FRONT_AMBIENT_SOURCE_SHIFT) | - (0xf << R200_FRONT_DIFFUSE_SOURCE_SHIFT) | - (0xf << R200_FRONT_SPECULAR_SOURCE_SHIFT)); - if (mask & MAT_BIT_FRONT_EMISSION) { - light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << R200_FRONT_EMISSIVE_SOURCE_SHIFT); } + else + light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_0 << + R200_FRONT_EMISSIVE_SOURCE_SHIFT); if (mask & MAT_BIT_FRONT_AMBIENT) { - light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << R200_FRONT_AMBIENT_SOURCE_SHIFT); } - + else + light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_0 << + R200_FRONT_AMBIENT_SOURCE_SHIFT); + if (mask & MAT_BIT_FRONT_DIFFUSE) { - light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << R200_FRONT_DIFFUSE_SOURCE_SHIFT); } - + else + light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_0 << + R200_FRONT_DIFFUSE_SOURCE_SHIFT); + if (mask & MAT_BIT_FRONT_SPECULAR) { - light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << R200_FRONT_SPECULAR_SOURCE_SHIFT); } - - if (light_model_ctl1 != rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1]) { - GLuint p; + else light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_0 << + R200_FRONT_SPECULAR_SOURCE_SHIFT); - R200_STATECHANGE( rmesa, tcl ); - rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1] = light_model_ctl1; + if (mask & MAT_BIT_BACK_EMISSION) { + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + R200_BACK_EMISSIVE_SOURCE_SHIFT); + } + else light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_1 << + R200_BACK_EMISSIVE_SOURCE_SHIFT); - for (p = 0 ; p < MAX_LIGHTS; p++) - update_light_colors( ctx, p ); - update_global_ambient( ctx ); + if (mask & MAT_BIT_BACK_AMBIENT) { + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + R200_BACK_AMBIENT_SOURCE_SHIFT); + } + else light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_1 << + R200_BACK_AMBIENT_SOURCE_SHIFT); + + if (mask & MAT_BIT_BACK_DIFFUSE) { + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + R200_BACK_DIFFUSE_SOURCE_SHIFT); + } + else light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_1 << + R200_BACK_DIFFUSE_SOURCE_SHIFT); + + if (mask & MAT_BIT_BACK_SPECULAR) { + light_model_ctl1 |= (R200_LM1_SOURCE_VERTEX_COLOR_0 << + R200_BACK_SPECULAR_SOURCE_SHIFT); } + else light_model_ctl1 |= (R200_LM1_SOURCE_MATERIAL_1 << + R200_BACK_SPECULAR_SOURCE_SHIFT); + + } + else { + /* Default to SOURCE_MATERIAL: + */ + light_model_ctl1 |= + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_AMBIENT_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_DIFFUSE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_SPECULAR_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_EMISSIVE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_AMBIENT_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_DIFFUSE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_SPECULAR_SOURCE_SHIFT); + } + + if (light_model_ctl1 != rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1]) { + R200_STATECHANGE( rmesa, tcl ); + rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1] = light_model_ctl1; } - - check_twoside_fallback( ctx ); } void r200UpdateMaterial( GLcontext *ctx ) @@ -924,9 +932,11 @@ r200ContextPtr rmesa = R200_CONTEXT(ctx); GLfloat (*mat)[4] = ctx->Light.Material.Attrib; GLfloat *fcmd = (GLfloat *)R200_DB_STATE( mtl[0] ); - GLuint p; + GLfloat *fcmd2 = (GLfloat *)R200_DB_STATE( mtl[1] ); GLuint mask = ~0; - + + /* it shouldn't hurt updating enabled materials. Just ditching all the ifs and updating + all values unconditionally should be much simpler and faster I guess */ if (ctx->Light.ColorMaterialEnabled) mask &= ~ctx->Light.ColorMaterialBitmask; @@ -962,13 +972,39 @@ fcmd[MTL_SHININESS] = mat[MAT_ATTRIB_FRONT_SHININESS][0]; } - R200_DB_STATECHANGE( rmesa, &rmesa->hw.mtl[0] ); + if (mask & MAT_BIT_BACK_EMISSION) { + fcmd2[MTL_EMMISSIVE_RED] = mat[MAT_ATTRIB_BACK_EMISSION][0]; + fcmd2[MTL_EMMISSIVE_GREEN] = mat[MAT_ATTRIB_BACK_EMISSION][1]; + fcmd2[MTL_EMMISSIVE_BLUE] = mat[MAT_ATTRIB_BACK_EMISSION][2]; + fcmd2[MTL_EMMISSIVE_ALPHA] = mat[MAT_ATTRIB_BACK_EMISSION][3]; + } + if (mask & MAT_BIT_BACK_AMBIENT) { + fcmd2[MTL_AMBIENT_RED] = mat[MAT_ATTRIB_BACK_AMBIENT][0]; + fcmd2[MTL_AMBIENT_GREEN] = mat[MAT_ATTRIB_BACK_AMBIENT][1]; + fcmd2[MTL_AMBIENT_BLUE] = mat[MAT_ATTRIB_BACK_AMBIENT][2]; + fcmd2[MTL_AMBIENT_ALPHA] = mat[MAT_ATTRIB_BACK_AMBIENT][3]; + } + if (mask & MAT_BIT_BACK_DIFFUSE) { + fcmd2[MTL_DIFFUSE_RED] = mat[MAT_ATTRIB_BACK_DIFFUSE][0]; + fcmd2[MTL_DIFFUSE_GREEN] = mat[MAT_ATTRIB_BACK_DIFFUSE][1]; + fcmd2[MTL_DIFFUSE_BLUE] = mat[MAT_ATTRIB_BACK_DIFFUSE][2]; + fcmd2[MTL_DIFFUSE_ALPHA] = mat[MAT_ATTRIB_BACK_DIFFUSE][3]; + } + if (mask & MAT_BIT_BACK_SPECULAR) { + fcmd2[MTL_SPECULAR_RED] = mat[MAT_ATTRIB_BACK_SPECULAR][0]; + fcmd2[MTL_SPECULAR_GREEN] = mat[MAT_ATTRIB_BACK_SPECULAR][1]; + fcmd2[MTL_SPECULAR_BLUE] = mat[MAT_ATTRIB_BACK_SPECULAR][2]; + fcmd2[MTL_SPECULAR_ALPHA] = mat[MAT_ATTRIB_BACK_SPECULAR][3]; + } + if (mask & MAT_BIT_BACK_SHININESS) { + fcmd2[MTL_SHININESS] = mat[MAT_ATTRIB_BACK_SHININESS][0]; + } - for (p = 0 ; p < MAX_LIGHTS; p++) - update_light_colors( ctx, p ); + R200_DB_STATECHANGE( rmesa, &rmesa->hw.mtl[0] ); + R200_DB_STATECHANGE( rmesa, &rmesa->hw.mtl[1] ); - check_twoside_fallback( ctx ); - update_global_ambient( ctx ); + /* currently material changes cannot (should they?) trigger a global ambient change + update_global_ambient( ctx ); */ } /* _NEW_LIGHT @@ -1183,16 +1219,11 @@ case GL_LIGHT_MODEL_TWO_SIDE: R200_STATECHANGE( rmesa, tcl ); if (ctx->Light.Model.TwoSide) - rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_0] |= R200_LIGHT_TWOSIDE; + rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_0] |= R200_LIGHT_TWOSIDE | + ( R200_LM0_SOURCE_MATERIAL_1 << R200_BACK_SHININESS_SOURCE_SHIFT ); else - rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_0] &= ~R200_LIGHT_TWOSIDE; - - check_twoside_fallback( ctx ); - - if (rmesa->TclFallback) { - r200ChooseRenderState( ctx ); - r200ChooseVertexState( ctx ); - } + rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_0] &= ~(R200_LIGHT_TWOSIDE| + ( R200_LM0_SOURCE_MATERIAL_1 << R200_BACK_SHININESS_SOURCE_SHIFT )); break; case GL_LIGHT_MODEL_COLOR_CONTROL: @@ -1809,7 +1840,6 @@ case GL_LIGHTING: r200UpdateSpecular(ctx); - check_twoside_fallback( ctx ); break; case GL_LINE_SMOOTH: @@ -2130,29 +2160,10 @@ r200VtxfmtInvalidate( ctx ); } -/* A hack. The r200 can actually cope just fine with materials - * between begin/ends, so fix this. - */ -static GLboolean check_material( GLcontext *ctx ) -{ - TNLcontext *tnl = TNL_CONTEXT(ctx); - GLint i; - - for (i = _TNL_ATTRIB_MAT_FRONT_AMBIENT; - i < _TNL_ATTRIB_MAT_BACK_INDEXES; - i++) - if (tnl->vb.AttribPtr[i] && - tnl->vb.AttribPtr[i]->stride) - return GL_TRUE; - - return GL_FALSE; -} - static void r200WrapRunPipeline( GLcontext *ctx ) { r200ContextPtr rmesa = R200_CONTEXT(ctx); - GLboolean has_material; if (0) fprintf(stderr, "%s, newstate: %x\n", __FUNCTION__, rmesa->NewGLState); @@ -2162,19 +2173,11 @@ if (rmesa->NewGLState) r200ValidateState( ctx ); - has_material = (ctx->Light.Enabled && check_material( ctx )); - - if (has_material) { - TCL_FALLBACK( ctx, R200_TCL_FALLBACK_MATERIAL, GL_TRUE ); - } /* Run the pipeline. */ _tnl_run_pipeline( ctx ); - if (has_material) { - TCL_FALLBACK( ctx, R200_TCL_FALLBACK_MATERIAL, GL_FALSE ); - } } Index: r200_state_init.c =================================================================== RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/r200/r200_state_init.c,v retrieving revision 1.7 diff -u -r1.7 r200_state_init.c --- r200_state_init.c 23 Jan 2004 03:19:47 -0000 1.7 +++ r200_state_init.c 31 Jan 2004 01:48:01 -0000 @@ -247,6 +247,7 @@ ALLOC_STATE( msl, tcl, MSL_STATE_SIZE, "MSL/matrix-select", 0 ); ALLOC_STATE( tcg, tcl, TCG_STATE_SIZE, "TCG/texcoordgen", 0 ); ALLOC_STATE( mtl[0], tcl_lighting, MTL_STATE_SIZE, "MTL0/material0", 0 ); + ALLOC_STATE( mtl[1], tcl_lighting, MTL_STATE_SIZE, "MTL1/material1", 1 ); ALLOC_STATE( grd, tcl, GRD_STATE_SIZE, "GRD/guard-band", 0 ); ALLOC_STATE( fog, fog, FOG_STATE_SIZE, "FOG/fog", 0 ); ALLOC_STATE( glt, tcl_lighting, GLT_STATE_SIZE, "GLT/light-global", 0 ); @@ -314,10 +315,15 @@ rmesa->hw.vtx.cmd[VTX_CMD_1] = cmdpkt(R200_EMIT_OUTPUT_VTX_COMP_SEL); rmesa->hw.vtx.cmd[VTX_CMD_2] = cmdpkt(R200_EMIT_SE_VTX_STATE_CNTL); rmesa->hw.vte.cmd[VTE_CMD_0] = cmdpkt(R200_EMIT_VTE_CNTL); - rmesa->hw.mtl[0].cmd[MTL_CMD_0] = + rmesa->hw.mtl[0].cmd[MTL_CMD_0] = cmdvec( R200_VS_MAT_0_EMISS, 1, 16 ); - rmesa->hw.mtl[0].cmd[MTL_CMD_1] = + rmesa->hw.mtl[0].cmd[MTL_CMD_1] = cmdscl2( R200_SS_MAT_0_SHININESS, 1, 1 ); + rmesa->hw.mtl[1].cmd[MTL_CMD_0] = + cmdvec( R200_VS_MAT_1_EMISS, 1, 16 ); + rmesa->hw.mtl[1].cmd[MTL_CMD_1] = + cmdscl2( R200_SS_MAT_1_SHININESS, 1, 1 ); + rmesa->hw.grd.cmd[GRD_CMD_0] = cmdscl( R200_SS_VERT_GUARD_CLIP_ADJ_ADDR, 1, 4 ); rmesa->hw.fog.cmd[FOG_CMD_0] = @@ -625,14 +631,14 @@ R200_LOCAL_LIGHT_VEC_GL); rmesa->hw.tcl.cmd[TCL_LIGHT_MODEL_CTL_1] = - ((R200_LM1_SOURCE_LIGHT_PREMULT << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_FRONT_AMBIENT_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_FRONT_DIFFUSE_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_FRONT_SPECULAR_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_BACK_EMISSIVE_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_BACK_AMBIENT_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_BACK_DIFFUSE_SOURCE_SHIFT) | - (R200_LM1_SOURCE_LIGHT_PREMULT << R200_BACK_SPECULAR_SOURCE_SHIFT)); + ((R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_EMISSIVE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_AMBIENT_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_DIFFUSE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_0 << R200_FRONT_SPECULAR_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_EMISSIVE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_AMBIENT_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_DIFFUSE_SOURCE_SHIFT) | + (R200_LM1_SOURCE_MATERIAL_1 << R200_BACK_SPECULAR_SOURCE_SHIFT)); rmesa->hw.tcl.cmd[TCL_PER_LIGHT_CTL_0] = 0; /* filled in via callbacks */ rmesa->hw.tcl.cmd[TCL_PER_LIGHT_CTL_1] = 0;