setRenderTarget(ITexture*) still writes to MRT

You discovered a bug in the engine, and you are sure that it is not a problem of your code? Just post it in here. Please read the bug posting guidelines first.
Post Reply
hendu
Posts: 2600
Joined: Sat Dec 18, 2010 12:53 pm

setRenderTarget(ITexture*) still writes to MRT

Post by hendu »

setRenderTarget(mrt);
draw(part of screen);

setRenderTarget(one rtt of all those included in the mrt)
// Add fix here
draw2(some other part);

In the above sequence draw2 still draws to all the MRT RTTs. If I replace the "add fix here" comment with this:

Code: Select all

GLenum tmp = GL_COLOR_ATTACHMENT0;
glDrawBuffers(1, &tmp);
Then it works properly, ie draw2 only draws to the one buffer it is told to.
hybrid
Admin
Posts: 14143
Joined: Wed Apr 19, 2006 9:20 pm
Location: Oldenburg(Oldb), Germany
Contact:

Re: setRenderTarget(ITexture*) still writes to MRT

Post by hybrid »

I think this only happens if you re-use the first RTT from the MRT. Problem is that we do not re-bind the RTT. Since the texture is also not necessarily in attachment0 we need to re-bind under all circumstances after disabling an MRT as it seems. An even cleaner config phase could be reached if I split those functions up into some more auxiliary functions. For now, fix in revision 4219 should be working.
hendu
Posts: 2600
Joined: Sat Dec 18, 2010 12:53 pm

Re: setRenderTarget(ITexture*) still writes to MRT

Post by hendu »

Thanks, I'll backport it to my 1.7 branch and test later.
hybrid
Admin
Posts: 14143
Joined: Wed Apr 19, 2006 9:20 pm
Location: Oldenburg(Oldb), Germany
Contact:

Re: setRenderTarget(ITexture*) still writes to MRT

Post by hybrid »

Ah yeah, sorry for the inconvenience. As it's a minor fix it would have probably been better to put this into the 1.7 branch indeed. It's just one change in the if line, though, so should be easy to backport.
hendu
Posts: 2600
Joined: Sat Dec 18, 2010 12:53 pm

Re: setRenderTarget(ITexture*) still writes to MRT

Post by hendu »

There's no such if in 1.7, back then the library could trust the user not to call setRenderTarget twice with the same arguments ;)

The backport ended up being a little involved, so I don't know if you want to put it to the official tree.

edit: But it works, yeah.

Code: Select all

Subject: [PATCH] Backport MRT changing fix from trunk
 
---
 include/IVideoDriver.h            |    9 +++++++++
 source/Irrlicht/COpenGLDriver.cpp |   31 +++++++++++++++++++++++++++++++
 source/Irrlicht/COpenGLDriver.h   |    1 +
 3 files changed, 41 insertions(+), 0 deletions(-)
 
diff --git a/include/IVideoDriver.h b/include/IVideoDriver.h
index f6a88e3..7f179c5 100644
--- a/include/IVideoDriver.h
+++ b/include/IVideoDriver.h
@@ -220,6 +220,15 @@ namespace video
                        TargetType(target), ColorMask(colorMask),
                        BlendFuncSrc(blendFuncSrc), BlendFuncDst(blendFuncDst),
                        BlendEnable(blendEnable) {}
+               bool operator!=(const IRenderTarget& other) const
+               {
+                       return ((RenderTexture != other.RenderTexture) ||
+                               (TargetType != other.TargetType) ||
+                               (ColorMask != other.ColorMask) ||
+                               (BlendFuncSrc != other.BlendFuncSrc) ||
+                               (BlendFuncDst != other.BlendFuncDst) ||
+                               (BlendEnable != other.BlendEnable));
+               }
                ITexture* RenderTexture;
                E_RENDER_TARGET TargetType:8;
                E_COLOR_PLANE ColorMask:8;
