From 1f4def63aefcf788a427cd9bb4354617490be67c Mon Sep 17 00:00:00 2001
From: Ian Chen <ichen@openrobotics.org>
Date: Thu, 31 Oct 2024 17:00:37 -0700
Subject: [PATCH] Check valid camera near far clip distances (#1082)

Signed-off-by: Ian Chen <ichen@openrobotics.org>
---
 include/gz/rendering/base/BaseCamera.hh | 13 +++++++++++++
 ogre/src/OgreCamera.cc                  |  4 ++--
 ogre2/src/Ogre2Camera.cc                |  4 ++--
 test/common_test/Camera_TEST.cc         |  8 ++++++++
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/gz/rendering/base/BaseCamera.hh b/include/gz/rendering/base/BaseCamera.hh
index c1771b2a6..5201a5e88 100644
--- a/include/gz/rendering/base/BaseCamera.hh
+++ b/include/gz/rendering/base/BaseCamera.hh
@@ -17,6 +17,7 @@
 #ifndef GZ_RENDERING_BASE_BASECAMERA_HH_
 #define GZ_RENDERING_BASE_BASECAMERA_HH_
 
+#include <cmath>
 #include <string>
 
 #include <gz/math/Matrix3.hh>
@@ -709,6 +710,12 @@ namespace gz
     template <class T>
     void BaseCamera<T>::SetFarClipPlane(const double _far)
     {
+      if (_far <= 0 || !std::isfinite(_far))
+      {
+        gzerr << "Far clip distance must be a finite number greater than 0."
+              << std::endl;
+        return;
+      }
       this->farClip = _far;
     }
 
@@ -723,6 +730,12 @@ namespace gz
     template <class T>
     void BaseCamera<T>::SetNearClipPlane(const double _near)
     {
+      if (_near <= 0 || !std::isfinite(_near))
+      {
+        gzerr << "Near clip distance must be a finite number greater than 0."
+              << std::endl;
+        return;
+      }
       this->nearClip = _near;
     }
 
diff --git a/ogre/src/OgreCamera.cc b/ogre/src/OgreCamera.cc
index 3b7af1ef8..03a652a6e 100644
--- a/ogre/src/OgreCamera.cc
+++ b/ogre/src/OgreCamera.cc
@@ -340,14 +340,14 @@ void OgreCamera::SetNearClipPlane(const double _near)
 {
   // this->nearClip = _near;
   BaseCamera::SetNearClipPlane(_near);
-  this->ogreCamera->setNearClipDistance(_near);
+  this->ogreCamera->setNearClipDistance(this->nearClip);
 }
 
 //////////////////////////////////////////////////
 void OgreCamera::SetFarClipPlane(const double _far)
 {
   BaseCamera::SetFarClipPlane(_far);
-  this->ogreCamera->setFarClipDistance(_far);
+  this->ogreCamera->setFarClipDistance(this->farClip);
 }
 
 //////////////////////////////////////////////////
diff --git a/ogre2/src/Ogre2Camera.cc b/ogre2/src/Ogre2Camera.cc
index aa3024431..61cca096b 100644
--- a/ogre2/src/Ogre2Camera.cc
+++ b/ogre2/src/Ogre2Camera.cc
@@ -403,14 +403,14 @@ void Ogre2Camera::SetProjectionType(CameraProjectionType _type)
 void Ogre2Camera::SetNearClipPlane(const double _near)
 {
   BaseCamera::SetNearClipPlane(_near);
-  this->ogreCamera->setNearClipDistance(_near);
+  this->ogreCamera->setNearClipDistance(this->nearClip);
 }
 
 //////////////////////////////////////////////////
 void Ogre2Camera::SetFarClipPlane(const double _far)
 {
   BaseCamera::SetFarClipPlane(_far);
-  this->ogreCamera->setFarClipDistance(_far);
+  this->ogreCamera->setFarClipDistance(this->farClip);
 }
 
 //////////////////////////////////////////////////
diff --git a/test/common_test/Camera_TEST.cc b/test/common_test/Camera_TEST.cc
index d42aabe9c..87cb4ef29 100644
--- a/test/common_test/Camera_TEST.cc
+++ b/test/common_test/Camera_TEST.cc
@@ -61,10 +61,18 @@ TEST_F(CameraTest, ViewProjectionMatrix)
   camera->SetNearClipPlane(0.1);
   EXPECT_DOUBLE_EQ(0.1, camera->NearClipPlane());
 
+  // set invalid clip distance and verify it is not set
+  camera->SetNearClipPlane(0.0);
+  EXPECT_DOUBLE_EQ(0.1, camera->NearClipPlane());
+
   EXPECT_GT(camera->FarClipPlane(), 0);
   camera->SetFarClipPlane(800);
   EXPECT_DOUBLE_EQ(800, camera->FarClipPlane());
 
+  // set invalid clip distance and verify it is not set
+  camera->SetFarClipPlane(-10.0);
+  EXPECT_DOUBLE_EQ(800, camera->FarClipPlane());
+
   EXPECT_NE(projMatrix, camera->ProjectionMatrix());
 
   // view matrix