Merge branch 'threading_gone_wrong' into 'master'

[Postprocessing] Fix race condition when techniques are dirtied

See merge request OpenMW/openmw!1979

apps/openmw/mwrender/pingpongcanvas.cpp

@@ -9,8 +9,6 @@ namespace MWRender
 {
     PingPongCanvas::PingPongCanvas(Shader::ShaderManager& shaderManager)
         : mFallbackStateSet(new osg::StateSet)
-        , mQueuedDispatchArray(std::nullopt)
-        , mQueuedDispatchFrameId(0)
     {
         setUseDisplayList(false);
         setUseVertexBufferObjects(true);
@@ -37,9 +35,6 @@ namespace MWRender
 
     void PingPongCanvas::setCurrentFrameData(size_t frameId, fx::DispatchArray&& data)
     {
-        mQueuedDispatchArray = fx::DispatchArray(data);
-        mQueuedDispatchFrameId = !frameId;
-
         mBufferData[frameId].data = std::move(data);
     }
 
@@ -65,20 +60,12 @@ namespace MWRender
 
         auto& bufferData = mBufferData[frameId];
 
-        if (mQueuedDispatchArray && mQueuedDispatchFrameId == frameId)
-        {
-            mBufferData[frameId].data = std::move(mQueuedDispatchArray.value());
-            mQueuedDispatchArray = std::nullopt;
-        }
-
         const auto& data = bufferData.data;
 
         std::vector<size_t> filtered;
 
         filtered.reserve(data.size());
 
-        const fx::DispatchNode::SubPass* resolvePass = nullptr;
-
         for (size_t i = 0; i < data.size(); ++i)
         {
             const auto& node = data[i];
@@ -86,15 +73,6 @@ namespace MWRender
             if (bufferData.mask & node.mFlags)
                 continue;
 
-            for (auto it = node.mPasses.crbegin(); it != node.mPasses.crend(); ++it)
-            {
-                if (!(*it).mRenderTarget)
-                {
-                    resolvePass = &(*it);
-                    break;
-                }
-            }
-
             filtered.push_back(i);
         }
 
@@ -113,10 +91,9 @@ namespace MWRender
             else
                 mLoggedLastError = false;
 
-            mFallbackStateSet->setTextureAttributeAndModes(0, bufferData.sceneTex);
-
             state.pushStateSet(mFallbackStateSet);
             state.apply();
+            state.applyTextureAttribute(0, bufferData.sceneTex);
             viewport->apply(state);
 
             drawGeometry(renderInfo);
@@ -223,7 +200,7 @@ namespace MWRender
 
                     lastApplied = pass.mRenderTarget->getHandle(state.getContextID());;
                 }
-                else if (&pass == resolvePass)
+                else if (pass.mResolve && index == filtered.back())
                 {
                     bindDestinationFbo();
                 }

apps/openmw/mwrender/pingpongcanvas.hpp

@@ -80,9 +80,6 @@ namespace MWRender
         mutable std::array<BufferData, 2> mBufferData;
         mutable std::array<osg::ref_ptr<osg::FrameBufferObject>, 3> mFbos;
 
-        mutable std::optional<fx::DispatchArray> mQueuedDispatchArray;
-        mutable size_t mQueuedDispatchFrameId;
-
         mutable bool mLoggedLastError = false;
     };
 }

apps/openmw/mwrender/postprocessor.cpp

@@ -419,6 +419,8 @@ namespace MWRender
             createTexturesAndCamera(frameId);
             createObjectsForFrame(frameId);
             mDirty = false;
+
+            mPingPongCanvas->setCurrentFrameData(frameId, fx::DispatchArray(mTemplateData));
         }
 
         if ((mNormalsSupported && mNormals != mPrevNormals) || (mPassLights != mPrevPassLights))
@@ -520,7 +522,12 @@ namespace MWRender
         if (!isEnabled())
             return;
 
-        fx::DispatchArray data;
+        size_t frameId = frame() % 2;
+
+        mDirty = true;
+        mDirtyFrameId = !frameId;
+
+        mTemplateData = {};
 
         bool sunglare = true;
         mHDR = false;
@@ -632,12 +639,12 @@ namespace MWRender
                 node.mPasses.emplace_back(std::move(subPass));
             }
 
-            data.emplace_back(std::move(node));
+            node.compile();
+
+            mTemplateData.emplace_back(std::move(node));
         }
 
-        size_t frameId = frame() % 2;
+        mPingPongCanvas->setCurrentFrameData(frameId, fx::DispatchArray(mTemplateData));
-
-        mPingPongCanvas->setCurrentFrameData(frameId, std::move(data));
 
         if (auto hud = MWBase::Environment::get().getWindowManager()->getPostProcessorHud())
             hud->updateTechniques();

apps/openmw/mwrender/postprocessor.hpp

@@ -251,6 +251,8 @@ namespace MWRender
     
         int mWidth;
         int mHeight;
+
+        fx::DispatchArray mTemplateData;
     };
 }
 

components/fx/technique.hpp

@@ -59,9 +59,11 @@ namespace fx
             osg::ref_ptr<osg::StateSet> mStateSet = new osg::StateSet;
             osg::ref_ptr<osg::FrameBufferObject> mRenderTarget;
             osg::ref_ptr<osg::Texture2D> mRenderTexture;
+            bool mResolve = false;
 
             SubPass(const SubPass& other, const osg::CopyOp& copyOp = osg::CopyOp::SHALLOW_COPY)
                 : mStateSet(new osg::StateSet(*other.mStateSet, copyOp))
+                , mResolve(other.mResolve)
             {
                 if (other.mRenderTarget)
                     mRenderTarget = new osg::FrameBufferObject(*other.mRenderTarget, copyOp);
@@ -70,6 +72,18 @@ namespace fx
             }
         };
 
+        void compile()
+        {
+            for (auto rit = mPasses.rbegin(); rit != mPasses.rend(); ++rit)
+            {
+                if (!rit->mRenderTarget)
+                {
+                    rit->mResolve = true;
+                    break;
+                }
+            }
+        }
+
         // not safe to read/write in draw thread
         std::shared_ptr<fx::Technique> mHandle = nullptr;
 

components/fx/widgets.hpp

@@ -87,6 +87,7 @@ namespace fx
                     mValueLabel->setCaption(std::to_string(mValue));
 
                 float range = 0.f;
+                float min = 0.f;
 
                 if (auto uniform = mUniform.lock())
                 {
@@ -94,6 +95,7 @@ namespace fx
                     {
                         uniform->template setValue<UType>(mValue);
                         range = uniform->template getMax<UType>() - uniform->template getMin<UType>();
+                        min = uniform->template getMin<UType>();
                     }
                     else
                     {
@@ -101,10 +103,11 @@ namespace fx
                         uvalue[mIndex] = mValue;
                         uniform->template setValue<UType>(uvalue);
                         range = uniform->template getMax<UType>()[mIndex] - uniform->template getMin<UType>()[mIndex];
+                        min = uniform->template getMin<UType>()[mIndex];
                     }
                 }
 
-                float fill = (range == 0.f) ? 1.f : mValue / range;
+                float fill = (range == 0.f) ? 1.f : (mValue - min) / range;
                 mFill->setRealSize(fill, 1.0);
             }