Bugfixes and docs improvement of AABB tensor class (#6486)

* Change invalid values of min_bound and max_bound as done in legacy, related pr #6444 * Correct points length check when creating AABB from points. * Print min and max bound of AABB when printing AABB * Add brief docs for methods and variables, fix warning messages. * Fix AABB ToString() test --------- Co-authored-by: Sameer Sheorey <[email protected]>
isl-org · Nov 14, 2023 · 392fcb9 · 392fcb9
1 parent 9b45f01
commit 392fcb9
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@
 * Fix printing of tensor in gpu and add validation check for bounds of axis-aligned bounding box (PR #6444)  
 * Python 3.11 support. bump pybind11 v2.6.2 -> v2.11.1
 * Check for support of CUDA Memory Pools at runtime (#4679)
+* Fix `toString`, `CreateFromPoints` methods and improve docs in `AxisAlignedBoundingBox`. 🐛📝
 
 ## 0.13
 

diff --git a/cpp/open3d/geometry/BoundingVolume.cpp b/cpp/open3d/geometry/BoundingVolume.cpp
@@ -303,7 +303,7 @@ AxisAlignedBoundingBox& AxisAlignedBoundingBox::Scale(
 AxisAlignedBoundingBox& AxisAlignedBoundingBox::Rotate(
         const Eigen::Matrix3d& rotation, const Eigen::Vector3d& center) {
     utility::LogError(
-            "A rotation of a AxisAlignedBoundingBox would not be axis aligned "
+            "A rotation of an AxisAlignedBoundingBox would not be axis-aligned "
             "anymore, convert it to an OrientedBoundingBox first");
     return *this;
 }
@@ -330,6 +330,9 @@ AxisAlignedBoundingBox AxisAlignedBoundingBox::CreateFromPoints(
         const std::vector<Eigen::Vector3d>& points) {
     AxisAlignedBoundingBox box;
     if (points.empty()) {
+        utility::LogWarning(
+                "The number of points is 0 when creating axis-aligned bounding "
+                "box.");
         box.min_bound_ = Eigen::Vector3d(0.0, 0.0, 0.0);
         box.max_bound_ = Eigen::Vector3d(0.0, 0.0, 0.0);
     } else {

diff --git a/cpp/open3d/geometry/BoundingVolume.h b/cpp/open3d/geometry/BoundingVolume.h
@@ -151,7 +151,8 @@ class OrientedBoundingBox : public Geometry3D {
 
 /// \class AxisAlignedBoundingBox
 ///
-/// \brief A bounding box that is aligned along the coordinate axes.
+/// \brief A bounding box that is aligned along the coordinate axes and defined
+/// by the min_bound and max_bound.
 ///
 ///  The AxisAlignedBoundingBox uses the coordinate axes for bounding box
 ///  generation. This means that the bounding box is oriented along the
@@ -227,14 +228,20 @@ class AxisAlignedBoundingBox : public Geometry3D {
     /// extents.
     double GetMaxExtent() const { return (max_bound_ - min_bound_).maxCoeff(); }
 
+    /// Calculates the percentage position of the given x-coordinate within
+    /// the x-axis range of this AxisAlignedBoundingBox.
     double GetXPercentage(double x) const {
         return (x - min_bound_(0)) / (max_bound_(0) - min_bound_(0));
     }
 
+    /// Calculates the percentage position of the given y-coordinate within
+    /// the y-axis range of this AxisAlignedBoundingBox.
     double GetYPercentage(double y) const {
         return (y - min_bound_(1)) / (max_bound_(1) - min_bound_(1));
     }
 
+    /// Calculates the percentage position of the given z-coordinate within
+    /// the z-axis range of this AxisAlignedBoundingBox.
     double GetZPercentage(double z) const {
         return (z - min_bound_(2)) / (max_bound_(2) - min_bound_(2));
     }

diff --git a/cpp/open3d/t/geometry/BoundingVolume.cpp b/cpp/open3d/t/geometry/BoundingVolume.cpp
@@ -42,9 +42,12 @@ AxisAlignedBoundingBox::AxisAlignedBoundingBox(const core::Tensor &min_bound,
 
     // Check if the bounding box is valid.
     if (Volume() < 0) {
-        utility::LogError(
-                "Invalid axis-aligned bounding box. Please make sure all "
-                "the elements in max bound are larger than min bound.");
+        utility::LogWarning(
+                "max_bound {} of bounding box is smaller than min_bound {} in "
+                "one or more axes. Fix input values to remove this warning.",
+                max_bound_.ToString(false), min_bound_.ToString(false));
+        max_bound_ = open3d::core::Maximum(min_bound, max_bound);
+        min_bound_ = open3d::core::Minimum(min_bound, max_bound);
     }
 }
 
@@ -80,7 +83,7 @@ void AxisAlignedBoundingBox::SetMinBound(const core::Tensor &min_bound) {
     if (Volume() < 0) {
         utility::LogWarning(
                 "Invalid axis-aligned bounding box. Please make sure all "
-                "the elements in min bound are smaller than min bound.");
+                "the elements in min bound are smaller than max bound.");
         min_bound_ = tmp;
     }
 }
@@ -110,8 +113,8 @@ void AxisAlignedBoundingBox::SetColor(const core::Tensor &color) {
     if (color.Max({0}).To(core::Float64).Item<double>() > 1.0 ||
         color.Min({0}).To(core::Float64).Item<double>() < 0.0) {
         utility::LogError(
-                "The color must be in the range [0, 1], but for range [{}, "
-                "{}].",
+                "The color must be in the range [0, 1], but found in range "
+                "[{}, {}].",
                 color.Min({0}).To(core::Float64).Item<double>(),
                 color.Max({0}).To(core::Float64).Item<double>());
     }
@@ -220,16 +223,20 @@ core::Tensor AxisAlignedBoundingBox::GetPointIndicesWithinBoundingBox(
 }
 
 std::string AxisAlignedBoundingBox::ToString() const {
-    return fmt::format("AxisAlignedBoundingBox[{}, {}]", GetDtype().ToString(),
+    return fmt::format("AxisAlignedBoundingBox[{} - {}, {}, {}]",
+                       GetMinBound().ToString(false),
+                       GetMaxBound().ToString(false), GetDtype().ToString(),
                        GetDevice().ToString());
 }
 
 AxisAlignedBoundingBox AxisAlignedBoundingBox::CreateFromPoints(
         const core::Tensor &points) {
     core::AssertTensorShape(points, {utility::nullopt, 3});
     core::AssertTensorDtypes(points, {core::Float32, core::Float64});
-    if (points.GetLength() <= 3) {
-        utility::LogWarning("The points number is less than 3.");
+    if (points.GetLength() <= 0) {
+        utility::LogWarning(
+                "The number of points is 0 when creating axis-aligned bounding "
+                "box.");
         return AxisAlignedBoundingBox(points.GetDevice());
     } else {
         const core::Tensor min_bound = points.Min({0});
@@ -385,8 +392,8 @@ void OrientedBoundingBox::SetColor(const core::Tensor &color) {
     if (color.Max({0}).To(core::Float64).Item<double>() > 1.0 ||
         color.Min({0}).To(core::Float64).Item<double>() < 0.0) {
         utility::LogError(
-                "The color must be in the range [0, 1], but for range [{}, "
-                "{}].",
+                "The color must be in the range [0, 1], but found in range "
+                "[{}, {}].",
                 color.Min({0}).To(core::Float64).Item<double>(),
                 color.Max({0}).To(core::Float64).Item<double>());
     }

diff --git a/cpp/open3d/t/geometry/BoundingVolume.h b/cpp/open3d/t/geometry/BoundingVolume.h
@@ -93,12 +93,12 @@ class AxisAlignedBoundingBox : public Geometry, public DrawableGeometry {
     /// \param min_bound Tensor with {3,} shape, and type float32 or float64.
     void SetMinBound(const core::Tensor &min_bound);
 
-    /// \brief Set the max boundof the box.
+    /// \brief Set the max bound of the box.
     /// If the data type of the given tensor differs from the data type of the
     /// box, an exception will be thrown.
     ///
     /// If the max bound makes the box invalid, it will not be set to the box.
-    /// \param min_bound Tensor with {3,} shape, and type float32 or float64.
+    /// \param max_bound Tensor with {3,} shape, and type float32 or float64.
     void SetMaxBound(const core::Tensor &max_bound);
 
     /// \brief Set the color of the box.
@@ -156,25 +156,32 @@ class AxisAlignedBoundingBox : public Geometry, public DrawableGeometry {
     /// Returns the half extent of the bounding box.
     core::Tensor GetHalfExtent() const { return GetExtent() / 2; }
 
-    /// Returns the maximum extent, i.e. the maximum of X, Y and Z axis'
+    /// \brief Returns the maximum extent, i.e. the maximum of X, Y and Z axis'
     /// extents.
     double GetMaxExtent() const {
         return GetExtent().Max({0}).To(core::Float64).Item<double>();
     }
 
+    /// Calculates the percentage position of the given x-coordinate within
+    /// the x-axis range of this AxisAlignedBoundingBox.
     double GetXPercentage(double x) const;
 
+    /// Calculates the percentage position of the given y-coordinate within
+    /// the y-axis range of this AxisAlignedBoundingBox.
     double GetYPercentage(double y) const;
 
+    /// Calculates the percentage position of the given z-coordinate within
+    /// the z-axis range of this AxisAlignedBoundingBox.
     double GetZPercentage(double z) const;
 
     /// Returns the volume of the bounding box.
     double Volume() const {
         return GetExtent().Prod({0}).To(core::Float64).Item<double>();
     }
 
-    /// Returns the eight points that define the bounding box. The Return tensor
-    /// has shape {8, 3} and data type same as the box.
+    /// \brief Returns the eight points that define the bounding box.
+    ///
+    /// The Return tensor has shape {8, 3} and data type same as the box.
     core::Tensor GetBoxPoints() const;
 
     /// \brief Indices to points that are within the bounding box.
@@ -206,16 +213,21 @@ class AxisAlignedBoundingBox : public Geometry, public DrawableGeometry {
 
     /// Creates the axis-aligned box that encloses the set of points.
     /// \param points A list of points with data type of float32 or float64 (N x
-    /// 3 tensor, where N must be larger than 3).
+    /// 3 tensor).
     /// \return AxisAlignedBoundingBox with same data type and device as input
     /// points.
     static AxisAlignedBoundingBox CreateFromPoints(const core::Tensor &points);
 
 protected:
+    /// The device to use for the bounding box. The default is CPU:0.
     core::Device device_ = core::Device("CPU:0");
+    /// The data type of the bounding box.
     core::Dtype dtype_ = core::Float32;
+    /// The lower x, y, z bounds of the bounding box.
     core::Tensor min_bound_;
+    /// The upper x, y, z bounds of the bounding box.
     core::Tensor max_bound_;
+    /// The color of the bounding box in RGB. The default is white.
     core::Tensor color_;
 };
 
@@ -373,8 +385,9 @@ class OrientedBoundingBox : public Geometry, public DrawableGeometry {
         return GetExtent().Prod({0}).To(core::Float64).Item<double>();
     }
 
-    /// Returns the eight points that define the bounding box. The Return tensor
-    /// has shape {8, 3} and data type same as the box.
+    /// \brief Returns the eight points that define the bounding box.
+    ///
+    /// The Return tensor has shape {8, 3} and data type same as the box.
     ///
     /// \verbatim
     ///      ------- x

diff --git a/cpp/open3d/visualization/visualizer/Visualizer.h b/cpp/open3d/visualization/visualizer/Visualizer.h
@@ -127,6 +127,7 @@ class Visualizer {
     /// Visualizer should be updated accordingly.
     ///
     /// \param geometry_ptr The Geometry object.
+    /// \param reset_bounding_box Reset viewpoint to view all geometries.
     virtual bool AddGeometry(
             std::shared_ptr<const geometry::Geometry> geometry_ptr,
             bool reset_bounding_box = true);
@@ -140,6 +141,7 @@ class Visualizer {
     /// added by AddGeometry
     ///
     /// \param geometry_ptr The Geometry object.
+    /// \param reset_bounding_box Reset viewpoint to view all geometries.
     virtual bool RemoveGeometry(
             std::shared_ptr<const geometry::Geometry> geometry_ptr,
             bool reset_bounding_box = true);

diff --git a/cpp/pybind/t/geometry/boundingvolume.cpp b/cpp/pybind/t/geometry/boundingvolume.cpp
@@ -197,7 +197,7 @@ The scaling center will be the box center if it is not specified.)",
             m, "AxisAlignedBoundingBox", "create_from_points",
             {{"points",
               "A list of points with data type of float32 or float64 (N x 3 "
-              "tensor, where N must be larger than 3)."}});
+              "tensor)."}});
 
     py::class_<OrientedBoundingBox, PyGeometry<OrientedBoundingBox>,
                std::shared_ptr<OrientedBoundingBox>, Geometry, DrawableGeometry>

diff --git a/cpp/tests/t/geometry/AxisAlignedBoundingBox.cpp b/cpp/tests/t/geometry/AxisAlignedBoundingBox.cpp
@@ -32,6 +32,7 @@ INSTANTIATE_TEST_SUITE_P(
                 AxisAlignedBoundingBoxPermuteDevicePairs::TestCases()));
 
 TEST_P(AxisAlignedBoundingBoxPermuteDevices, ConstructorNoArg) {
+    using ::testing::AnyOf;
     t::geometry::AxisAlignedBoundingBox aabb;
 
     // Inherited from Geometry3D.
@@ -51,7 +52,11 @@ TEST_P(AxisAlignedBoundingBoxPermuteDevices, ConstructorNoArg) {
     EXPECT_EQ(aabb.GetDevice(), core::Device("CPU:0"));
 
     // Print Information.
-    EXPECT_EQ(aabb.ToString(), "AxisAlignedBoundingBox[Float32, CPU:0]");
+    EXPECT_THAT(
+            aabb.ToString(),  // Compiler dependent output
+            AnyOf("AxisAlignedBoundingBox[[0 0 0] - [0 0 0], Float32, CPU:0]",
+                  "AxisAlignedBoundingBox[[0.0 0.0 0.0] - [0.0 0.0 0.0], "
+                  "Float32, CPU:0]"));
 }
 
 TEST_P(AxisAlignedBoundingBoxPermuteDevices, Constructor) {
@@ -60,10 +65,6 @@ TEST_P(AxisAlignedBoundingBoxPermuteDevices, Constructor) {
     core::Tensor min_bound = core::Tensor::Init<float>({-1, -1, -1}, device);
     core::Tensor max_bound = core::Tensor::Init<float>({1, 1, 1}, device);
 
-    // Attempt to construct with invalid min/max bound.
-    EXPECT_THROW(t::geometry::AxisAlignedBoundingBox(max_bound, min_bound),
-                 std::runtime_error);
-
     t::geometry::AxisAlignedBoundingBox aabb(min_bound, max_bound);
 
     // Public members.
@@ -76,6 +77,11 @@ TEST_P(AxisAlignedBoundingBoxPermuteDevices, Constructor) {
             core::Tensor::Init<float>({1, 1, 1}, device)));
 
     EXPECT_EQ(aabb.GetDevice(), device);
+
+    // Attempt to construct with invalid min/max bound should create a valid
+    // bounding box with a warning.
+    t::geometry::AxisAlignedBoundingBox aabb_invalid(max_bound, min_bound);
+    EXPECT_TRUE(aabb_invalid.GetBoxPoints().AllClose(aabb.GetBoxPoints()));
 }
 
 TEST_P(AxisAlignedBoundingBoxPermuteDevicePairs, CopyDevice) {

diff --git a/docs/Doxyfile.in b/docs/Doxyfile.in
@@ -1146,13 +1146,6 @@ VERBATIM_HEADERS       = YES
 
 ALPHABETICAL_INDEX     = YES
 
-# The COLS_IN_ALPHA_INDEX tag can be used to specify the number of columns in
-# which the alphabetical index list will be split.
-# Minimum value: 1, maximum value: 20, default value: 5.
-# This tag requires that the tag ALPHABETICAL_INDEX is set to YES.
-
-COLS_IN_ALPHA_INDEX    = 5
-
 # In case all classes in a project start with a common prefix, all classes will
 # be put under the same header in the alphabetical index. The IGNORE_PREFIX tag
 # can be used to specify a prefix (or a list of prefixes) that should be ignored