From 5c69a7363efeedf9c1bec0a5dedd99d6e6c5362b Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 5 Nov 2021 09:40:35 -0700 Subject: [PATCH 1/3] More nullptr checking on visuals and base axis Signed-off-by: Louise Poubel --- .../ignition/rendering/base/BaseAxisVisual.hh | 3 +- ogre/src/OgreVisual.cc | 47 +++++++++++++++---- ogre2/src/Ogre2Visual.cc | 25 +++++++++- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/include/ignition/rendering/base/BaseAxisVisual.hh b/include/ignition/rendering/base/BaseAxisVisual.hh index 4e3f301ca..ec70445df 100644 --- a/include/ignition/rendering/base/BaseAxisVisual.hh +++ b/include/ignition/rendering/base/BaseAxisVisual.hh @@ -167,7 +167,8 @@ namespace ignition { auto arrow = std::dynamic_pointer_cast( this->ChildByIndex(i)); - arrow->SetVisible(_visible); + if (arrow != nullptr) + arrow->SetVisible(_visible); } } } diff --git a/ogre/src/OgreVisual.cc b/ogre/src/OgreVisual.cc index 4923da43e..065383bb7 100644 --- a/ogre/src/OgreVisual.cc +++ b/ogre/src/OgreVisual.cc @@ -51,6 +51,9 @@ void OgreVisual::SetWireframe(bool _show) if (this->dataPtr->wireframe == _show) return; + if (!this->ogreNode) + return; + this->dataPtr->wireframe = _show; for (unsigned int i = 0; i < this->ogreNode->numAttachedObjects(); i++) @@ -100,6 +103,9 @@ bool OgreVisual::Wireframe() const ////////////////////////////////////////////////// void OgreVisual::SetVisible(bool _visible) { + if (!this->ogreNode) + return; + this->ogreNode->setVisible(_visible); } @@ -124,6 +130,19 @@ GeometryStorePtr OgreVisual::Geometries() const ////////////////////////////////////////////////// bool OgreVisual::AttachGeometry(GeometryPtr _geometry) { + if (!_geometry) + { + ignerr << "Cannot attach null geometry." << std::endl; + + return false; + } + + if (!this->ogreNode) + { + ignerr << "Cannot attach geometry, null Ogre node." << std::endl; + return false; + } + OgreGeometryPtr derived = std::dynamic_pointer_cast(_geometry); @@ -137,16 +156,20 @@ bool OgreVisual::AttachGeometry(GeometryPtr _geometry) // Some geometries, like heightmaps, may not have an OgreObject Ogre::MovableObject *ogreObj = derived->OgreObject(); - if (ogreObj) + if (!ogreObj) { - // set user data for mouse queries - ogreObj->getUserObjectBindings().setUserAny( - Ogre::Any(this->Id())); - ogreObj->setVisibilityFlags(this->visibilityFlags); - this->ogreNode->attachObject(ogreObj); + ignerr << "Cannot attach a null geometry object" << std::endl; + return false; } + // set user data for mouse queries + ogreObj->getUserObjectBindings().setUserAny( + Ogre::Any(this->Id())); + ogreObj->setVisibilityFlags(this->visibilityFlags); + derived->SetParent(this->SharedThis()); + this->ogreNode->attachObject(ogreObj); + return true; } @@ -154,7 +177,10 @@ bool OgreVisual::AttachGeometry(GeometryPtr _geometry) bool OgreVisual::DetachGeometry(GeometryPtr _geometry) { if (!this->ogreNode) - return true; + { + ignerr << "Cannot detach geometry, null Ogre node." << std::endl; + return false; + } OgreGeometryPtr derived = std::dynamic_pointer_cast(_geometry); @@ -200,6 +226,9 @@ void OgreVisual::BoundsHelper(ignition::math::AxisAlignedBox &_box, void OgreVisual::BoundsHelper(ignition::math::AxisAlignedBox &_box, bool _local, const ignition::math::Pose3d &_pose) const { + if (!this->ogreNode) + return; + this->ogreNode->_updateBounds(); this->ogreNode->_update(false, true); @@ -228,6 +257,7 @@ void OgreVisual::BoundsHelper(ignition::math::AxisAlignedBox &_box, Ogre::Vector3 ogreMin = bb.getMinimum(); Ogre::Vector3 ogreMax = bb.getMaximum(); + // Get ogre bounding boxes and size to object's scale min = scale * ignition::math::Vector3d(ogreMin.x, ogreMin.y, ogreMin.z); max = scale * ignition::math::Vector3d(ogreMax.x, ogreMax.y, ogreMax.z); box.Min() = min, @@ -240,9 +270,8 @@ void OgreVisual::BoundsHelper(ignition::math::AxisAlignedBox &_box, if (_local) { ignition::math::Pose3d worldPose = this->WorldPose(); - ignition::math::Quaternion parentRot = _pose.Rot(); ignition::math::Vector3d parentPos = _pose.Pos(); - ignition::math::Quaternion parentRotInv = parentRot.Inverse(); + ignition::math::Quaternion parentRotInv = _pose.Rot().Inverse(); ignition::math::Pose3d localTransform = ignition::math::Pose3d( (parentRotInv * (worldPose.Pos() - parentPos)), diff --git a/ogre2/src/Ogre2Visual.cc b/ogre2/src/Ogre2Visual.cc index 91bd93e2e..99bc65702 100644 --- a/ogre2/src/Ogre2Visual.cc +++ b/ogre2/src/Ogre2Visual.cc @@ -61,6 +61,9 @@ void Ogre2Visual::SetWireframe(bool _show) if (this->dataPtr->wireframe == _show) return; + if (!this->ogreNode) + return; + this->dataPtr->wireframe = _show; for (unsigned int i = 0; i < this->ogreNode->numAttachedObjects(); i++) @@ -96,6 +99,9 @@ bool Ogre2Visual::Wireframe() const ////////////////////////////////////////////////// void Ogre2Visual::SetVisible(bool _visible) { + if (!this->ogreNode) + return; + this->ogreNode->setVisible(_visible); } @@ -130,6 +136,12 @@ bool Ogre2Visual::AttachGeometry(GeometryPtr _geometry) return false; } + if (!this->ogreNode) + { + ignerr << "Cannot attach geometry, null Ogre node." << std::endl; + return false; + } + Ogre2GeometryPtr derived = std::dynamic_pointer_cast(_geometry); @@ -141,6 +153,7 @@ bool Ogre2Visual::AttachGeometry(GeometryPtr _geometry) return false; } + // Some geometries, like heightmaps, may not have an OgreObject Ogre::MovableObject *ogreObj = derived->OgreObject(); if (!ogreObj) { @@ -164,6 +177,12 @@ bool Ogre2Visual::AttachGeometry(GeometryPtr _geometry) ////////////////////////////////////////////////// bool Ogre2Visual::DetachGeometry(GeometryPtr _geometry) { + if (!this->ogreNode) + { + ignerr << "Cannot detach geometry, null Ogre node." << std::endl; + return false; + } + Ogre2GeometryPtr derived = std::dynamic_pointer_cast(_geometry); @@ -175,7 +194,8 @@ bool Ogre2Visual::DetachGeometry(GeometryPtr _geometry) return false; } - this->ogreNode->detachObject(derived->OgreObject()); + if (nullptr != derived->OgreObject()) + this->ogreNode->detachObject(derived->OgreObject()); derived->SetParent(nullptr); return true; } @@ -207,6 +227,9 @@ void Ogre2Visual::BoundsHelper(ignition::math::AxisAlignedBox &_box, void Ogre2Visual::BoundsHelper(ignition::math::AxisAlignedBox &_box, bool _local, const ignition::math::Pose3d &_pose) const { + if (!this->ogreNode) + return; + ignition::math::Vector3d scale = this->WorldScale(); for (size_t i = 0; i < this->ogreNode->numAttachedObjects(); i++) From 74b5fffd3866de0e382f37e1a2ace9cfb064a32a Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 5 Nov 2021 11:00:29 -0700 Subject: [PATCH 2/3] revert change that affected heightmaps Signed-off-by: Louise Poubel --- ogre/src/OgreVisual.cc | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/ogre/src/OgreVisual.cc b/ogre/src/OgreVisual.cc index 065383bb7..fba1eee91 100644 --- a/ogre/src/OgreVisual.cc +++ b/ogre/src/OgreVisual.cc @@ -156,20 +156,16 @@ bool OgreVisual::AttachGeometry(GeometryPtr _geometry) // Some geometries, like heightmaps, may not have an OgreObject Ogre::MovableObject *ogreObj = derived->OgreObject(); - if (!ogreObj) + if (ogreObj) { - ignerr << "Cannot attach a null geometry object" << std::endl; - return false; + // set user data for mouse queries + ogreObj->getUserObjectBindings().setUserAny( + Ogre::Any(this->Id())); + ogreObj->setVisibilityFlags(this->visibilityFlags); + this->ogreNode->attachObject(ogreObj); } - // set user data for mouse queries - ogreObj->getUserObjectBindings().setUserAny( - Ogre::Any(this->Id())); - ogreObj->setVisibilityFlags(this->visibilityFlags); - derived->SetParent(this->SharedThis()); - this->ogreNode->attachObject(ogreObj); - return true; } From fb5469ff9f56f831d9a112a3c50710bbc8a75a1b Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 5 Nov 2021 11:01:57 -0700 Subject: [PATCH 3/3] remove note that doesn't apply to ogre2 Signed-off-by: Louise Poubel --- ogre2/src/Ogre2Visual.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/ogre2/src/Ogre2Visual.cc b/ogre2/src/Ogre2Visual.cc index 99bc65702..ba2658e41 100644 --- a/ogre2/src/Ogre2Visual.cc +++ b/ogre2/src/Ogre2Visual.cc @@ -153,7 +153,6 @@ bool Ogre2Visual::AttachGeometry(GeometryPtr _geometry) return false; } - // Some geometries, like heightmaps, may not have an OgreObject Ogre::MovableObject *ogreObj = derived->OgreObject(); if (!ogreObj) {