diff --git a/source/Irrlicht/COpenGLDriver.cpp b/source/Irrlicht/COpenGLDriver.cpp
index 8fd435b..51e7fce 100644
--- a/source/Irrlicht/COpenGLDriver.cpp
+++ b/source/Irrlicht/COpenGLDriver.cpp
@@ -3660,6 +3660,22 @@ bool COpenGLDriver::setRenderTarget(video::ITexture* texture, bool clearBackBuff
                return false;
        }
 
+#if defined(GL_EXT_framebuffer_object)
+       if (CurrentTarget==ERT_MULTI_RENDER_TEXTURES)
+       {
+               for (u32 i=0; i<MRTargets.size(); ++i)
+               {
+                       if (MRTargets[i].TargetType==ERT_RENDER_TEXTURE)
+                       {
+                               for (++i; i<MRTargets.size(); ++i)
+                                       if (MRTargets[i].TargetType==ERT_RENDER_TEXTURE)
+                                               extGlFramebufferTexture2D(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0_EXT+i, GL_TEXTURE_2D, 0, 0);
+                       }
+               }
+               MRTargets.clear();
+       }
+#endif
+
        // check if we should set the previous RT back
 
        setActiveTexture(0, 0);
@@ -3686,6 +3702,8 @@ bool COpenGLDriver::setRenderTarget(video::ITexture* texture, bool clearBackBuff
                CurrentTarget=ERT_FRAME_BUFFER;
                glDrawBuffer(Doublebuffer?GL_BACK_LEFT:GL_FRONT_LEFT);
        }
+       // we need to update the matrices due to the rendersize change.
+       Transformation3DChanged = true;
 
        clearBuffers(clearBackBuffer, clearZBuffer, false, color);
        return true;
@@ -3696,8 +3714,21 @@ bool COpenGLDriver::setRenderTarget(video::ITexture* texture, bool clearBackBuff
 bool COpenGLDriver::setRenderTarget(const core::array<video::IRenderTarget>& targets,
                                bool clearBackBuffer, bool clearZBuffer, SColor color)
 {
+       // if simply disabling the MRT via array call
        if (targets.size()==0)
                return setRenderTarget(0, clearBackBuffer, clearZBuffer, color);
+       // if disabling old MRT, but enabling new one as well
+       if ((MRTargets.size()!=0) && (targets != MRTargets))
+               setRenderTarget(0, clearBackBuffer, clearZBuffer, color);
+       // if no change, simply clear buffers
+       else if (targets == MRTargets)
+       {
+               clearBuffers(clearBackBuffer, clearZBuffer, false, color);
+               return true;
+       }
+
+       // copy to storage for correct disabling
+       MRTargets = targets;
 
        u32 maxMultipleRTTs = core::min_(static_cast<u32>(MaxMultipleRenderTargets), targets.size());
 
diff --git a/source/Irrlicht/COpenGLDriver.h b/source/Irrlicht/COpenGLDriver.h
index b882887..b76477a 100644
--- a/source/Irrlicht/COpenGLDriver.h
+++ b/source/Irrlicht/COpenGLDriver.h
@@ -431,6 +431,7 @@ namespace video
 
                SMaterial Material, LastMaterial;
                COpenGLTexture* RenderTargetTexture;
+               core::array<video::IRenderTarget> MRTargets;
                const ITexture* CurrentTexture[MATERIAL_MAX_TEXTURES];
                core::array<COpenGLFBODepthTexture*> DepthTextures;
                struct SUserClipPlane
-- 
1.7.2.1
 
hybrid
Admin
Posts: 14143
Joined: Wed Apr 19, 2006 9:20 pm
Location: Oldenburg(Oldb), Germany
Contact:

Re: setRenderTarget(ITexture*) still writes to MRT

Post by hybrid »

Well, the change necessary is indeed a different one, but should be also just a one-liner. The glDrawBuffer call in bindRTT of the FBO texture in COpenGLTexture is absent. So just put that one into bindRTT or directly after the bindRTT call in setRenderTarget.
Post Reply