apply same logic to render targets, remove UB

apps/openmw/mwrender/pingpongcanvas.cpp

@@ -196,6 +196,39 @@ namespace MWRender
             }
         };
 
+        // When textures are created (or resized) we need to either dirty them and/or clear them.
+        // Otherwise, there will be undefined behavior when reading from a texture that has yet to be written to in a
+        // later pass.
+        for (const auto& attachment : mDirtyAttachments)
+        {
+            const auto [w, h]
+                = attachment.mSize.get(mTextureScene->getTextureWidth(), mTextureScene->getTextureHeight());
+
+            attachment.mTarget->setTextureSize(w, h);
+            if (attachment.mMipMap)
+                attachment.mTarget->setNumMipmapLevels(osg::Image::computeNumberOfMipmapLevels(w, h));
+            attachment.mTarget->dirtyTextureObject();
+
+            osg::ref_ptr<osg::FrameBufferObject> fbo = new osg::FrameBufferObject;
+
+            fbo->setAttachment(
+                osg::FrameBufferObject::BufferComponent::COLOR_BUFFER0, osg::FrameBufferAttachment(attachment.mTarget));
+            fbo->apply(state, osg::FrameBufferObject::DRAW_FRAMEBUFFER);
+
+            glViewport(0, 0, attachment.mTarget->getTextureWidth(), attachment.mTarget->getTextureHeight());
+            state.haveAppliedAttribute(osg::StateAttribute::VIEWPORT);
+            glClearColor(attachment.mClearColor.r(), attachment.mClearColor.g(), attachment.mClearColor.b(),
+                attachment.mClearColor.a());
+            glClear(GL_COLOR_BUFFER_BIT);
+
+            if (attachment.mTarget->getNumMipmapLevels() > 0)
+            {
+                state.setActiveTextureUnit(0);
+                state.applyTextureAttribute(0, attachment.mTarget);
+                ext->glGenerateMipmap(GL_TEXTURE_2D);
+            }
+        }
+
         for (const size_t& index : filtered)
         {
             const auto& node = mPasses[index];
@@ -239,16 +272,11 @@ namespace MWRender
 
                 if (pass.mRenderTarget)
                 {
-                    if (mDirtyAttachments)
+                    if (mDirtyAttachments.size() > 0)
                     {
                         const auto [w, h]
                             = pass.mSize.get(mTextureScene->getTextureWidth(), mTextureScene->getTextureHeight());
 
-                        pass.mRenderTexture->setTextureSize(w, h);
-                        if (pass.mMipMap)
-                            pass.mRenderTexture->setNumMipmapLevels(osg::Image::computeNumberOfMipmapLevels(w, h));
-                        pass.mRenderTexture->dirtyTextureObject();
-
                         // Custom render targets must be shared between frame ids, so it's impossible to double buffer
                         // without expensive copies. That means the only thread-safe place to resize is in the draw
                         // thread.
@@ -265,7 +293,6 @@ namespace MWRender
 
                     if (pass.mRenderTexture->getNumMipmapLevels() > 0)
                     {
-
                         state.setActiveTextureUnit(0);
                         state.applyTextureAttribute(0,
                             pass.mRenderTarget->getAttachment(osg::FrameBufferObject::BufferComponent::COLOR_BUFFER0)
@@ -336,7 +363,6 @@ namespace MWRender
             bindDestinationFbo();
         }
 
-        if (mDirtyAttachments)
-            mDirtyAttachments = false;
+        mDirtyAttachments.clear();
     }
 }

apps/openmw/mwrender/pingpongcanvas.hpp

@@ -31,7 +31,10 @@ namespace MWRender
 
         void dirty() { mDirty = true; }
 
-        void resizeRenderTargets() { mDirtyAttachments = true; }
+        void setDirtyAttachments(const std::vector<fx::Types::RenderTarget>& attachments)
+        {
+            mDirtyAttachments = attachments;
+        }
 
         const fx::DispatchArray& getPasses() { return mPasses; }
 
@@ -68,7 +71,7 @@ namespace MWRender
         osg::ref_ptr<osg::Texture> mTextureNormals;
 
         mutable bool mDirty = false;
-        mutable bool mDirtyAttachments = false;
+        mutable std::vector<fx::Types::RenderTarget> mDirtyAttachments;
         mutable osg::ref_ptr<osg::Viewport> mRenderViewport;
         mutable osg::ref_ptr<osg::FrameBufferObject> mMultiviewResolveFramebuffer;
         mutable osg::ref_ptr<osg::FrameBufferObject> mDestinationFBO;

apps/openmw/mwrender/postprocessor.cpp

@@ -541,6 +541,8 @@ namespace MWRender
         mNormals = false;
         mPassLights = false;
 
+        std::vector<fx::Types::RenderTarget> attachmentsToDirty;
+
         for (const auto& technique : mTechniques)
         {
             if (!technique->isValid())
@@ -617,7 +619,7 @@ namespace MWRender
 
                 if (!pass->getTarget().empty())
                 {
-                    const auto& renderTarget = technique->getRenderTargetsMap()[pass->getTarget()];
+                    auto& renderTarget = technique->getRenderTargetsMap()[pass->getTarget()];
                     subPass.mSize = renderTarget.mSize;
                     subPass.mRenderTexture = renderTarget.mTarget;
                     subPass.mMipMap = renderTarget.mMipMap;
@@ -628,13 +630,27 @@ namespace MWRender
 
                     const auto [w, h] = renderTarget.mSize.get(renderWidth(), renderHeight());
                     subPass.mStateSet->setAttributeAndModes(new osg::Viewport(0, 0, w, h));
+
+                    if (std::find_if(attachmentsToDirty.cbegin(), attachmentsToDirty.cend(),
+                            [renderTarget](const auto& rt) { return renderTarget.mTarget == rt.mTarget; })
+                        == attachmentsToDirty.cend())
+                    {
+                        attachmentsToDirty.push_back(fx::Types::RenderTarget(renderTarget));
+                    }
                 }
 
                 for (const auto& name : pass->getRenderTargets())
                 {
-                    subPass.mStateSet->setTextureAttribute(subTexUnit, technique->getRenderTargetsMap()[name].mTarget);
+                    auto& renderTarget = technique->getRenderTargetsMap()[name];
+                    subPass.mStateSet->setTextureAttribute(subTexUnit, renderTarget.mTarget);
                     subPass.mStateSet->addUniform(new osg::Uniform(name.c_str(), subTexUnit));
 
+                    if (std::find_if(attachmentsToDirty.cbegin(), attachmentsToDirty.cend(),
+                            [renderTarget](const auto& rt) { return renderTarget.mTarget == rt.mTarget; })
+                        == attachmentsToDirty.cend())
+                    {
+                        attachmentsToDirty.push_back(fx::Types::RenderTarget(renderTarget));
+                    }
                     subTexUnit++;
                 }
 
@@ -654,7 +670,7 @@ namespace MWRender
         mRendering.getSkyManager()->setSunglare(sunglare);
 
         if (dirtyAttachments)
-            mCanvases[frameId]->resizeRenderTargets();
+            mCanvases[frameId]->setDirtyAttachments(attachmentsToDirty);
     }
 
     PostProcessor::Status PostProcessor::enableTechnique(
@@ -668,7 +684,7 @@ namespace MWRender
         int pos = std::min<int>(location.value_or(mTechniques.size()), mTechniques.size());
 
         mTechniques.insert(mTechniques.begin() + pos, technique);
-        dirtyTechniques();
+        dirtyTechniques(Settings::ShaderManager::get().getMode() == Settings::ShaderManager::Mode::Debug);
 
         return Status_Toggled;
     }

components/fx/pass.cpp

@@ -12,7 +12,6 @@
 #include <osg/StateSet>
 
 #include <components/resource/scenemanager.hpp>
-#include <components/sceneutil/clearcolor.hpp>
 #include <components/sceneutil/lightmanager.hpp>
 #include <components/settings/values.hpp>
 #include <components/stereo/multiview.hpp>
@@ -326,9 +325,6 @@ float omw_EstimateFogCoverageFromUV(vec2 uv)
 
         if (mBlendEq)
             stateSet->setAttributeAndModes(new osg::BlendEquation(mBlendEq.value()));
-
-        if (mClearColor)
-            stateSet->setAttributeAndModes(new SceneUtil::ClearColor(mClearColor.value(), GL_COLOR_BUFFER_BIT));
     }
 
     void Pass::dirty()

components/fx/pass.hpp

@@ -72,7 +72,6 @@ namespace fx
         std::array<std::string, 3> mRenderTargets;
 
         std::string mTarget;
-        std::optional<osg::Vec4f> mClearColor;
 
         std::optional<osg::BlendFunc::BlendFuncMode> mBlendSource;
         std::optional<osg::BlendFunc::BlendFuncMode> mBlendDest;

components/fx/technique.cpp

@@ -313,6 +313,8 @@ namespace fx
                 rt.mTarget->setSourceFormat(parseSourceFormat());
             else if (key == "mipmaps")
                 rt.mMipMap = parseBool();
+            else if (key == "clear_color")
+                rt.mClearColor = parseVec<osg::Vec4f, Lexer::Vec4>();
             else
                 error(Misc::StringUtils::format("unexpected key '%s'", std::string(key)));
 
@@ -798,9 +800,6 @@ namespace fx
         if (!pass)
             pass = std::make_shared<fx::Pass>();
 
-        bool clear = true;
-        osg::Vec4f clearColor = { 1, 1, 1, 1 };
-
         while (!isNext<Lexer::Eof>())
         {
             expect<Lexer::Literal>("invalid key in block header");
@@ -844,10 +843,6 @@ namespace fx
                 if (blendEq != osg::BlendEquation::FUNC_ADD)
                     pass->mBlendEq = blendEq;
             }
-            else if (key == "clear")
-                clear = parseBool();
-            else if (key == "clear_color")
-                clearColor = parseVec<osg::Vec4f, Lexer::Vec4>();
             else
                 error(Misc::StringUtils::format("unrecognized key '%s' in block header", std::string(key)));
 
@@ -865,9 +860,6 @@ namespace fx
                 return;
         }
 
