From 3ef815661652d241623e5e0810972a147353cb72 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sat, 1 May 2021 17:36:02 -0300 Subject: [PATCH 1/9] Fix FSAA in UI and lower VRAM consumption FSAA was being requested however due to how the compositor was setup, this effect was not taking effect. Additionally, the Compositor setup was improved to lower memory consumption. Originally the setup was taken from Ogre samples which assume they will ultimately be rendering to a window. However this is not the case and thus IGN was creating 3 render targets (two for ping-ponging between postprocess FXs + one for storing the final result) This was optimized so that we only create 2 render targets: two for ping-ponging between postprocess FXs and we pick at runtime which one is storing the final result via the new variable renderTargetResultsIdx Further performance optimizations could be made in Ogre 2.2 to improve unnecessary MSAA resolving when doing postprocess, though considering there's currently only one postprocessing effect (the Gaussian filter) I doubt this optimization would make much of a difference Signed-off-by: Matias N. Goldberg --- .../rendering/ogre2/Ogre2RenderTarget.hh | 33 +++- ogre2/src/Ogre2DepthCamera.cc | 106 ++++++------- ogre2/src/Ogre2RenderTarget.cc | 145 ++++++++++-------- 3 files changed, 157 insertions(+), 127 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index c6aec49cc..06651f641 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -115,8 +115,16 @@ namespace ignition /// \see Camera::SetShadowsNodeDefDirty public: void SetShadowsNodeDefDirty(); + /// \brief Get a pointer to the ogre's internal texture name + /// \param[in] _idx In range [0; 1] + /// \see Ogre2RenderTarget::renderTargetResultsIdx + public: virtual const std::string* InternalTextureName( + size_t _idx) const = 0; + /// \brief Get a pointer to the ogre render target - public: virtual Ogre::RenderTarget *RenderTarget() const = 0; + /// \param[in] _idx In range [0; 1] + /// \see Ogre2RenderTarget::renderTargetResultsIdx + public: virtual Ogre::RenderTarget *RenderTarget(size_t _idx) const = 0; /// \brief Get visibility mask for the viewport associated with this /// render target @@ -133,7 +141,8 @@ namespace ignition Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, const std::string &_baseNode, const std::string &_finalNode, - const std::vector &_renderPasses, bool _recreateNodes); + const std::vector &_renderPasses, + uint8_t &_renderTargetResultsIdx, bool _recreateNodes); /// \brief Update the background color protected: virtual void UpdateBackgroundColor(); @@ -188,7 +197,7 @@ namespace ignition protected: MaterialPtr material; /// \brief Helper class that applies the material to the render target - protected: Ogre2RenderTargetMaterialPtr materialApplicator; + protected: Ogre2RenderTargetMaterialPtr materialApplicator[2]; /// \brief Flag to indicate if the render target color has changed protected: bool colorDirty = true; @@ -197,6 +206,10 @@ namespace ignition /// changed protected: bool backgroundMaterialDirty = false; + /// \brief The index to RenderTarget(_idx) where the results are being + /// stored. In range [0; 1] + protected: uint8_t renderTargetResultsIdx = 0u; + /// \brief Anti-aliasing level protected: unsigned int antiAliasing = 4; @@ -230,7 +243,12 @@ namespace ignition public: virtual unsigned int GLId() const override; // Documentation inherited. - public: virtual Ogre::RenderTarget *RenderTarget() const override; + public: virtual const std::string* InternalTextureName( + size_t _idx) const override; + + // Documentation inherited. + public: virtual Ogre::RenderTarget *RenderTarget( + size_t _idx) const override; // Documentation inherited. protected: virtual void RebuildTarget() override; @@ -242,7 +260,9 @@ namespace ignition protected: virtual void BuildTarget(); /// \brief Pointer to the internal ogre render texture object - protected: Ogre::Texture *ogreTexture = nullptr; + /// There's two because we ping pong postprocessing effects + /// and the final result may be in either of them + protected: Ogre::Texture *ogreTexture[2] = {nullptr, nullptr}; /// \brief Make scene our friend so it can create a ogre2 render texture private: friend class Ogre2Scene; @@ -262,7 +282,8 @@ namespace ignition public: virtual void Destroy() override; // Documentation inherited. - public: virtual Ogre::RenderTarget *RenderTarget() const override; + public: virtual Ogre::RenderTarget *RenderTarget( + size_t _idx) const override; // Documentation inherited. protected: virtual void RebuildTarget() override; diff --git a/ogre2/src/Ogre2DepthCamera.cc b/ogre2/src/Ogre2DepthCamera.cc index c45457558..1b23ae72d 100644 --- a/ogre2/src/Ogre2DepthCamera.cc +++ b/ogre2/src/Ogre2DepthCamera.cc @@ -104,7 +104,7 @@ class ignition::rendering::Ogre2DepthCameraPrivate public: Ogre::CompositorWorkspace *ogreCompositorWorkspace = nullptr; /// \brief Output texture with depth and color data - public: Ogre::TexturePtr ogreDepthTexture; + public: Ogre::TexturePtr ogreDepthTexture[2]; /// \brief Dummy render texture for the depth data public: RenderTexturePtr depthTexture; @@ -121,6 +121,10 @@ class ignition::rendering::Ogre2DepthCameraPrivate /// \brief Flag to indicate if render pass need to be rebuilt public: bool renderPassDirty = false; + /// \brief The index to ogreDepthTexture[idx] where the results are being + /// stored. In range [0; 1] + public: uint8_t renderTargetResultsIdx = 0u; + /// \brief Event used to signal rgb point cloud data public: ignition::common::EventTgetCompositorManager2(); // remove depth texture, material, compositor - if (this->dataPtr->ogreDepthTexture) + for( size_t i = 0u; i < 2u; ++i ) { - Ogre::TextureManager::getSingleton().remove( - this->dataPtr->ogreDepthTexture->getName()); + if (this->dataPtr->ogreDepthTexture[i]) + { + Ogre::TextureManager::getSingleton().remove( + this->dataPtr->ogreDepthTexture[i]->getName()); + } } if (this->dataPtr->ogreCompositorWorkspace) { @@ -588,41 +595,11 @@ void Ogre2DepthCamera::CreateDepthTexture() this->dataPtr->ogreCompositorBaseNodeDef = baseNodeDefName; Ogre::CompositorNodeDef *baseNodeDef = ogreCompMgr->addNodeDefinition(baseNodeDefName); - Ogre::TextureDefinitionBase::TextureDefinition *rt0TexDef = - baseNodeDef->addTextureDefinition("rt0"); - rt0TexDef->textureType = Ogre::TEX_TYPE_2D; - rt0TexDef->width = 0; - rt0TexDef->height = 0; - rt0TexDef->depth = 1; - rt0TexDef->numMipmaps = 0; - rt0TexDef->widthFactor = 1; - rt0TexDef->heightFactor = 1; - rt0TexDef->formatList = {Ogre::PF_FLOAT32_RGBA}; - rt0TexDef->fsaa = 0; - rt0TexDef->uav = false; - rt0TexDef->automipmaps = false; - rt0TexDef->hwGammaWrite = Ogre::TextureDefinitionBase::BoolFalse; - rt0TexDef->depthBufferId = Ogre::DepthBuffer::POOL_INVALID; - rt0TexDef->depthBufferFormat = Ogre::PF_UNKNOWN; - rt0TexDef->fsaaExplicitResolve = false; - - Ogre::TextureDefinitionBase::TextureDefinition *rt1TexDef = - baseNodeDef->addTextureDefinition("rt1"); - rt1TexDef->textureType = Ogre::TEX_TYPE_2D; - rt1TexDef->width = 0; - rt1TexDef->height = 0; - rt1TexDef->depth = 1; - rt1TexDef->numMipmaps = 0; - rt1TexDef->widthFactor = 1; - rt1TexDef->heightFactor = 1; - rt1TexDef->formatList = {Ogre::PF_FLOAT32_RGBA}; - rt1TexDef->fsaa = 0; - rt1TexDef->uav = false; - rt1TexDef->automipmaps = false; - rt1TexDef->hwGammaWrite = Ogre::TextureDefinitionBase::BoolFalse; - rt1TexDef->depthBufferId = Ogre::DepthBuffer::POOL_INVALID; - rt1TexDef->depthBufferFormat = Ogre::PF_UNKNOWN; - rt1TexDef->fsaaExplicitResolve = false; + + baseNodeDef->addTextureSourceName( + "rt0", 0u, Ogre::TextureDefinitionBase::TEXTURE_INPUT); + baseNodeDef->addTextureSourceName( + "rt1", 1u, Ogre::TextureDefinitionBase::TEXTURE_INPUT); Ogre::TextureDefinitionBase::TextureDefinition *depthTexDef = baseNodeDef->addTextureDefinition("depthTexture"); @@ -853,10 +830,10 @@ void Ogre2DepthCamera::CreateDepthTexture() Ogre::CompositorNodeDef *finalNodeDef = ogreCompMgr->addNodeDefinition(finalNodeDefName); - // output texture - finalNodeDef->addTextureSourceName("rt_output", 0, + finalNodeDef->addTextureSourceName("rt_input", 0, Ogre::TextureDefinitionBase::TEXTURE_INPUT); - finalNodeDef->addTextureSourceName("rt_input", 1, + // output texture + finalNodeDef->addTextureSourceName("rt_output", 1, Ogre::TextureDefinitionBase::TEXTURE_INPUT); finalNodeDef->setNumTargetPass(1); @@ -892,8 +869,9 @@ void Ogre2DepthCamera::CreateDepthTexture() Ogre::CompositorWorkspaceDef *workDef = ogreCompMgr->addWorkspaceDefinition(wsDefName); - workDef->connect(baseNodeDefName, 0, finalNodeDefName, 1); - workDef->connectExternal(0, finalNodeDefName, 0); + workDef->connectExternal(0, baseNodeDefName, 0); + workDef->connectExternal(1, baseNodeDefName, 1); + workDef->connect(baseNodeDefName, finalNodeDefName); } Ogre::CompositorWorkspaceDef *wsDef = ogreCompMgr->getWorkspaceDefinition(wsDefName); @@ -905,12 +883,21 @@ void Ogre2DepthCamera::CreateDepthTexture() } // create render texture - these textures pack the range data - this->dataPtr->ogreDepthTexture = - Ogre::TextureManager::getSingleton().createManual( - this->Name() + "_depth", "General", Ogre::TEX_TYPE_2D, - this->ImageWidth(), this->ImageHeight(), 1, 0, - Ogre::PF_FLOAT32_RGBA, Ogre::TU_RENDERTARGET, - 0, false, 0, Ogre::BLANKSTRING, false, true); + for( size_t i = 0u; i < 2u; ++i ) + { + this->dataPtr->ogreDepthTexture[i] = + Ogre::TextureManager::getSingleton().createManual( + this->Name() + "_depth" + std::to_string(i), "General", + Ogre::TEX_TYPE_2D, this->ImageWidth(), this->ImageHeight(), 1, 0, + Ogre::PF_FLOAT32_RGBA, Ogre::TU_RENDERTARGET, + 0, false, 0, Ogre::BLANKSTRING, false, true); + + Ogre::RenderTarget *rt = + this->dataPtr->ogreDepthTexture[i]->getBuffer()->getRenderTarget(); + rt->setDepthBufferPool(Ogre::DepthBuffer::POOL_INVALID); + } + + this->dataPtr->renderTargetResultsIdx = 1u; CreateWorkspaceInstance(); } @@ -922,13 +909,19 @@ void Ogre2DepthCamera::CreateWorkspaceInstance() auto ogreRoot = engine->OgreRoot(); Ogre::CompositorManager2 *ogreCompMgr = ogreRoot->getCompositorManager2(); - Ogre::RenderTarget *rt = - this->dataPtr->ogreDepthTexture->getBuffer()->getRenderTarget(); + Ogre::CompositorChannelVec externalTargets(2u); + for( size_t i = 0u; i < 2u; ++i ) + { + externalTargets[i].target = + this->dataPtr->ogreDepthTexture[i]->getBuffer()->getRenderTarget(); + externalTargets[i].textures.push_back(this->dataPtr->ogreDepthTexture[i]); + } // create compositor worksspace this->dataPtr->ogreCompositorWorkspace = ogreCompMgr->addWorkspace(this->scene->OgreSceneManager(), - rt, this->ogreCamera, this->dataPtr->ogreCompositorWorkspaceDef, false); + externalTargets, this->ogreCamera, + this->dataPtr->ogreCompositorWorkspaceDef, false); // add the listener Ogre::CompositorNode *node = @@ -963,7 +956,7 @@ void Ogre2DepthCamera::Render() ////////////////////////////////////////////////// void Ogre2DepthCamera::PreRender() { - if (!this->dataPtr->ogreDepthTexture) + if (!this->dataPtr->ogreDepthTexture[0]) this->CreateDepthTexture(); if (!this->dataPtr->ogreCompositorWorkspace) @@ -976,6 +969,7 @@ void Ogre2DepthCamera::PreRender() this->dataPtr->ogreCompositorBaseNodeDef, this->dataPtr->ogreCompositorFinalNodeDef, this->dataPtr->renderPasses, + this->dataPtr->renderTargetResultsIdx, this->dataPtr->renderPassDirty); for (auto &pass : this->dataPtr->renderPasses) pass->PreRender(); @@ -1028,7 +1022,9 @@ void Ogre2DepthCamera::PostRender() 1, imageFormat, this->dataPtr->depthBuffer); // blit data from gpu to cpu - auto rt = this->dataPtr->ogreDepthTexture->getBuffer()->getRenderTarget(); + auto rt = this->dataPtr->ogreDepthTexture[ + this->dataPtr->renderTargetResultsIdx]-> + getBuffer()->getRenderTarget(); rt->copyContentsToMemory(dstBox, Ogre::RenderTarget::FB_AUTO); if (!this->dataPtr->depthImage) diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index d677c14bb..b04e8107a 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -153,42 +153,10 @@ void Ogre2RenderTarget::BuildCompositor() Ogre::CompositorNodeDef *nodeDef = ogreCompMgr->addNodeDefinition(nodeDefName); - // Input texture - Ogre::TextureDefinitionBase::TextureDefinition *rt0TexDef = - nodeDef->addTextureDefinition("rt0"); - rt0TexDef->textureType = Ogre::TEX_TYPE_2D; - rt0TexDef->width = 0; - rt0TexDef->height = 0; - rt0TexDef->depth = 1; - rt0TexDef->numMipmaps = 0; - rt0TexDef->widthFactor = 1; - rt0TexDef->heightFactor = 1; - rt0TexDef->formatList = {Ogre::PF_R8G8B8}; - rt0TexDef->fsaa = 0; - rt0TexDef->uav = false; - rt0TexDef->automipmaps = false; - rt0TexDef->hwGammaWrite = Ogre::TextureDefinitionBase::BoolTrue; - rt0TexDef->depthBufferId = Ogre::DepthBuffer::POOL_DEFAULT; - rt0TexDef->depthBufferFormat = Ogre::PF_UNKNOWN; - rt0TexDef->fsaaExplicitResolve = false; - - Ogre::TextureDefinitionBase::TextureDefinition *rt1TexDef = - nodeDef->addTextureDefinition("rt1"); - rt1TexDef->textureType = Ogre::TEX_TYPE_2D; - rt1TexDef->width = 0; - rt1TexDef->height = 0; - rt1TexDef->depth = 1; - rt1TexDef->numMipmaps = 0; - rt1TexDef->widthFactor = 1; - rt1TexDef->heightFactor = 1; - rt1TexDef->formatList = {Ogre::PF_R8G8B8}; - rt1TexDef->fsaa = 0; - rt1TexDef->uav = false; - rt1TexDef->automipmaps = false; - rt1TexDef->hwGammaWrite = Ogre::TextureDefinitionBase::BoolTrue; - rt1TexDef->depthBufferId = Ogre::DepthBuffer::POOL_DEFAULT; - rt1TexDef->depthBufferFormat = Ogre::PF_UNKNOWN; - rt1TexDef->fsaaExplicitResolve = false; + nodeDef->addTextureSourceName( + "rt0", 0u, Ogre::TextureDefinitionBase::TEXTURE_INPUT); + nodeDef->addTextureSourceName( + "rt1", 1u, Ogre::TextureDefinitionBase::TEXTURE_INPUT); nodeDef->setNumTargetPass(2); Ogre::CompositorTargetDef *rt0TargetDef = @@ -233,9 +201,9 @@ void Ogre2RenderTarget::BuildCompositor() this->dataPtr->kFinalNodeName; Ogre::CompositorNodeDef *finalNodeDef = ogreCompMgr->addNodeDefinition(finalNodeDefName); - finalNodeDef->addTextureSourceName("rt_output", 0, + finalNodeDef->addTextureSourceName("rtN", 0, Ogre::TextureDefinitionBase::TEXTURE_INPUT); - finalNodeDef->addTextureSourceName("rtN", 1, + finalNodeDef->addTextureSourceName("rt_output", 1, Ogre::TextureDefinitionBase::TEXTURE_INPUT); finalNodeDef->setNumTargetPass(2); @@ -262,15 +230,26 @@ void Ogre2RenderTarget::BuildCompositor() Ogre::CompositorWorkspaceDef *workDef = ogreCompMgr->addWorkspaceDefinition(wsDefName); - workDef->connectExternal(0, finalNodeDefName, 0); - workDef->connect(nodeDefName, 0, finalNodeDefName, 1); + workDef->connectExternal(0, nodeDefName, 0); + workDef->connectExternal(1, nodeDefName, 1); + workDef->connect(nodeDefName, finalNodeDefName); } + auto &manager = Ogre::TextureManager::getSingleton(); + Ogre::CompositorChannelVec externalTargets(2u); + for( size_t i = 0u; i < 2u; ++i ) + { + externalTargets[i].target = this->RenderTarget(i); + externalTargets[i].textures.push_back( + manager.getByName(*this->InternalTextureName(i))); + } this->ogreCompositorWorkspace = ogreCompMgr->addWorkspace(this->scene->OgreSceneManager(), - this->RenderTarget(), this->ogreCamera, + externalTargets, this->ogreCamera, this->ogreCompositorWorkspaceDefName, false); + this->renderTargetResultsIdx = 1u; + this->dataPtr->rtListener = new Ogre2RenderTargetCompositorListener(this); this->ogreCompositorWorkspace->setListener(this->dataPtr->rtListener); } @@ -319,7 +298,8 @@ void Ogre2RenderTarget::Copy(Image &_image) const void *data = _image.Data(); Ogre::PixelFormat imageFormat = Ogre2Conversions::Convert(_image.Format()); Ogre::PixelBox ogrePixelBox(this->width, this->height, 1, imageFormat, data); - this->RenderTarget()->copyContentsToMemory(ogrePixelBox); + this->RenderTarget(this->renderTargetResultsIdx)-> + copyContentsToMemory(ogrePixelBox); } ////////////////////////////////////////////////// @@ -498,7 +478,9 @@ void Ogre2RenderTarget::UpdateRenderPassChain() this->dataPtr->kBaseNodeName, this->ogreCompositorWorkspaceDefName + "/" + this->dataPtr->kFinalNodeName, - this->renderPasses, this->renderPassDirty); + this->renderPasses, + this->renderTargetResultsIdx, + this->renderPassDirty); this->renderPassDirty = false; } @@ -508,7 +490,7 @@ void Ogre2RenderTarget::UpdateRenderPassChain( Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, const std::string &_baseNode, const std::string &_finalNode, const std::vector &_renderPasses, - bool _recreateNodes) + uint8_t &_renderTargetResultsIdx, bool _recreateNodes) { if (!_workspace || _workspaceDefName.empty() || _baseNode.empty() || _finalNode.empty() || _renderPasses.empty()) @@ -564,7 +546,7 @@ void Ogre2RenderTarget::UpdateRenderPassChain( // the first node is the base scene pass node std::string outNodeDefName = _baseNode; // the final compositor node - std::string finalNodeDefName = _finalNode; + const std::string finalNodeDefName = _finalNode; std::string inNodeDefName; // if new nodes need to be added then clear everything, @@ -574,6 +556,8 @@ void Ogre2RenderTarget::UpdateRenderPassChain( else workspaceDef->clearAllInterNodeConnections(); + int numActiveNodes = 0; + // chain the render passes by connecting all the ogre compositor nodes // in between the base scene pass node and the final compositor node for (const auto &pass : _renderPasses) @@ -587,18 +571,25 @@ void Ogre2RenderTarget::UpdateRenderPassChain( { workspaceDef->connect(outNodeDefName, inNodeDefName); outNodeDefName = inNodeDefName; + ++numActiveNodes; } } // connect the last render pass to the final compositor node - workspaceDef->connect(outNodeDefName, 0, finalNodeDefName, 1); + workspaceDef->connect(outNodeDefName, finalNodeDefName); + + if( numActiveNodes & 0x01 ) + _renderTargetResultsIdx = 0; + else + _renderTargetResultsIdx = 1; // if new node definitions were added then recreate all the compositor nodes, // otherwise update the connections if (_recreateNodes) { // clearAll requires the output to be connected again. - workspaceDef->connectExternal(0, finalNodeDefName, 0); + workspaceDef->connectExternal(0, _baseNode, 0); + workspaceDef->connectExternal(1, _baseNode, 1); _workspace->recreateAllNodes(); } else @@ -651,9 +642,12 @@ void Ogre2RenderTarget::RebuildMaterial() Ogre::MaterialPtr matPtr = ogreMaterial->Material(); Ogre::SceneManager *sceneMgr = this->scene->OgreSceneManager(); - Ogre::RenderTarget *target = this->RenderTarget(); - this->materialApplicator.reset(new Ogre2RenderTargetMaterial( - sceneMgr, target, matPtr.get())); + for( size_t i = 0u; i < 2u; ++i ) + { + Ogre::RenderTarget *target = this->RenderTarget(i); + this->materialApplicator[i].reset(new Ogre2RenderTargetMaterial( + sceneMgr, target, matPtr.get())); + } } } @@ -676,9 +670,18 @@ void Ogre2RenderTexture::Destroy() } ////////////////////////////////////////////////// -Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget() const +const std::string* Ogre2RenderTexture::InternalTextureName(size_t _idx) const { - return this->ogreTexture->getBuffer()->getRenderTarget(); + if(nullptr == this->ogreTexture[_idx]) + return nullptr; + + return &this->ogreTexture[_idx]->getName(); +} + +////////////////////////////////////////////////// +Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget(size_t _idx) const +{ + return this->ogreTexture[_idx]->getBuffer()->getRenderTarget(); } @@ -692,18 +695,22 @@ void Ogre2RenderTexture::RebuildTarget() ////////////////////////////////////////////////// void Ogre2RenderTexture::DestroyTarget() { - if (nullptr == this->ogreTexture) + if (nullptr == this->ogreTexture[0]) return; auto &manager = Ogre::TextureManager::getSingleton(); - manager.unload(this->ogreTexture->getName()); - manager.remove(this->ogreTexture->getName()); - // TODO(anyone) there is memory leak when a render texture is destroyed. - // The RenderSystem::_cleanupDepthBuffers method used in ogre1 does not - // seem to work in ogre2 + for( size_t i = 0u; i < 2u; ++i ) + { + manager.unload(this->ogreTexture[i]->getName()); + manager.remove(this->ogreTexture[i]->getName()); - this->ogreTexture = nullptr; + // TODO(anyone) there is memory leak when a render texture is destroyed. + // The RenderSystem::_cleanupDepthBuffers method used in ogre1 does not + // seem to work in ogre2 + + this->ogreTexture[i] = nullptr; + } } ////////////////////////////////////////////////// @@ -734,20 +741,26 @@ void Ogre2RenderTexture::BuildTarget() } } - // Ogre 2 PBS expects gamma correction to be enabled - this->ogreTexture = (manager.createManual(this->name, "General", - Ogre::TEX_TYPE_2D, this->width, this->height, 0, ogreFormat, - Ogre::TU_RENDERTARGET, 0, true, fsaa)).get(); + for( size_t i = 0u; i < 2u; ++i ) + { + // Ogre 2 PBS expects gamma correction to be enabled + // Only the first target uses FSAA + this->ogreTexture[i] = (manager.createManual( + this->name + std::to_string(i), "General", + Ogre::TEX_TYPE_2D, this->width, this->height, 0, ogreFormat, + Ogre::TU_RENDERTARGET, 0, true, i == 0 ? fsaa : 0)).get(); + } } ////////////////////////////////////////////////// unsigned int Ogre2RenderTexture::GLId() const { - if (!this->ogreTexture) + if (!this->ogreTexture[0]) return 0; GLuint texId; - this->ogreTexture->getCustomAttribute("GLID", &texId); + this->ogreTexture[this->renderTargetResultsIdx]-> + getCustomAttribute("GLID", &texId); return static_cast(texId); } @@ -777,7 +790,7 @@ Ogre2RenderWindow::~Ogre2RenderWindow() } ////////////////////////////////////////////////// -Ogre::RenderTarget *Ogre2RenderWindow::RenderTarget() const +Ogre::RenderTarget *Ogre2RenderWindow::RenderTarget(size_t /*_idx*/) const { return this->ogreRenderWindow; } From 400811d7a52c14227b995dcb040b5bfcdc980ed1 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sun, 2 May 2021 19:53:29 -0300 Subject: [PATCH 2/9] Add Ogre2RenderTarget::RenderTarget back Signed-off-by: Matias N. Goldberg --- ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh | 6 ++++++ ogre2/src/Ogre2RenderTarget.cc | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index 06651f641..9037d839c 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -121,6 +121,12 @@ namespace ignition public: virtual const std::string* InternalTextureName( size_t _idx) const = 0; + /// \brief Get a pointer to the ogre render target + /// \remarks Same as calling RenderTarget(this->renderTargetResultsIdx) + /// which is most likely what's intended (i.e. access to the final + /// results of rendering) + public: virtual Ogre::RenderTarget *RenderTarget() const; + /// \brief Get a pointer to the ogre render target /// \param[in] _idx In range [0; 1] /// \see Ogre2RenderTarget::renderTargetResultsIdx diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index b04e8107a..4f1106f30 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -407,6 +407,12 @@ void Ogre2RenderTarget::Render() // engine->OgreRoot()->getRenderSystem()->_update(); } +////////////////////////////////////////////////// +Ogre::RenderTarget *Ogre2RenderTarget::RenderTarget() const +{ + return this->RenderTarget(this->renderTargetResultsIdx); +} + ////////////////////////////////////////////////// uint32_t Ogre2RenderTarget::VisibilityMask() const { From a1c1968f1a2499ae49494e81c5cc4675f06d9567 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sat, 8 May 2021 12:56:03 -0300 Subject: [PATCH 3/9] Rewrote the compositor changes to support RenderWindows As a bonus this new method breaks ABI far less. Fix leak: DestroyCompositor would often not be called Signed-off-by: Matias N. Goldberg --- .../rendering/ogre2/Ogre2RenderTarget.hh | 54 +++----- ogre2/src/Ogre2DepthCamera.cc | 28 ++-- ogre2/src/Ogre2RenderTarget.cc | 124 ++++++++++++------ 3 files changed, 119 insertions(+), 87 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index 9037d839c..60864c057 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -115,22 +115,12 @@ namespace ignition /// \see Camera::SetShadowsNodeDefDirty public: void SetShadowsNodeDefDirty(); - /// \brief Get a pointer to the ogre's internal texture name - /// \param[in] _idx In range [0; 1] - /// \see Ogre2RenderTarget::renderTargetResultsIdx - public: virtual const std::string* InternalTextureName( - size_t _idx) const = 0; - - /// \brief Get a pointer to the ogre render target - /// \remarks Same as calling RenderTarget(this->renderTargetResultsIdx) - /// which is most likely what's intended (i.e. access to the final - /// results of rendering) - public: virtual Ogre::RenderTarget *RenderTarget() const; + /// \brief Returns true if this is a render window + public: virtual bool isRenderWindow() const; - /// \brief Get a pointer to the ogre render target - /// \param[in] _idx In range [0; 1] - /// \see Ogre2RenderTarget::renderTargetResultsIdx - public: virtual Ogre::RenderTarget *RenderTarget(size_t _idx) const = 0; + /// \brief Get a pointer to the ogre render target containing + /// the results of the render + public: virtual Ogre::RenderTarget *RenderTarget() const; /// \brief Get visibility mask for the viewport associated with this /// render target @@ -148,7 +138,9 @@ namespace ignition const std::string &_workspaceDefName, const std::string &_baseNode, const std::string &_finalNode, const std::vector &_renderPasses, - uint8_t &_renderTargetResultsIdx, bool _recreateNodes); + bool _recreateNodes, + Ogre::Texture *(*_ogreTextures)[2], + bool _isRenderWindow); /// \brief Update the background color protected: virtual void UpdateBackgroundColor(); @@ -205,6 +197,13 @@ namespace ignition /// \brief Helper class that applies the material to the render target protected: Ogre2RenderTargetMaterialPtr materialApplicator[2]; + /// \brief Pointer to the internal ogre render texture objects + /// There's two because we ping pong postprocessing effects + /// and the final result is always in ogreTexture[1] + /// RenderWindows may have a 3rd texture which is the + /// actual window + protected: Ogre::Texture *ogreTexture[2] = {nullptr, nullptr}; + /// \brief Flag to indicate if the render target color has changed protected: bool colorDirty = true; @@ -212,10 +211,6 @@ namespace ignition /// changed protected: bool backgroundMaterialDirty = false; - /// \brief The index to RenderTarget(_idx) where the results are being - /// stored. In range [0; 1] - protected: uint8_t renderTargetResultsIdx = 0u; - /// \brief Anti-aliasing level protected: unsigned int antiAliasing = 4; @@ -248,14 +243,6 @@ namespace ignition // Documentation inherited public: virtual unsigned int GLId() const override; - // Documentation inherited. - public: virtual const std::string* InternalTextureName( - size_t _idx) const override; - - // Documentation inherited. - public: virtual Ogre::RenderTarget *RenderTarget( - size_t _idx) const override; - // Documentation inherited. protected: virtual void RebuildTarget() override; @@ -265,11 +252,6 @@ namespace ignition /// \brief Build the render texture protected: virtual void BuildTarget(); - /// \brief Pointer to the internal ogre render texture object - /// There's two because we ping pong postprocessing effects - /// and the final result may be in either of them - protected: Ogre::Texture *ogreTexture[2] = {nullptr, nullptr}; - /// \brief Make scene our friend so it can create a ogre2 render texture private: friend class Ogre2Scene; }; @@ -288,8 +270,10 @@ namespace ignition public: virtual void Destroy() override; // Documentation inherited. - public: virtual Ogre::RenderTarget *RenderTarget( - size_t _idx) const override; + public: virtual bool isRenderWindow() const override; + + // Documentation inherited. + public: virtual Ogre::RenderTarget *RenderTarget() const override; // Documentation inherited. protected: virtual void RebuildTarget() override; diff --git a/ogre2/src/Ogre2DepthCamera.cc b/ogre2/src/Ogre2DepthCamera.cc index 1b23ae72d..60e6b9510 100644 --- a/ogre2/src/Ogre2DepthCamera.cc +++ b/ogre2/src/Ogre2DepthCamera.cc @@ -121,10 +121,6 @@ class ignition::rendering::Ogre2DepthCameraPrivate /// \brief Flag to indicate if render pass need to be rebuilt public: bool renderPassDirty = false; - /// \brief The index to ogreDepthTexture[idx] where the results are being - /// stored. In range [0; 1] - public: uint8_t renderTargetResultsIdx = 0u; - /// \brief Event used to signal rgb point cloud data public: ignition::common::EventTsetDepthBufferPool(Ogre::DepthBuffer::POOL_INVALID); } - this->dataPtr->renderTargetResultsIdx = 1u; - CreateWorkspaceInstance(); } @@ -962,6 +956,12 @@ void Ogre2DepthCamera::PreRender() if (!this->dataPtr->ogreCompositorWorkspace) this->CreateWorkspaceInstance(); + Ogre::Texture *rawDepthTextures[2] = + { + this->dataPtr->ogreDepthTexture[0].get(), + this->dataPtr->ogreDepthTexture[1].get() + }; + // update depth camera render passes Ogre2RenderTarget::UpdateRenderPassChain( this->dataPtr->ogreCompositorWorkspace, @@ -969,8 +969,16 @@ void Ogre2DepthCamera::PreRender() this->dataPtr->ogreCompositorBaseNodeDef, this->dataPtr->ogreCompositorFinalNodeDef, this->dataPtr->renderPasses, - this->dataPtr->renderTargetResultsIdx, - this->dataPtr->renderPassDirty); + this->dataPtr->renderPassDirty, + &rawDepthTextures, + false); + + if (rawDepthTextures[0] != this->dataPtr->ogreDepthTexture[0].get()) + { + std::swap( this->dataPtr->ogreDepthTexture[0], + this->dataPtr->ogreDepthTexture[1] ); + } + for (auto &pass : this->dataPtr->renderPasses) pass->PreRender(); @@ -1022,9 +1030,7 @@ void Ogre2DepthCamera::PostRender() 1, imageFormat, this->dataPtr->depthBuffer); // blit data from gpu to cpu - auto rt = this->dataPtr->ogreDepthTexture[ - this->dataPtr->renderTargetResultsIdx]-> - getBuffer()->getRenderTarget(); + auto rt = this->dataPtr->ogreDepthTexture[1]->getBuffer()->getRenderTarget(); rt->copyContentsToMemory(dstBox, Ogre::RenderTarget::FB_AUTO); if (!this->dataPtr->depthImage) diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index 4f1106f30..7a1b95ecd 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -226,30 +226,43 @@ void Ogre2RenderTarget::BuildCompositor() passScene->mIncludeOverlays = true; passScene->mFirstRQ = 254; passScene->mLastRQ = 255; - } + } Ogre::CompositorWorkspaceDef *workDef = ogreCompMgr->addWorkspaceDefinition(wsDefName); + workDef->connectExternal(0, nodeDefName, 0); workDef->connectExternal(1, nodeDefName, 1); - workDef->connect(nodeDefName, finalNodeDefName); + + if (!this->isRenderWindow()) + { + workDef->connect(nodeDefName, finalNodeDefName); + } + else + { + // connect the last render pass to the final compositor node + // but only input, since output goes to the render window + workDef->connect(nodeDefName, 0, finalNodeDefName, 0); + workDef->connectExternal(2, finalNodeDefName, 1); + } } auto &manager = Ogre::TextureManager::getSingleton(); Ogre::CompositorChannelVec externalTargets(2u); for( size_t i = 0u; i < 2u; ++i ) { - externalTargets[i].target = this->RenderTarget(i); + // Connect them in reverse order + const size_t srcIdx = 2u - i - 1u; + externalTargets[i].target = + this->ogreTexture[srcIdx]->getBuffer()->getRenderTarget(); externalTargets[i].textures.push_back( - manager.getByName(*this->InternalTextureName(i))); + manager.getByName(this->ogreTexture[srcIdx]->getName())); } this->ogreCompositorWorkspace = ogreCompMgr->addWorkspace(this->scene->OgreSceneManager(), externalTargets, this->ogreCamera, this->ogreCompositorWorkspaceDefName, false); - this->renderTargetResultsIdx = 1u; - this->dataPtr->rtListener = new Ogre2RenderTargetCompositorListener(this); this->ogreCompositorWorkspace->setListener(this->dataPtr->rtListener); } @@ -260,6 +273,16 @@ void Ogre2RenderTarget::DestroyCompositor() if (!this->ogreCompositorWorkspace) return; + // Restore the original order so that this->ogreTexture[1] is the one with + // FSAA (which we need for BuildCompositor to connect correctly) + const Ogre::CompositorChannelVec &externalTargets = + this->ogreCompositorWorkspace->getExternalRenderTargets(); + for( size_t i = 0u; i < 2u; ++i ) + { + const size_t srcIdx = (2u - i - 1u); + this->ogreTexture[srcIdx] = externalTargets[i].textures.front().get(); + } + auto engine = Ogre2RenderEngine::Instance(); auto ogreRoot = engine->OgreRoot(); Ogre::CompositorManager2 *ogreCompMgr = ogreRoot->getCompositorManager2(); @@ -298,8 +321,7 @@ void Ogre2RenderTarget::Copy(Image &_image) const void *data = _image.Data(); Ogre::PixelFormat imageFormat = Ogre2Conversions::Convert(_image.Format()); Ogre::PixelBox ogrePixelBox(this->width, this->height, 1, imageFormat, data); - this->RenderTarget(this->renderTargetResultsIdx)-> - copyContentsToMemory(ogrePixelBox); + this->RenderTarget()->copyContentsToMemory(ogrePixelBox); } ////////////////////////////////////////////////// @@ -407,10 +429,16 @@ void Ogre2RenderTarget::Render() // engine->OgreRoot()->getRenderSystem()->_update(); } +////////////////////////////////////////////////// +bool Ogre2RenderTarget::isRenderWindow() const +{ + return false; +} + ////////////////////////////////////////////////// Ogre::RenderTarget *Ogre2RenderTarget::RenderTarget() const { - return this->RenderTarget(this->renderTargetResultsIdx); + return this->ogreTexture[1]->getBuffer()->getRenderTarget(); } ////////////////////////////////////////////////// @@ -485,8 +513,9 @@ void Ogre2RenderTarget::UpdateRenderPassChain() this->ogreCompositorWorkspaceDefName + "/" + this->dataPtr->kFinalNodeName, this->renderPasses, - this->renderTargetResultsIdx, - this->renderPassDirty); + this->renderPassDirty, + &this->ogreTexture, + this->isRenderWindow()); this->renderPassDirty = false; } @@ -496,7 +525,8 @@ void Ogre2RenderTarget::UpdateRenderPassChain( Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, const std::string &_baseNode, const std::string &_finalNode, const std::vector &_renderPasses, - uint8_t &_renderTargetResultsIdx, bool _recreateNodes) + bool _recreateNodes, Ogre::Texture *(*_ogreTextures)[2], + bool _isRenderWindow) { if (!_workspace || _workspaceDefName.empty() || _baseNode.empty() || _finalNode.empty() || _renderPasses.empty()) @@ -581,21 +611,39 @@ void Ogre2RenderTarget::UpdateRenderPassChain( } } - // connect the last render pass to the final compositor node - workspaceDef->connect(outNodeDefName, finalNodeDefName); + workspaceDef->connectExternal(0, _baseNode, 0); + workspaceDef->connectExternal(1, _baseNode, 1); + + if (!_isRenderWindow) + { + // connect the last render pass to the final compositor node + workspaceDef->connect(outNodeDefName, finalNodeDefName); - if( numActiveNodes & 0x01 ) - _renderTargetResultsIdx = 0; + // We must ensure the output is always in ogreTextures[1] + const bool bMustSwapRts = (numActiveNodes & 0x01) == 0u; + + const Ogre::CompositorChannelVec &externalTargets = + _workspace->getExternalRenderTargets(); + for( size_t i = 0u; i < 2u; ++i ) + { + const size_t srcIdx = bMustSwapRts ? (2u - i - 1u) : i; + *_ogreTextures[srcIdx] = externalTargets[i].textures.front().get(); + } + } else - _renderTargetResultsIdx = 1; + { + // connect the last render pass to the final compositor node + // but only input, since output goes to the render window + workspaceDef->connect(outNodeDefName, 0, finalNodeDefName, 0); + workspaceDef->connectExternal(2, _finalNode, 1); + } + // if new node definitions were added then recreate all the compositor nodes, // otherwise update the connections if (_recreateNodes) { // clearAll requires the output to be connected again. - workspaceDef->connectExternal(0, _baseNode, 0); - workspaceDef->connectExternal(1, _baseNode, 1); _workspace->recreateAllNodes(); } else @@ -650,7 +698,8 @@ void Ogre2RenderTarget::RebuildMaterial() Ogre::SceneManager *sceneMgr = this->scene->OgreSceneManager(); for( size_t i = 0u; i < 2u; ++i ) { - Ogre::RenderTarget *target = this->RenderTarget(i); + Ogre::RenderTarget *target = + this->ogreTexture[i]->getBuffer()->getRenderTarget(); this->materialApplicator[i].reset(new Ogre2RenderTargetMaterial( sceneMgr, target, matPtr.get())); } @@ -675,22 +724,6 @@ void Ogre2RenderTexture::Destroy() this->DestroyTarget(); } -////////////////////////////////////////////////// -const std::string* Ogre2RenderTexture::InternalTextureName(size_t _idx) const -{ - if(nullptr == this->ogreTexture[_idx]) - return nullptr; - - return &this->ogreTexture[_idx]->getName(); -} - -////////////////////////////////////////////////// -Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget(size_t _idx) const -{ - return this->ogreTexture[_idx]->getBuffer()->getRenderTarget(); -} - - ////////////////////////////////////////////////// void Ogre2RenderTexture::RebuildTarget() { @@ -704,6 +737,8 @@ void Ogre2RenderTexture::DestroyTarget() if (nullptr == this->ogreTexture[0]) return; + this->DestroyCompositor(); + auto &manager = Ogre::TextureManager::getSingleton(); for( size_t i = 0u; i < 2u; ++i ) @@ -750,11 +785,13 @@ void Ogre2RenderTexture::BuildTarget() for( size_t i = 0u; i < 2u; ++i ) { // Ogre 2 PBS expects gamma correction to be enabled - // Only the first target uses FSAA + // Only the second target uses FSAA. + // Note: It's not guaranteed the 2nd target will remain + // the one using FSAA this->ogreTexture[i] = (manager.createManual( this->name + std::to_string(i), "General", Ogre::TEX_TYPE_2D, this->width, this->height, 0, ogreFormat, - Ogre::TU_RENDERTARGET, 0, true, i == 0 ? fsaa : 0)).get(); + Ogre::TU_RENDERTARGET, 0, true, i == 1u ? fsaa : 0)).get(); } } @@ -765,8 +802,7 @@ unsigned int Ogre2RenderTexture::GLId() const return 0; GLuint texId; - this->ogreTexture[this->renderTargetResultsIdx]-> - getCustomAttribute("GLID", &texId); + this->ogreTexture[1]->getCustomAttribute("GLID", &texId); return static_cast(texId); } @@ -796,7 +832,13 @@ Ogre2RenderWindow::~Ogre2RenderWindow() } ////////////////////////////////////////////////// -Ogre::RenderTarget *Ogre2RenderWindow::RenderTarget(size_t /*_idx*/) const +bool Ogre2RenderWindow::isRenderWindow() const +{ + return true; +} + +////////////////////////////////////////////////// +Ogre::RenderTarget *Ogre2RenderWindow::RenderTarget() const { return this->ogreRenderWindow; } From b52af929a99295ca2eb5e35b321cd6086515a6c5 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sat, 15 May 2021 17:45:48 -0300 Subject: [PATCH 4/9] Mantain ABI compatibility #Ogre2IsRenderWindowABI When merging to newer branches that can break the ABI, revert this commit Signed-off-by: Matias N. Goldberg --- .../ignition/rendering/ogre2/Ogre2RenderTarget.hh | 13 ++++++++++--- ogre2/src/Ogre2RenderTarget.cc | 5 +++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index 60864c057..26c423cf0 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -116,7 +116,11 @@ namespace ignition public: void SetShadowsNodeDefDirty(); /// \brief Returns true if this is a render window - public: virtual bool isRenderWindow() const; + /// TODO(anyone): this function should be virtual. + /// We didn't do it to preserve ABI. + /// Look in commit history for '#Ogre2IsRenderWindowABI' to + /// see changes made and revert + public: bool isRenderWindow() const; /// \brief Get a pointer to the ogre render target containing /// the results of the render @@ -269,8 +273,11 @@ namespace ignition // Documentation inherited. public: virtual void Destroy() override; - // Documentation inherited. - public: virtual bool isRenderWindow() const override; + // TODO(anyone): this function should be virtual. + // We didn't do it to preserve ABI. + // Looks in commit history for '#Ogre2IsRenderWindowABI' to + // see changes made and revert + public: bool isRenderWindow() const; // Documentation inherited. public: virtual Ogre::RenderTarget *RenderTarget() const override; diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index 7a1b95ecd..af0fe6cc6 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -432,6 +432,11 @@ void Ogre2RenderTarget::Render() ////////////////////////////////////////////////// bool Ogre2RenderTarget::isRenderWindow() const { + const Ogre2RenderWindow *asWindow = + dynamic_cast(this); + if (asWindow) + return true; + return false; } From d83468d0a8f5b81903ec80ebdf0003d19c6da7ea Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sun, 23 May 2021 14:32:56 -0300 Subject: [PATCH 5/9] Fix ABI problems I gave up on commit undoing It's clear that on the next release, Ogre2RenderTarget and Ogre2RenderTexture need to be merged together. Signed-off-by: Matias N. Goldberg --- .../rendering/ogre2/Ogre2RenderTarget.hh | 56 ++++- .../ogre2/Ogre2RenderTargetMaterial.hh | 3 + ogre2/src/Ogre2RenderTarget.cc | 234 ++++++++++++------ ogre2/src/Ogre2RenderTargetMaterial.cc | 7 + 4 files changed, 222 insertions(+), 78 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index 26c423cf0..eb75f5069 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -122,6 +122,15 @@ namespace ignition /// see changes made and revert public: bool isRenderWindow() const; + // Documentation inherited + public: unsigned int GLIdImpl() const; + + /// \brief Destroy the render texture + protected: void DestroyTargetImpl(); + + /// \brief Build the render texture + protected: void BuildTargetImpl(); + /// \brief Get a pointer to the ogre render target containing /// the results of the render public: virtual Ogre::RenderTarget *RenderTarget() const; @@ -136,6 +145,14 @@ namespace ignition /// \param[in] _mask Visibility mask public: virtual void SetVisibilityMask(uint32_t _mask); + /// \brief Deprecated. Use other overloads. + public: static IGN_DEPRECATED(5) void UpdateRenderPassChain( + Ogre::CompositorWorkspace *_workspace, + const std::string &_workspaceDefName, + const std::string &_baseNode, const std::string &_finalNode, + const std::vector &_renderPasses, + bool _recreateNodes); + /// \brief Update the render pass chain public: static void UpdateRenderPassChain( Ogre::CompositorWorkspace *_workspace, @@ -179,6 +196,10 @@ namespace ignition /// \sa BaseRenderTarget::Rebuild() protected: void RebuildMaterial(); + /// Calls Ogre2RenderTexture::SetOgreTexture if appropiate to ensure + /// Ogre2RenderTexture::ogreTexture always has our outputs + protected: void SyncOgreTextureVars(); + /// \brief Pointer to the internal ogre camera protected: Ogre::Camera *ogreCamera = nullptr; @@ -198,15 +219,12 @@ namespace ignition /// \brief a material used by for the render target protected: MaterialPtr material; - /// \brief Helper class that applies the material to the render target - protected: Ogre2RenderTargetMaterialPtr materialApplicator[2]; - - /// \brief Pointer to the internal ogre render texture objects - /// There's two because we ping pong postprocessing effects - /// and the final result is always in ogreTexture[1] - /// RenderWindows may have a 3rd texture which is the - /// actual window - protected: Ogre::Texture *ogreTexture[2] = {nullptr, nullptr}; + /// \brief Unused. Kept for ABI reasons. + /// + /// Just in case we set this value to + /// Ogre2RenderTargetPrivate::materialApplicator[0] which is what + /// most client applications may want. + protected: Ogre2RenderTargetMaterialPtr materialApplicator; /// \brief Flag to indicate if the render target color has changed protected: bool colorDirty = true; @@ -247,6 +265,11 @@ namespace ignition // Documentation inherited public: virtual unsigned int GLId() const override; + // Documentation inherited + // TODO(anyone): this function should be removed. + // We didn't do it to preserve ABI. + public: virtual Ogre::RenderTarget *RenderTarget() const override; + // Documentation inherited. protected: virtual void RebuildTarget() override; @@ -256,6 +279,21 @@ namespace ignition /// \brief Build the render texture protected: virtual void BuildTarget(); + /// \brief Do not call this function directly. + /// + /// It's used to keep ABI compatibility to sync ogreTexture + /// with the internal pointer from our base class. + /// \param[in] _ogreTexture texture from + /// Ogre2RenderTargetPrivate::ogreTexture[1] + public: void SetOgreTexture(Ogre::Texture *_ogreTexture); + + /// \brief Unused. Kept for ABI reasons. + /// + /// Just in case we set this value to + /// Ogre2RenderTargetPrivate::ogreTexture[1] which is what most client + /// applications may want. + protected: IGN_DEPRECATED(5) Ogre::Texture * ogreTexture = nullptr; + /// \brief Make scene our friend so it can create a ogre2 render texture private: friend class Ogre2Scene; }; diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh index 624c01ba0..4fe93e7b3 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh @@ -96,6 +96,9 @@ namespace ignition Ogre::Material *_originalMaterial, uint16_t _lodIndex, const Ogre::Renderable *_rend) override; + /// \brief Returns this->renderTarget == _renderTarget + public: bool isSameRenderTarget(Ogre::RenderTarget *_renderTarget); + /// \brief scene manager responsible for rendering private: Ogre::SceneManager *scene = nullptr; diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index af0fe6cc6..2c4c9b831 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -100,6 +100,17 @@ class ignition::rendering::Ogre2RenderTargetPrivate /// \brief Name of shadow compositor node public: const std::string kShadowNodeName = "PbsMaterialsShadowNode"; + + /// \brief Helper class that applies the material to the render target + Ogre2RenderTargetMaterialPtr materialApplicator[2]; + + /// \brief Pointer to the internal ogre render texture objects + /// There's two because we ping pong postprocessing effects + /// and the final result is always in ogreTexture[1] + /// RenderWindows may have a 3rd texture which is the + /// actual window + /// + Ogre::Texture *ogreTexture[2] = {nullptr, nullptr}; }; using namespace ignition; @@ -254,10 +265,13 @@ void Ogre2RenderTarget::BuildCompositor() // Connect them in reverse order const size_t srcIdx = 2u - i - 1u; externalTargets[i].target = - this->ogreTexture[srcIdx]->getBuffer()->getRenderTarget(); + this->dataPtr->ogreTexture[srcIdx]->getBuffer()->getRenderTarget(); externalTargets[i].textures.push_back( - manager.getByName(this->ogreTexture[srcIdx]->getName())); + manager.getByName(this->dataPtr->ogreTexture[srcIdx]->getName())); } + + this->SyncOgreTextureVars(); + this->ogreCompositorWorkspace = ogreCompMgr->addWorkspace(this->scene->OgreSceneManager(), externalTargets, this->ogreCamera, @@ -280,8 +294,19 @@ void Ogre2RenderTarget::DestroyCompositor() for( size_t i = 0u; i < 2u; ++i ) { const size_t srcIdx = (2u - i - 1u); - this->ogreTexture[srcIdx] = externalTargets[i].textures.front().get(); + this->dataPtr->ogreTexture[srcIdx] = + externalTargets[i].textures.front().get(); + } + this->SyncOgreTextureVars(); + + if (this->dataPtr->materialApplicator[0] && + this->dataPtr->materialApplicator[0]->isSameRenderTarget( + this->dataPtr->ogreTexture[0]->getBuffer()->getRenderTarget())) + { + std::swap( this->dataPtr->materialApplicator[0], + this->dataPtr->materialApplicator[1] ); } + this->materialApplicator = this->dataPtr->materialApplicator[0]; auto engine = Ogre2RenderEngine::Instance(); auto ogreRoot = engine->OgreRoot(); @@ -440,10 +465,93 @@ bool Ogre2RenderTarget::isRenderWindow() const return false; } +////////////////////////////////////////////////// +void Ogre2RenderTarget::DestroyTargetImpl() +{ + if (nullptr == this->dataPtr->ogreTexture[0]) + return; + + this->DestroyCompositor(); + + auto &manager = Ogre::TextureManager::getSingleton(); + + this->materialApplicator.reset(); + + for( size_t i = 0u; i < 2u; ++i ) + { + manager.unload(this->dataPtr->ogreTexture[i]->getName()); + manager.remove(this->dataPtr->ogreTexture[i]->getName()); + + // TODO(anyone) there is memory leak when a render texture is destroyed. + // The RenderSystem::_cleanupDepthBuffers method used in ogre1 does not + // seem to work in ogre2 + + this->dataPtr->materialApplicator[i].reset(); + this->dataPtr->ogreTexture[i] = nullptr; + } + + this->SyncOgreTextureVars(); +} + +////////////////////////////////////////////////// +void Ogre2RenderTarget::BuildTargetImpl() +{ + Ogre::TextureManager &manager = Ogre::TextureManager::getSingleton(); + Ogre::PixelFormat ogreFormat = Ogre2Conversions::Convert(this->format); + + // check if target fsaa is supported + unsigned int fsaa = 0; + std::vector fsaaLevels = + Ogre2RenderEngine::Instance()->FSAALevels(); + unsigned int targetFSAA = this->antiAliasing; + auto const it = std::find(fsaaLevels.begin(), fsaaLevels.end(), targetFSAA); + if (it != fsaaLevels.end()) + { + fsaa = targetFSAA; + } + else + { + // output warning but only do it once + static bool ogre2FSAAWarn = false; + if (ogre2FSAAWarn) + { + ignwarn << "Anti-aliasing level of '" << this->antiAliasing << "' " + << "is not supported. Setting to 0" << std::endl; + ogre2FSAAWarn = true; + } + } + + for( size_t i = 0u; i < 2u; ++i ) + { + // Ogre 2 PBS expects gamma correction to be enabled + // Only the second target uses FSAA. + // Note: It's not guaranteed the 2nd target will remain + // the one using FSAA + this->dataPtr->ogreTexture[i] = (manager.createManual( + this->name + std::to_string(i), "General", + Ogre::TEX_TYPE_2D, this->width, this->height, 0, ogreFormat, + Ogre::TU_RENDERTARGET, 0, true, i == 1u ? fsaa : 0)).get(); + } + + this->SyncOgreTextureVars(); +} + +////////////////////////////////////////////////// +unsigned int Ogre2RenderTarget::GLIdImpl() const +{ + if (!this->dataPtr->ogreTexture[0]) + return 0; + + GLuint texId; + this->dataPtr->ogreTexture[1]->getCustomAttribute("GLID", &texId); + + return static_cast(texId); +} + ////////////////////////////////////////////////// Ogre::RenderTarget *Ogre2RenderTarget::RenderTarget() const { - return this->ogreTexture[1]->getBuffer()->getRenderTarget(); + return this->dataPtr->ogreTexture[1]->getBuffer()->getRenderTarget(); } ////////////////////////////////////////////////// @@ -519,12 +627,35 @@ void Ogre2RenderTarget::UpdateRenderPassChain() this->dataPtr->kFinalNodeName, this->renderPasses, this->renderPassDirty, - &this->ogreTexture, + &this->dataPtr->ogreTexture, this->isRenderWindow()); + // this->dataPtr->ogreTexture[0] may have changed + if (this->dataPtr->materialApplicator[0] && + this->dataPtr->materialApplicator[0]->isSameRenderTarget( + this->dataPtr->ogreTexture[0]->getBuffer()->getRenderTarget())) + { + std::swap( this->dataPtr->materialApplicator[0], + this->dataPtr->materialApplicator[1] ); + } + this->materialApplicator = this->dataPtr->materialApplicator[0]; + + this->SyncOgreTextureVars(); + this->renderPassDirty = false; } +////////////////////////////////////////////////// +void Ogre2RenderTarget::UpdateRenderPassChain( + Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, + const std::string &_baseNode, const std::string &_finalNode, + const std::vector &_renderPasses, + bool _recreateNodes) +{ + ignwarn << "Warning: This Ogre2RenderTarget::UpdateRenderPassChain " + << "overload is deprecated" << std::endl; +} + ////////////////////////////////////////////////// void Ogre2RenderTarget::UpdateRenderPassChain( Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, @@ -704,13 +835,24 @@ void Ogre2RenderTarget::RebuildMaterial() for( size_t i = 0u; i < 2u; ++i ) { Ogre::RenderTarget *target = - this->ogreTexture[i]->getBuffer()->getRenderTarget(); - this->materialApplicator[i].reset(new Ogre2RenderTargetMaterial( - sceneMgr, target, matPtr.get())); + this->dataPtr->ogreTexture[i]->getBuffer()->getRenderTarget(); + this->dataPtr->materialApplicator[i].reset( + new Ogre2RenderTargetMaterial(sceneMgr, target, matPtr.get())); } + + this->materialApplicator = this->dataPtr->materialApplicator[0]; } } +////////////////////////////////////////////////// +void Ogre2RenderTarget::SyncOgreTextureVars() +{ + Ogre2RenderTexture *asRenderTexture = + dynamic_cast(this); + if (asRenderTexture) + asRenderTexture->SetOgreTexture(this->dataPtr->ogreTexture[1]); +} + ////////////////////////////////////////////////// // Ogre2RenderTexture ////////////////////////////////////////////////// @@ -739,77 +881,19 @@ void Ogre2RenderTexture::RebuildTarget() ////////////////////////////////////////////////// void Ogre2RenderTexture::DestroyTarget() { - if (nullptr == this->ogreTexture[0]) - return; - - this->DestroyCompositor(); - - auto &manager = Ogre::TextureManager::getSingleton(); - - for( size_t i = 0u; i < 2u; ++i ) - { - manager.unload(this->ogreTexture[i]->getName()); - manager.remove(this->ogreTexture[i]->getName()); - - // TODO(anyone) there is memory leak when a render texture is destroyed. - // The RenderSystem::_cleanupDepthBuffers method used in ogre1 does not - // seem to work in ogre2 - - this->ogreTexture[i] = nullptr; - } + Ogre2RenderTarget::DestroyTargetImpl(); } ////////////////////////////////////////////////// void Ogre2RenderTexture::BuildTarget() { - Ogre::TextureManager &manager = Ogre::TextureManager::getSingleton(); - Ogre::PixelFormat ogreFormat = Ogre2Conversions::Convert(this->format); - - // check if target fsaa is supported - unsigned int fsaa = 0; - std::vector fsaaLevels = - Ogre2RenderEngine::Instance()->FSAALevels(); - unsigned int targetFSAA = this->antiAliasing; - auto const it = std::find(fsaaLevels.begin(), fsaaLevels.end(), targetFSAA); - if (it != fsaaLevels.end()) - { - fsaa = targetFSAA; - } - else - { - // output warning but only do it once - static bool ogre2FSAAWarn = false; - if (ogre2FSAAWarn) - { - ignwarn << "Anti-aliasing level of '" << this->antiAliasing << "' " - << "is not supported. Setting to 0" << std::endl; - ogre2FSAAWarn = true; - } - } - - for( size_t i = 0u; i < 2u; ++i ) - { - // Ogre 2 PBS expects gamma correction to be enabled - // Only the second target uses FSAA. - // Note: It's not guaranteed the 2nd target will remain - // the one using FSAA - this->ogreTexture[i] = (manager.createManual( - this->name + std::to_string(i), "General", - Ogre::TEX_TYPE_2D, this->width, this->height, 0, ogreFormat, - Ogre::TU_RENDERTARGET, 0, true, i == 1u ? fsaa : 0)).get(); - } + Ogre2RenderTarget::BuildTargetImpl(); } ////////////////////////////////////////////////// unsigned int Ogre2RenderTexture::GLId() const { - if (!this->ogreTexture[0]) - return 0; - - GLuint texId; - this->ogreTexture[1]->getCustomAttribute("GLID", &texId); - - return static_cast(texId); + return Ogre2RenderTarget::GLIdImpl(); } ////////////////////////////////////////////////// @@ -824,6 +908,18 @@ void Ogre2RenderTexture::PostRender() Ogre2RenderTarget::PostRender(); } +////////////////////////////////////////////////// +Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget() const +{ + return Ogre2RenderTarget::RenderTarget(); +} + +////////////////////////////////////////////////// +void Ogre2RenderTexture::SetOgreTexture(Ogre::Texture *_ogreTexture) +{ + this->ogreTexture = _ogreTexture; +} + ////////////////////////////////////////////////// // Ogre2RenderWindow ////////////////////////////////////////////////// diff --git a/ogre2/src/Ogre2RenderTargetMaterial.cc b/ogre2/src/Ogre2RenderTargetMaterial.cc index 3d416a2e7..b679875ec 100644 --- a/ogre2/src/Ogre2RenderTargetMaterial.cc +++ b/ogre2/src/Ogre2RenderTargetMaterial.cc @@ -75,3 +75,10 @@ Ogre::Technique *Ogre2RenderTargetMaterial::handleSchemeNotFound( } return nullptr; } + +////////////////////////////////////////////////// +bool Ogre2RenderTargetMaterial::isSameRenderTarget( + Ogre::RenderTarget *_renderTarget) +{ + return this->renderTarget == _renderTarget; +} From 10d493fbbf7ff727690c486c0f5e436f2eccea5f Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sun, 23 May 2021 17:12:33 -0300 Subject: [PATCH 6/9] Fix camel case convention Signed-off-by: Matias N. Goldberg --- .../ignition/rendering/ogre2/Ogre2RenderTarget.hh | 12 ++++++------ .../rendering/ogre2/Ogre2RenderTargetMaterial.hh | 2 +- ogre2/src/Ogre2RenderTarget.cc | 12 ++++++------ ogre2/src/Ogre2RenderTargetMaterial.cc | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index eb75f5069..091a69527 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -115,12 +115,16 @@ namespace ignition /// \see Camera::SetShadowsNodeDefDirty public: void SetShadowsNodeDefDirty(); + /// \brief Get a pointer to the ogre render target containing + /// the results of the render + public: virtual Ogre::RenderTarget *RenderTarget() const; + /// \brief Returns true if this is a render window /// TODO(anyone): this function should be virtual. /// We didn't do it to preserve ABI. /// Look in commit history for '#Ogre2IsRenderWindowABI' to /// see changes made and revert - public: bool isRenderWindow() const; + public: bool IsRenderWindow() const; // Documentation inherited public: unsigned int GLIdImpl() const; @@ -131,10 +135,6 @@ namespace ignition /// \brief Build the render texture protected: void BuildTargetImpl(); - /// \brief Get a pointer to the ogre render target containing - /// the results of the render - public: virtual Ogre::RenderTarget *RenderTarget() const; - /// \brief Get visibility mask for the viewport associated with this /// render target /// \return Visibility mask @@ -315,7 +315,7 @@ namespace ignition // We didn't do it to preserve ABI. // Looks in commit history for '#Ogre2IsRenderWindowABI' to // see changes made and revert - public: bool isRenderWindow() const; + public: bool IsRenderWindow() const; // Documentation inherited. public: virtual Ogre::RenderTarget *RenderTarget() const override; diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh index 4fe93e7b3..57bfaae61 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh @@ -97,7 +97,7 @@ namespace ignition const Ogre::Renderable *_rend) override; /// \brief Returns this->renderTarget == _renderTarget - public: bool isSameRenderTarget(Ogre::RenderTarget *_renderTarget); + public: bool IsSameRenderTarget(Ogre::RenderTarget *_renderTarget); /// \brief scene manager responsible for rendering private: Ogre::SceneManager *scene = nullptr; diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index 2c4c9b831..a0e8fda6d 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -245,7 +245,7 @@ void Ogre2RenderTarget::BuildCompositor() workDef->connectExternal(0, nodeDefName, 0); workDef->connectExternal(1, nodeDefName, 1); - if (!this->isRenderWindow()) + if (!this->IsRenderWindow()) { workDef->connect(nodeDefName, finalNodeDefName); } @@ -300,7 +300,7 @@ void Ogre2RenderTarget::DestroyCompositor() this->SyncOgreTextureVars(); if (this->dataPtr->materialApplicator[0] && - this->dataPtr->materialApplicator[0]->isSameRenderTarget( + this->dataPtr->materialApplicator[0]->IsSameRenderTarget( this->dataPtr->ogreTexture[0]->getBuffer()->getRenderTarget())) { std::swap( this->dataPtr->materialApplicator[0], @@ -455,7 +455,7 @@ void Ogre2RenderTarget::Render() } ////////////////////////////////////////////////// -bool Ogre2RenderTarget::isRenderWindow() const +bool Ogre2RenderTarget::IsRenderWindow() const { const Ogre2RenderWindow *asWindow = dynamic_cast(this); @@ -628,11 +628,11 @@ void Ogre2RenderTarget::UpdateRenderPassChain() this->renderPasses, this->renderPassDirty, &this->dataPtr->ogreTexture, - this->isRenderWindow()); + this->IsRenderWindow()); // this->dataPtr->ogreTexture[0] may have changed if (this->dataPtr->materialApplicator[0] && - this->dataPtr->materialApplicator[0]->isSameRenderTarget( + this->dataPtr->materialApplicator[0]->IsSameRenderTarget( this->dataPtr->ogreTexture[0]->getBuffer()->getRenderTarget())) { std::swap( this->dataPtr->materialApplicator[0], @@ -933,7 +933,7 @@ Ogre2RenderWindow::~Ogre2RenderWindow() } ////////////////////////////////////////////////// -bool Ogre2RenderWindow::isRenderWindow() const +bool Ogre2RenderWindow::IsRenderWindow() const { return true; } diff --git a/ogre2/src/Ogre2RenderTargetMaterial.cc b/ogre2/src/Ogre2RenderTargetMaterial.cc index b679875ec..bcea1ae90 100644 --- a/ogre2/src/Ogre2RenderTargetMaterial.cc +++ b/ogre2/src/Ogre2RenderTargetMaterial.cc @@ -77,7 +77,7 @@ Ogre::Technique *Ogre2RenderTargetMaterial::handleSchemeNotFound( } ////////////////////////////////////////////////// -bool Ogre2RenderTargetMaterial::isSameRenderTarget( +bool Ogre2RenderTargetMaterial::IsSameRenderTarget( Ogre::RenderTarget *_renderTarget) { return this->renderTarget == _renderTarget; From 40d04d94c89000fa68d3ea50bd02b9800e6445cb Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Fri, 28 May 2021 12:55:50 -0300 Subject: [PATCH 7/9] Make Ogre2RenderTarget::RenderTarget pure virtual again Hopefully this will prevent ABI breakage Signed-off-by: Matias N. Goldberg --- .../include/ignition/rendering/ogre2/Ogre2RenderTarget.hh | 7 ++++++- ogre2/src/Ogre2RenderTarget.cc | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh index 091a69527..32672db24 100644 --- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh +++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTarget.hh @@ -115,9 +115,14 @@ namespace ignition /// \see Camera::SetShadowsNodeDefDirty public: void SetShadowsNodeDefDirty(); + /// \brief Get a pointer to the ogre render target containing + /// the results of the render (implemented separately + /// to avoid breaking ABI of the pure virtual function) + protected: Ogre::RenderTarget *RenderTargetImpl() const; + /// \brief Get a pointer to the ogre render target containing /// the results of the render - public: virtual Ogre::RenderTarget *RenderTarget() const; + public: virtual Ogre::RenderTarget *RenderTarget() const = 0; /// \brief Returns true if this is a render window /// TODO(anyone): this function should be virtual. diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index a0e8fda6d..049da76f4 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -549,7 +549,7 @@ unsigned int Ogre2RenderTarget::GLIdImpl() const } ////////////////////////////////////////////////// -Ogre::RenderTarget *Ogre2RenderTarget::RenderTarget() const +Ogre::RenderTarget *Ogre2RenderTarget::RenderTargetImpl() const { return this->dataPtr->ogreTexture[1]->getBuffer()->getRenderTarget(); } @@ -911,7 +911,7 @@ void Ogre2RenderTexture::PostRender() ////////////////////////////////////////////////// Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget() const { - return Ogre2RenderTarget::RenderTarget(); + return Ogre2RenderTarget::RenderTargetImpl(); } ////////////////////////////////////////////////// From bd4aa2b1dfaf0861cf5b3b613166bfec0fb42815 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Fri, 28 May 2021 17:20:42 -0300 Subject: [PATCH 8/9] Fix deprecation warnings during build Signed-off-by: Matias N. Goldberg --- ogre2/src/Ogre2RenderTarget.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index 049da76f4..a2f75e8bb 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -647,10 +647,11 @@ void Ogre2RenderTarget::UpdateRenderPassChain() ////////////////////////////////////////////////// void Ogre2RenderTarget::UpdateRenderPassChain( - Ogre::CompositorWorkspace *_workspace, const std::string &_workspaceDefName, - const std::string &_baseNode, const std::string &_finalNode, - const std::vector &_renderPasses, - bool _recreateNodes) + Ogre::CompositorWorkspace * /*_workspace*/, + const std::string & /*_workspaceDefName*/, + const std::string & /*_baseNode*/, const std::string & /*_finalNode*/, + const std::vector & /*_renderPasses*/, + bool /*_recreateNodes*/) { ignwarn << "Warning: This Ogre2RenderTarget::UpdateRenderPassChain " << "overload is deprecated" << std::endl; @@ -856,9 +857,12 @@ void Ogre2RenderTarget::SyncOgreTextureVars() ////////////////////////////////////////////////// // Ogre2RenderTexture ////////////////////////////////////////////////// +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" Ogre2RenderTexture::Ogre2RenderTexture() { } +#pragma GCC diagnostic pop ////////////////////////////////////////////////// Ogre2RenderTexture::~Ogre2RenderTexture() @@ -917,7 +921,10 @@ Ogre::RenderTarget *Ogre2RenderTexture::RenderTarget() const ////////////////////////////////////////////////// void Ogre2RenderTexture::SetOgreTexture(Ogre::Texture *_ogreTexture) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" this->ogreTexture = _ogreTexture; +#pragma GCC diagnostic pop } ////////////////////////////////////////////////// From 9b01d228c799153641bfd73f77482113f1b1f611 Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Sun, 13 Jun 2021 21:17:46 -0300 Subject: [PATCH 9/9] Fix invalid write of size 8 This was causing heap corruption. At best it would crash. At worst it would manifeset in subtle weird behaviors Signed-off-by: Matias N. Goldberg --- ogre2/src/Ogre2RenderTarget.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ogre2/src/Ogre2RenderTarget.cc b/ogre2/src/Ogre2RenderTarget.cc index a2f75e8bb..0280a2f19 100644 --- a/ogre2/src/Ogre2RenderTarget.cc +++ b/ogre2/src/Ogre2RenderTarget.cc @@ -764,7 +764,7 @@ void Ogre2RenderTarget::UpdateRenderPassChain( for( size_t i = 0u; i < 2u; ++i ) { const size_t srcIdx = bMustSwapRts ? (2u - i - 1u) : i; - *_ogreTextures[srcIdx] = externalTargets[i].textures.front().get(); + (*_ogreTextures)[srcIdx] = externalTargets[i].textures.front().get(); } } else