On Wed, 06 Oct 2004 10:16:23 -0700
Eric Anholt <[EMAIL PROTECTED]> wrote:
> On Wed, 2004-10-06 at 04:54, Felix Kühling wrote:
> > On Mon, 04 Oct 2004 12:09:09 +0100
> > Keith Whitwell <[EMAIL PROTECTED]> wrote:
> >
> > > John,
> > >
> > > I'd say the problem is with these lines in savagetris.c:
> > >
> > >
> > > if (index & (_TNL_BIT_COLOR1|_TNL_BIT_FOG)) {
> > > EMIT_ATTR( _TNL_ATTRIB_COLOR1, EMIT_3UB_3F_BGR, SAVAGE_HW_NO_CS );
> > > EMIT_ATTR( _TNL_ATTRIB_FOG, EMIT_1UB_1F, SAVAGE_HW_NO_CS );
> > > }
> > >
> > > This is a cut and paste of old code from another driver. Have a look at how
> > > other drivers handle this now to avoid trying to emit FOG when only COLOR1 is
> > > enabled, and vice versa.
> >
> > Is there a simpler test case than RTCW? I can't reproduce a segfault
> > with a simple program that draws triangles with diffuse lighting and
> > Fog. John, can you try if the attached patch fixes it?
>
> One thing to note is, what happens if you change from, say,
> specular-without-fog to specular-with-fog? You won't end up doing the
> install_attrs again like you want. For Rage 128, I just stored the
> index in the context and checked if that had changed, instead of the
> vertex format register's value.
You're right. Thanks for catching this. I tried to understand what
force_emit in the i830 driver was for, now I understand. An updated
patch is attached. I'm going to commit that.
>
> --
> Eric Anholt [EMAIL PROTECTED]
> http://people.freebsd.org/~anholt/ [EMAIL PROTECTED]
>
>
| Felix Kühling <[EMAIL PROTECTED]> http://fxk.de.vu |
| PGP Fingerprint: 6A3C 9566 5B30 DDED 73C3 B152 151C 5CC1 D888 E595 |
Index: savagetris.c
===================================================================
RCS file: /cvs/mesa/Mesa/src/mesa/drivers/dri/savage/savagetris.c,v
retrieving revision 1.12
diff -u -r1.12 savagetris.c
--- savagetris.c 1 Jul 2004 13:14:06 -0000 1.12
+++ savagetris.c 6 Oct 2004 19:58:10 -0000
@@ -715,6 +715,15 @@
drawCmd &= ~SKIP; \
} while (0)
+#define EMIT_PAD( N ) \
+do { \
+ imesa->vertex_attrs[imesa->vertex_attr_count].attrib = 0; \
+ imesa->vertex_attrs[imesa->vertex_attr_count].format = EMIT_PAD; \
+ imesa->vertex_attrs[imesa->vertex_attr_count].offset = (N); \
+ imesa->vertex_attr_count++; \
+} while (0)
+
+
static void savageRenderStart( GLcontext *ctx )
{
savageContextPtr imesa = SAVAGE_CONTEXT(ctx);
@@ -736,6 +745,11 @@
*/
if ((index & _TNL_BITS_TEX_ANY) || !(ctx->_TriangleCaps & DD_FLATSHADE)) {
EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, SAVAGE_HW_NO_W );
+ /* We use index below to check if the emit code changed. Since
+ * we're depending on something outside the index here we need
+ * to make sure index reflects the change in the emit
+ * code. Using an otherwise unused bit. */
+ index |= _TNL_BIT_TEX7;
}
else {
EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, 0 );
@@ -745,8 +759,14 @@
EMIT_ATTR( _TNL_ATTRIB_COLOR0, EMIT_4UB_4F_BGRA, SAVAGE_HW_NO_CD );
if (index & (_TNL_BIT_COLOR1|_TNL_BIT_FOG)) {
- EMIT_ATTR( _TNL_ATTRIB_COLOR1, EMIT_3UB_3F_BGR, SAVAGE_HW_NO_CS );
- EMIT_ATTR( _TNL_ATTRIB_FOG, EMIT_1UB_1F, SAVAGE_HW_NO_CS );
+ if ((index & _TNL_BIT_COLOR1))
+ EMIT_ATTR( _TNL_ATTRIB_COLOR1, EMIT_3UB_3F_BGR, SAVAGE_HW_NO_CS );
+ else
+ EMIT_PAD( 3 );
+ if ((index & _TNL_BIT_FOG))
+ EMIT_ATTR( _TNL_ATTRIB_FOG, EMIT_1UB_1F, SAVAGE_HW_NO_CS );
+ else
+ EMIT_PAD( 1 );
}
if (index & _TNL_BIT_TEX(0)) {
@@ -770,19 +790,16 @@
EMIT_ATTR( _TNL_ATTRIB_TEX1, EMIT_1F, SAVAGE_HW_NO_U1 );
}
- /* Only need to change the vertex emit code if there has been a
- * statechange to a new hardware vertex format and also when the
- * vertex format is set for the first time. This is indicated by
- * imesa->vertex_size == 0.
- */
- if (drawCmd != (imesa->DrawPrimitiveCmd & SAVAGE_HW_SKIPFLAGS) ||
- imesa->vertex_size == 0) {
+ /* Need to change the vertex emit code if the SetupIndex changed or
+ * is set for the first time (indicated by vertex_size == 0). */
+ if (index != imesa->SetupIndex || imesa->vertex_size == 0) {
imesa->vertex_size =
_tnl_install_attrs( ctx,
imesa->vertex_attrs,
imesa->vertex_attr_count,
imesa->hw_viewport, 0 );
imesa->vertex_size >>= 2;
+ imesa->SetupIndex = index;
imesa->DrawPrimitiveCmd = drawCmd;
}
@@ -802,7 +819,8 @@
/* Flush the last primitive now, before any state is changed.
* Alternatively state could be emitted in all state-changing
- * functions in savagestate.c. */
+ * functions in savagestate.c and when changing the vertex format
+ * above. */
FLUSH_BATCH(SAVAGE_CONTEXT(ctx));
if (SAVAGE_CONTEXT(ctx)->RenderIndex & SAVAGE_FALLBACK_BIT)