-        if (clear)
-            pass->mClearColor = clearColor;
-
         error("malformed block header");
     }
 

components/fx/types.hpp

@@ -63,6 +63,17 @@ namespace fx
             osg::ref_ptr<osg::Texture2D> mTarget = new osg::Texture2D;
             SizeProxy mSize;
             bool mMipMap = false;
+            osg::Vec4f mClearColor = osg::Vec4f(0.0, 0.0, 0.0, 1.0);
+
+            RenderTarget() = default;
+
+            RenderTarget(const RenderTarget& other)
+                : mTarget(other.mTarget)
+                , mSize(other.mSize)
+                , mMipMap(other.mMipMap)
+                , mClearColor(other.mClearColor)
+            {
+            }
         };
 
         template <class T>

docs/source/reference/postprocessing/omwfx.rst

@@ -539,6 +539,8 @@ is not wanted and you want a custom render target.
 +------------------+---------------------+-----------------------------------------------------------------------------+
 | mipmaps          | boolean             | Whether mipmaps should be generated every frame                             |
 +------------------+---------------------+-----------------------------------------------------------------------------+
+| clear_color      | vec4                | The color the texture will be cleared to when it's first created            |
++------------------+---------------------+-----------------------------------------------------------------------------+
 
 To use the render target a pass must be assigned to it, along with any optional blend modes.
 As a restriction, only three render targets can be bound per pass with ``rt1``, ``rt2``, ``rt3``, respectively.
@@ -555,6 +557,7 @@ color buffer will accumulate.
         source_format = rgb;
         internal_format = rgb16f;
         source_type = float;
+        clear_color = vec4(1,0,0,1);
     }
 
     fragment red(target=RT_Red,blend=(add, src_color, one), rt1=RT_Red) {