Problem:

If lighting and color sum are disabled, the secondary color is not used.  If
secondary color is set (via one of the glSecondaryColor3*EXT functions), the
radeon and r200 will segfault.

Detailed description:

This only happens if the vtxfmt paths are enabled.  That is, if either
RADEON_TCL_FORCE_DISABLE or RADEON_NO_VTXFMT are set, the problem will not
occur.  The problem is that when the vtxfmt path is used and the secondary
color is not part of the vertex format, vb.specptr (vb.ubytespecptr in the
R200 driver) is set to NULL.  When this happens and the application tries to
set the secondary color, the NULL pointer will be dereferenced by one of the
{radeon,r200}_SecondaryColor3*EXT functions in {radeon,r200}_vtxfmt_c.c.

Additionally, the context state will not be set.  the glSecondaryColor3*EXT
functions essentially become no-ops in this case.  This is /not/ the way
that the glColor4* functions behave if alpha is not used.  The alpha value
in the GLcontext structure will be set.

Fix:

A floatspecptr needs to be added to the {r200,radeon}_vb structure.  If the
vertex format does not use the secondary color, floatspecptr will point to
ctx->Current.Attrib[VERT_ATTRIB_COLOR1].  Functions need to be added to
{r200,radeon}_vtxfmt_c.c to handle the floating point destination.  A new
chooser macro (called CHOOSE_SECONDARY_COLOR to match CHOOSE_COLOR) needs to
be added to select the correct destination function based on the vertex
format.

Notes:

The changes to the r200 driver are virtually identical to the radeon driver.
However, since I don't have access to R200/RV250 hardware, I have not tested
these changes.  I have verified that the changes to the R100 driver work and
that the R200 driver changes compile.

If this patch looks good to all, and it works on R200, I'll commit it ASAP.

Also attached is a test program that could be bundled w/Mesa. :)
-- 
Smile!  http://antwrp.gsfc.nasa.gov/apod/ap990315.html

Attachment: radeon_secondary_color.patch.gz
Description: application/gunzip

/*
 * (c) Copyright IBM Corporation 2002
 * All Rights Reserved.
 *
 * Permission is hereby granted, free of charge, to any person obtaining a
 * copy of this software and associated documentation files (the "Software"),
 * to deal in the Software without restriction, including without limitation
 * on the rights to use, copy, modify, merge, publish, distribute, sub
 * license, and/or sell copies of the Software, and to permit persons to whom
 * the Software is furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice (including the next
 * paragraph) shall be included in all copies or substantial portions of the
 * Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
 * THE COPYRIGHT HOLDERS AND/OR THEIR SUPPLIERS BE LIABLE FOR ANY CLAIM,
 * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
 * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 * USE OR OTHER DEALINGS IN THE SOFTWARE.
 *
 * Authors:
 *    Ian Romanick <[EMAIL PROTECTED]>
 */

/* Test the behavior of the OpenGL library if state is set that does not use
 * the secondary / specular color, but the secondary color is set.  At the
 * time of this writing (26-Nov-2002) the Radeon & R200 DRI drivers (when
 * TCL & VTXFMT was enabled) would segfault in this situation.
 */

#include <GL/gl.h>
#include <GL/glut.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void
draw( void )
{
    GLubyte    ub_vec[3] = { 0, 0, 0 };
    GLfloat    f_vec[3]  = { 0, 0, 0 };
    GLdouble   d_vec[3]  = { 0, 0, 0 };


    glDisable( GL_COLOR_SUM_EXT );
    glDisable( GL_LIGHTING );
    
    glBegin( GL_TRIANGLES );
    printf( "Calling non-vector versions...\n" );
    glSecondaryColor3ubEXT( 0, 0, 0 );
    glSecondaryColor3fEXT ( 0, 0, 0 );
    glSecondaryColor3dEXT ( 0, 0, 0 );

    printf( "Calling vector versions...\n" );
    glSecondaryColor3ubvEXT( ub_vec );
    glSecondaryColor3fvEXT ( f_vec );
    glSecondaryColor3dvEXT ( d_vec );

    glVertex3f( 0, 0, 0 );
    glVertex3f( 0, 0, 0 );
    glVertex3f( 0, 0, 0 );

    glEnd();

    printf( "Done.\n" );
    exit( EXIT_SUCCESS );
}

int
main( int argc, char ** argv )
{
    glutInit(&argc, argv);
    glutInitDisplayMode(GLUT_RGB | GLUT_DEPTH | GLUT_DOUBLE);

    glutInitWindowPosition(0, 0);
    glutInitWindowSize(300, 300);
    glutCreateWindow("Test");

    glutDisplayFunc(draw);
    glutMainLoop();
    return 0;             /* ANSI C requires main to return int. */
}

Reply via email to