Fix mismatching surface and bbox lists when building BVH (#272)

* Fix mismatching surface and bbox lists when building BVH * Add unit tests
doyubkim · Nov 25, 2019 · db0bc54 · db0bc54
1 parent 5a97d62
commit db0bc54
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 4 deletions.
diff --git a/src/jet/implicit_surface_set2.cpp b/src/jet/implicit_surface_set2.cpp
@@ -240,13 +240,15 @@ void ImplicitSurfaceSet2::invalidateBvh() { _bvhInvalidated = true; }
 
 void ImplicitSurfaceSet2::buildBvh() const {
     if (_bvhInvalidated) {
+        std::vector<ImplicitSurface2Ptr> surfs;
         std::vector<BoundingBox2D> bounds;
         for (size_t i = 0; i < _surfaces.size(); ++i) {
             if (_surfaces[i]->isBounded()) {
+                surfs.push_back(_surfaces[i]);
                 bounds.push_back(_surfaces[i]->boundingBox());
             }
         }
-        _bvh.build(_surfaces, bounds);
+        _bvh.build(surfs, bounds);
         _bvhInvalidated = false;
     }
 }

diff --git a/src/jet/implicit_surface_set3.cpp b/src/jet/implicit_surface_set3.cpp
@@ -240,13 +240,15 @@ void ImplicitSurfaceSet3::invalidateBvh() { _bvhInvalidated = true; }
 
 void ImplicitSurfaceSet3::buildBvh() const {
     if (_bvhInvalidated) {
+        std::vector<ImplicitSurface3Ptr> surfs;
         std::vector<BoundingBox3D> bounds;
         for (size_t i = 0; i < _surfaces.size(); ++i) {
             if (_surfaces[i]->isBounded()) {
+                surfs.push_back(_surfaces[i]);
                 bounds.push_back(_surfaces[i]->boundingBox());
             }
         }
-        _bvh.build(_surfaces, bounds);
+        _bvh.build(surfs, bounds);
         _bvhInvalidated = false;
     }
 }

diff --git a/src/jet/surface_set2.cpp b/src/jet/surface_set2.cpp
@@ -212,13 +212,15 @@ void SurfaceSet2::invalidateBvh() { _bvhInvalidated = true; }
 
 void SurfaceSet2::buildBvh() const {
     if (_bvhInvalidated) {
+        std::vector<Surface2Ptr> surfs;
         std::vector<BoundingBox2D> bounds;
         for (size_t i = 0; i < _surfaces.size(); ++i) {
             if (_surfaces[i]->isBounded()) {
+                surfs.push_back(_surfaces[i]);
                 bounds.push_back(_surfaces[i]->boundingBox());
             }
         }
-        _bvh.build(_surfaces, bounds);
+        _bvh.build(surfs, bounds);
         _bvhInvalidated = false;
     }
 }

diff --git a/src/jet/surface_set3.cpp b/src/jet/surface_set3.cpp
@@ -212,13 +212,15 @@ void SurfaceSet3::invalidateBvh() { _bvhInvalidated = true; }
 
 void SurfaceSet3::buildBvh() const {
     if (_bvhInvalidated) {
+        std::vector<Surface3Ptr> surfs;
         std::vector<BoundingBox3D> bounds;
         for (size_t i = 0; i < _surfaces.size(); ++i) {
             if (_surfaces[i]->isBounded()) {
+                surfs.push_back(_surfaces[i]);
                 bounds.push_back(_surfaces[i]->boundingBox());
             }
         }
-        _bvh.build(_surfaces, bounds);
+        _bvh.build(surfs, bounds);
         _bvhInvalidated = false;
     }
 }

diff --git a/src/tests/unit_tests/implicit_surface_set2_tests.cpp b/src/tests/unit_tests/implicit_surface_set2_tests.cpp
@@ -293,4 +293,11 @@ TEST(ImplicitSurfaceSet2, UpdateQueryEngine) {
     surfaceSet->updateQueryEngine();
     auto bbox3 = surfaceSet->boundingBox();
     EXPECT_BOUNDING_BOX2_EQ(BoundingBox2D({-4.5, 5.5}, {-3.5, 6.5}), bbox3);
+
+    // Plane is unbounded. Total bbox should ignore it.
+    auto plane = Plane2::builder().withNormal({1.0, 0.0}).makeShared();
+    surfaceSet->addExplicitSurface(plane);
+    surfaceSet->updateQueryEngine();
+    auto bbox4 = surfaceSet->boundingBox();
+    EXPECT_BOUNDING_BOX2_EQ(BoundingBox2D({-4.5, 5.5}, {-3.5, 6.5}), bbox4);
 }
diff --git a/src/tests/unit_tests/implicit_surface_set3_tests.cpp b/src/tests/unit_tests/implicit_surface_set3_tests.cpp
@@ -303,4 +303,12 @@ TEST(ImplicitSurfaceSet3, UpdateQueryEngine) {
     auto bbox3 = surfaceSet->boundingBox();
     EXPECT_BOUNDING_BOX2_EQ(BoundingBox3D({-4.5, 5.5, 10.5}, {-3.5, 6.5, 11.5}),
                             bbox3);
+
+    // Plane is unbounded. Total bbox should ignore it.
+    auto plane = Plane3::builder().withNormal({1.0, 0.0, 0.0}).makeShared();
+    surfaceSet->addExplicitSurface(plane);
+    surfaceSet->updateQueryEngine();
+    auto bbox4 = surfaceSet->boundingBox();
+    EXPECT_BOUNDING_BOX2_EQ(BoundingBox3D({-4.5, 5.5, 10.5}, {-3.5, 6.5, 11.5}),
+                            bbox4);
 }
diff --git a/src/tests/unit_tests/surface_set2_tests.cpp b/src/tests/unit_tests/surface_set2_tests.cpp
@@ -479,4 +479,11 @@ TEST(SurfaceSet2, UpdateQueryEngine) {
     surfaceSet->updateQueryEngine();
     auto bbox3 = surfaceSet->boundingBox();
     EXPECT_BOUNDING_BOX2_EQ(BoundingBox2D({-4.5, 5.5}, {-3.5, 6.5}), bbox3);
+
+    // Plane is unbounded. Total bbox should ignore it.
+    auto plane = Plane2::builder().withNormal({1.0, 0.0}).makeShared();
+    surfaceSet->addSurface(plane);
+    surfaceSet->updateQueryEngine();
+    auto bbox4 = surfaceSet->boundingBox();
+    EXPECT_BOUNDING_BOX2_EQ(BoundingBox2D({-4.5, 5.5}, {-3.5, 6.5}), bbox4);
 }
diff --git a/src/tests/unit_tests/surface_set3_tests.cpp b/src/tests/unit_tests/surface_set3_tests.cpp
@@ -487,4 +487,12 @@ TEST(SurfaceSet3, UpdateQueryEngine) {
     auto bbox3 = surfaceSet->boundingBox();
     EXPECT_BOUNDING_BOX2_EQ(BoundingBox3D({-4.5, 5.5, 10.5}, {-3.5, 6.5, 11.5}),
                             bbox3);
+
+    // Plane is unbounded. Total bbox should ignore it.
+    auto plane = Plane3::builder().withNormal({1.0, 0.0, 0.0}).makeShared();
+    surfaceSet->addSurface(plane);
+    surfaceSet->updateQueryEngine();
+    auto bbox4 = surfaceSet->boundingBox();
+    EXPECT_BOUNDING_BOX2_EQ(BoundingBox3D({-4.5, 5.5, 10.5}, {-3.5, 6.5, 11.5}),
+                            bbox4);
 }