Hi Richard,

I have finally got on to reviewing your changes.  First your analysis
of the problem is correct - both the osg::State's cloning of the
affected StateAttribute types was broken, and the invalidation of the
StateSet positioning of these attributes when there assingnment
number/target values was changed.

I have merge a little bit of your changes to the headers, but not the
changes to the .cpp's.  First up the use of asserts is something that
I don't like - since end users application can load any scene graph
type in an application you don't want it crashing because something
might potentially get out of sync, a warning is far kinder and more
informative.

There was also the opportunity to actually fix the problem of the
invalation of StateSet positioning - by temporarily removing the
attribute from its parent StateSets then changing the assignment
number, then adding them back in - this time in the correct new place.
 The code now looks like this:

void Light::setLightNum(int num)
{
    if (_lightnum==num) return;

    if (_parents.empty())
    {
        _lightnum = num;
        return;
    }

    // take a reference to this clip plane to prevent it from going out of scope
    // when we remove it temporarily from its parents.
    osg::ref_ptr<Light> lightRef = this;

    // copy the parents as they _parents list will be changed by the
subsequent removeAttributes.
    ParentList parents = _parents;

    // remove this attribute from its parents as its position is being changed
    // and would no longer be valid.
    ParentList::iterator itr;
    for(itr = parents.begin();
        itr != parents.end();
        ++itr)
    {
        osg::StateSet* stateset = *itr;
        stateset->removeAttribute(this);
    }

    // assign the hint target
    _lightnum = num;

    // add this attribute back into its original parents with its new position
    for(itr = parents.begin();
        itr != parents.end();
        ++itr)
    {
        osg::StateSet* stateset = *itr;
        stateset->setAttribute(this);
    }
}

I also change the Hint cloneType to use _target, GL_DONT_CARE, as
using cloning the mode won't set it back to default value.

I have now checked these changes in, could you please do an svn update
and review my changes and see if you app now works fine with them.

Cheers,
Robert.

On Oct 23, 2007 7:41 AM, Schmidt, Richard, SDGE1
<[EMAIL PROTECTED]> wrote:
>
>
>
>
> Hi,
>
> if you have problems with multiple lightsources in your scenegraph not
> glEnableling/glDisableing correctly this may be a solution for you.
>
>
>
> First lets recap the error:
>
>
>
> osg::Light* l1 = new osg::Light();
>
> osg::Light* l2 = new osg::Light();
>
>
>
> osg::StateSet* stateSet = new osg::StateSet();
>
> stateSet->setAttributeAndModes( l1 );
>
> stateSet->setAttributeAndModes( l2 );
>
>
>
> l1->setLightNum( 0 ); l2->setLightNum( 1 );
>
>
>
> 1. This won't work, because the stateset caches the type/member of the
> attribute. If you change the member afterwards things will blow up…
>
> 2. Next bug is here that osg caches stateattributes internally, using the
> method cloneType. cloneType however needs to clone the member as well, so
> the glDisable will be called for the correct light.
>
>
>
> Attached you will find a version which fixed (2.) . For 1. the interface osg
> provides should be changed (currently catched using assertions).
>
>
>
> Richard
>
>
> _______________________________________________
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
>
_______________________________________________
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Reply via email to