Merge pull request #4431 from NREL/issue-4374

Fixed an issue in Surface splitSurfaceForSubSurfaces which caused the…
NREL · Oct 5, 2021 · bcc562e · bcc562e
2 parents 65607ae + e732db8
commit bcc562e
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 4 deletions.
diff --git a/src/model/Surface.cpp b/src/model/Surface.cpp
@@ -1761,17 +1761,18 @@ namespace model {
 
         std::vector<Point3dVector> tmpFaces;
         unsigned numIntersects = 0;
-        for (const Point3dVector& face : newFaces) {
+        for (const Point3dVector& newFace : newFaces) {
           // each mask should intersect one and only one newFace
-          boost::optional<IntersectionResult> intersection = openstudio::intersect(faceVertices, mask, tol);
+          boost::optional<IntersectionResult> intersection = openstudio::intersect(newFace, mask, tol);
+
           if (intersection) {
             numIntersects += 1;
             tmpFaces.push_back(intersection->polygon1());
             for (const Point3dVector& tmpFace : intersection->newPolygons1()) {
               tmpFaces.push_back(tmpFace);
             }
           } else {
-            tmpFaces.push_back(face);
+            tmpFaces.push_back(newFace);
           }
         }
 
@@ -1787,17 +1788,19 @@ namespace model {
       // sort new faces in descending order by area
       std::sort(newFaces.begin(), newFaces.end(), PolygonAreaGreater());
 
-      // loop over all new faces
+      // loop over all new faces to create new Surfaces
       bool changedThis = false;
       unsigned numReparented = 0;
       Model model = this->model();
       for (const Point3dVector& newFace : newFaces) {
 
         boost::optional<Surface> surface;
         if (!changedThis) {
+          // Re-use the original surface
           changedThis = true;
           surface = getObject<Surface>();
         } else {
+          // Create new surfaces
           boost::optional<ModelObject> object = this->clone(model);
           OS_ASSERT(object);
           surface = object->optionalCast<Surface>();

diff --git a/src/model/test/Surface_GTest.cpp b/src/model/test/Surface_GTest.cpp
@@ -39,6 +39,7 @@
 #include "../BuildingStory_Impl.hpp"
 #include "../FoundationKiva.hpp"
 #include "../Space.hpp"
+#include "../ThermalZone.hpp"
 #include "../Space_Impl.hpp"
 #include "../Surface.hpp"
 #include "../Surface_Impl.hpp"
@@ -3684,3 +3685,78 @@ TEST_F(ModelFixture, Surface_isXXXAutocalculated_4399) {
   surface.autocalculateNumberofVertices();
   EXPECT_TRUE(surface.isNumberofVerticesAutocalculated());
 }
+
+TEST_F(ModelFixture, Issue_4374) {
+  Model model;
+  Space space(model);
+  //ThermalZone zone(model);
+  //space.setThermalZone(zone);
+
+  // Lets say we create a surface and add three subsurfaces to it and then call the
+  // split surface for sub surfaces method to see what happens, eh!
+  // We could even repro one of the surfaces form the school ref building
+  Point3dVector vertices;
+  vertices.push_back(Point3d(0, 0, 4));
+  vertices.push_back(Point3d(0, 0, 0));
+  vertices.push_back(Point3d(53, 0, 0));
+  vertices.push_back(Point3d(53, 0, 4));
+  Surface wall = Surface(vertices, model);
+  wall.setParent(space);
+  wall.setSurfaceType("Wall");
+
+  //vertices.clear();
+  //vertices.push_back(Point3d(17.0, 0, 2.512845));
+  //vertices.push_back(Point3d(0.05, 0, 0.922073));
+  //vertices.push_back(Point3d(1.2692, 0, 0.922073));
+  //vertices.push_back(Point3d(1.2692, 0, 2.512845));
+  //SubSurface window1 = SubSurface(vertices, model);
+  //window1.setSubSurfaceType("Door");
+  //window1.setSurface(wall);
+
+  //vertices.clear();
+  //vertices.push_back(Point3d(2.4884, 0, 2.512845));
+  //vertices.push_back(Point3d(2.4884, 0, 0.922073));
+  //vertices.push_back(Point3d(10.95, 0, 0.922073));
+  //vertices.push_back(Point3d(10.95, 0, 2.512845));
+  //SubSurface window2 = SubSurface(vertices, model);
+  //window2.setSubSurfaceType("FixedWindow");
+  //window2.setSurface(wall);
+
+  vertices.clear();
+  vertices.push_back(Point3d(17.0462, 0, 2.1336));
+  vertices.push_back(Point3d(17.0462, 0, 0));
+  vertices.push_back(Point3d(17.9606, 0, 0));
+  vertices.push_back(Point3d(17.9606, 0, 2.1336));
+  SubSurface door = SubSurface(vertices, model);
+  door.setSubSurfaceType("Door");
+  door.setSurface(wall);
+
+  vertices.clear();
+  vertices.push_back(Point3d(35.0787, 0, 2.1336));
+  vertices.push_back(Point3d(35.0787, 0, 0));
+  vertices.push_back(Point3d(35.9931, 0, 0));
+  vertices.push_back(Point3d(35.9931, 0, 2.1336));
+  SubSurface door1 = SubSurface(vertices, model);
+  door1.setSubSurfaceType("Door");
+  door1.setSurface(wall);
+
+  // Wall will have two sub surfaces
+  EXPECT_EQ(wall.subSurfaces().size(), 2);
+
+  // Subdiuvide the wall surface
+  auto surfaces = wall.splitSurfaceForSubSurfaces();
+  surfaces.push_back(wall);
+
+  // Result should be four new surfaces plus the original surface
+  EXPECT_EQ(surfaces.size(), 5);
+
+  // Chekc the surfaces
+  for (auto surface : surfaces) {
+    std::string surfName = *surface.name();
+    std::string surfType = surface.surfaceType();
+    auto vertices = surface.vertices();
+    LOG(Info, vertices);
+    // Each surface should have no more than one sub surface
+    EXPECT_TRUE(surface.subSurfaces().size() <= 1);
+  }
+}