Double buffer the light StateAttributes and StateSets

Fixes a race condition where the position of a light could jump a frame ahead.

apps/openmw/mwrender/animation.cpp

@@ -1091,8 +1091,7 @@ namespace MWRender
         }
 
         osg::ref_ptr<SceneUtil::LightSource> lightSource = new SceneUtil::LightSource;
-        osg::Light* light = new osg::Light;
+        osg::ref_ptr<osg::Light> light (new osg::Light);
-        lightSource->setLight(light);
         lightSource->setNodeMask(Mask_Lighting);
 
         const MWWorld::Fallback* fallback = MWBase::Environment::get().getWorld()->getFallback();
@@ -1123,6 +1122,8 @@ namespace MWRender
         light->setAmbient(osg::Vec4f(0,0,0,1));
         light->setSpecular(osg::Vec4f(0,0,0,0));
 
+        lightSource->setLight(light);
+
         osg::ref_ptr<SceneUtil::LightController> ctrl (new SceneUtil::LightController);
         ctrl->setDiffuse(light->getDiffuse());
         if (esmLight->mData.mFlags & ESM::Light::Flicker)
@@ -1318,22 +1319,31 @@ namespace MWRender
         }
         else
         {
-            if (!mGlowLight)
+            effect += 3;
+            float radius = effect * 66.f;
+            float linearAttenuation = 0.5f / effect;
+
+            if (!mGlowLight || linearAttenuation != mGlowLight->getLight(0)->getLinearAttenuation())
             {
-                mGlowLight = new SceneUtil::LightSource;
+                if (mGlowLight)
-                mGlowLight->setLight(new osg::Light);
+                {
-                mGlowLight->setNodeMask(Mask_Lighting);
+                    mInsert->removeChild(mGlowLight);
-                osg::Light* light = mGlowLight->getLight();
+                    mGlowLight = NULL;
+                }
+
+                osg::ref_ptr<osg::Light> light (new osg::Light);
                 light->setDiffuse(osg::Vec4f(0,0,0,0));
                 light->setSpecular(osg::Vec4f(0,0,0,0));
                 light->setAmbient(osg::Vec4f(1.5f,1.5f,1.5f,1.f));
+                light->setLinearAttenuation(linearAttenuation);
+
+                mGlowLight = new SceneUtil::LightSource;
+                mGlowLight->setNodeMask(Mask_Lighting);
                 mInsert->addChild(mGlowLight);
+                mGlowLight->setLight(light);
             }
 
-            effect += 3;
+            mGlowLight->setRadius(radius);
-            osg::Light* light = mGlowLight->getLight();
-            mGlowLight->setRadius(effect * 66.f);
-            light->setLinearAttenuation(0.5f/effect);
         }
     }
 

components/sceneutil/lightcontroller.cpp

@@ -118,7 +118,7 @@ namespace SceneUtil
         else if(mType == LT_PulseSlow)
             brightness = 0.7f + pulseAmplitude(mDeltaCount*slow)*0.3f;
 
-        static_cast<SceneUtil::LightSource*>(node)->getLight()->setDiffuse(mDiffuseColor * brightness);
+        static_cast<SceneUtil::LightSource*>(node)->getLight(nv->getTraversalNumber())->setDiffuse(mDiffuseColor * brightness);
     }
 
     void LightController::setDiffuse(osg::Vec4f color)

components/sceneutil/lightmanager.cpp

@@ -98,7 +98,7 @@ namespace SceneUtil
                     throw std::runtime_error("can't find parent LightManager");
             }
 
-            mLightManager->addLight(static_cast<LightSource*>(node), osg::computeLocalToWorld(nv->getNodePath()));
+            mLightManager->addLight(static_cast<LightSource*>(node), osg::computeLocalToWorld(nv->getNodePath()), nv->getTraversalNumber());
 
             traverse(node, nv);
         }
@@ -160,37 +160,42 @@ namespace SceneUtil
         mLightsInViewSpace.clear();
 
         // do an occasional cleanup for orphaned lights
-        if (mStateSetCache.size() > 5000)
+        for (int i=0; i<2; ++i)
-            mStateSetCache.clear();
+        {
+            if (mStateSetCache[i].size() > 5000)
+                mStateSetCache[i].clear();
+        }
     }
 
-    void LightManager::addLight(LightSource* lightSource, const osg::Matrixf& worldMat)
+    void LightManager::addLight(LightSource* lightSource, const osg::Matrixf& worldMat, unsigned int frameNum)
     {
         LightSourceTransform l;
         l.mLightSource = lightSource;
         l.mWorldMatrix = worldMat;
-        lightSource->getLight()->setPosition(osg::Vec4f(worldMat.getTrans().x(),
+        lightSource->getLight(frameNum)->setPosition(osg::Vec4f(worldMat.getTrans().x(),
                                                         worldMat.getTrans().y(),
                                                         worldMat.getTrans().z(), 1.f));
         mLights.push_back(l);
     }
 
-    osg::ref_ptr<osg::StateSet> LightManager::getLightListStateSet(const LightList &lightList)
+    osg::ref_ptr<osg::StateSet> LightManager::getLightListStateSet(const LightList &lightList, unsigned int frameNum)
     {
         // possible optimization: return a StateSet containing all requested lights plus some extra lights (if a suitable one exists)
         size_t hash = 0;
         for (unsigned int i=0; i<lightList.size();++i)
             boost::hash_combine(hash, lightList[i]->mLightSource->getId());
 
-        LightStateSetMap::iterator found = mStateSetCache.find(hash);
+        LightStateSetMap& stateSetCache = mStateSetCache[frameNum%2];
-        if (found != mStateSetCache.end())
+
+        LightStateSetMap::iterator found = stateSetCache.find(hash);
+        if (found != stateSetCache.end())
             return found->second;
         else
         {
 
             std::vector<osg::ref_ptr<osg::Light> > lights;
             for (unsigned int i=0; i<lightList.size();++i)
-                lights.push_back(lightList[i]->mLightSource->getLight());
+                lights.push_back(lightList[i]->mLightSource->getLight(frameNum));
 
             osg::ref_ptr<LightStateAttribute> attr = new LightStateAttribute(mStartLight, lights);
 
@@ -200,7 +205,7 @@ namespace SceneUtil
             stateset->setAttribute(attr, osg::StateAttribute::ON);
             stateset->setAssociatedModes(attr, osg::StateAttribute::ON);
 
-            mStateSetCache.insert(std::make_pair(hash, stateset));
+            stateSetCache.insert(std::make_pair(hash, stateset));
             return stateset;
         }
     }
@@ -348,10 +353,10 @@ namespace SceneUtil
                     while (lightList.size() > maxLights)
                         lightList.pop_back();
                 }
-                stateset = mLightManager->getLightListStateSet(lightList);
+                stateset = mLightManager->getLightListStateSet(lightList, nv->getTraversalNumber());
             }
             else
-                stateset = mLightManager->getLightListStateSet(mLightList);
+                stateset = mLightManager->getLightListStateSet(mLightList, nv->getTraversalNumber());
 
 
             cv->pushStateSet(stateset);

components/sceneutil/lightmanager.hpp

@@ -12,7 +12,7 @@ namespace SceneUtil
     /// LightSource managed by a LightManager.
     class LightSource : public osg::Node
     {
-        osg::ref_ptr<osg::Light> mLight;
+        osg::ref_ptr<osg::Light> mLight[2];
 
         // The activation radius
         float mRadius;
@@ -37,17 +37,24 @@ namespace SceneUtil
             mRadius = radius;
         }
 
-        osg::Light* getLight()
+        /// Get the osg::Light safe for modification in the given frame.
+        osg::Light* getLight(unsigned int frame)
         {
-            return mLight;
+            return mLight[frame % 2];
         }
 
+        /// @warning It is recommended not to replace an existing osg::Light, because there might still be
+        /// references to it in the light StateSet cache that are associated with this LightSource's ID.
+        /// These references will stay valid due to ref_ptr but will point to the old object.
+        /// @warning Do not modify the \a light after you've called this function.
         void setLight(osg::Light* light)
         {
-            mLight = light;
+            mLight[0] = light;
+            mLight[1] = osg::clone(light);
         }
 
-        int getId()
+        /// Get the unique ID for this light source.
+        int getId() const
         {
             return mId;
         }
@@ -77,7 +84,7 @@ namespace SceneUtil
         void update();
 
         // Called automatically by the LightSource's UpdateCallback
-        void addLight(LightSource* lightSource, const osg::Matrixf& worldMat);
+        void addLight(LightSource* lightSource, const osg::Matrixf& worldMat, unsigned int frameNum);
 
         struct LightSourceTransform
         {
@@ -97,7 +104,7 @@ namespace SceneUtil
 
         typedef std::vector<const LightSourceViewBound*> LightList;
 
-        osg::ref_ptr<osg::StateSet> getLightListStateSet(const LightList& lightList);
+        osg::ref_ptr<osg::StateSet> getLightListStateSet(const LightList& lightList, unsigned int frameNum);
 
         /// Set the first light index that should be used by this manager, typically the number of directional lights in the scene.
         void setStartLight(int start);
@@ -113,7 +120,7 @@ namespace SceneUtil
 
         // < Light list hash , StateSet >
         typedef std::map<size_t, osg::ref_ptr<osg::StateSet> > LightStateSetMap;
-        LightStateSetMap mStateSetCache;
+        LightStateSetMap mStateSetCache[2];
 
         int mStartLight;