From 81c34bfdb07b282bb38fd576f7ee4af09b70dea6 Mon Sep 17 00:00:00 2001 From: Dan Macumber Date: Sun, 22 Oct 2023 11:15:57 -0600 Subject: [PATCH 1/6] Allow surfaces with holes to have edges with same surface on both sides --- src/utilities/geometry/Polyhedron.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index 71fb420ab5b..a839968b266 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -242,9 +242,6 @@ void Polyhedron::performEdgeMatching() { for (size_t i = 0; i < m_surfaces.size(); ++i) { for (size_t j = 0; j < m_surfaces.size(); ++j) { - if (i == j) { - continue; - } auto& surface1 = m_surfaces[i]; auto& surface2 = m_surfaces[j]; for (Surface3dEdge& edge1 : surface1.edges) { From ea8416ee9dd7202aa152a7a31bd34308322637b8 Mon Sep 17 00:00:00 2001 From: Dan Macumber Date: Sun, 22 Oct 2023 11:16:20 -0600 Subject: [PATCH 2/6] Fix compile error for VS 2019 --- src/cli/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/main.cpp b/src/cli/main.cpp index 24d99cdb6e4..0eb655caa49 100644 --- a/src/cli/main.cpp +++ b/src/cli/main.cpp @@ -112,7 +112,7 @@ int main(int argc, char* argv[]) { ->add_option_function( "-l,--loglevel", [](const LogLevel& level) { - fmt::print("Setting Log Level to {} ({})\n", logLevelStrs[static_cast(level) - static_cast(LogLevel::Trace)], level); + fmt::print("Setting Log Level to {} ({})\n", logLevelStrs[static_cast(level) - static_cast(LogLevel::Trace)], std::to_string(level)); openstudio::Logger::instance().standardOutLogger().setLogLevel(level); }, "LogLevel settings: One of {Trace, Debug, Info, Warn, Error, Fatal} [Default: Warn]") From aa5955e4ab1d126cc69146bf23e1b98cae1fedc2 Mon Sep 17 00:00:00 2001 From: Dan Macumber Date: Sun, 22 Oct 2023 17:45:35 -0600 Subject: [PATCH 3/6] Update implementation and add new test case --- src/utilities/geometry/Polyhedron.cpp | 22 ++++++++ .../geometry/Test/Polyhedron_GTest.cpp | 50 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index a839968b266..1b83adeac16 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -242,6 +242,9 @@ void Polyhedron::performEdgeMatching() { for (size_t i = 0; i < m_surfaces.size(); ++i) { for (size_t j = 0; j < m_surfaces.size(); ++j) { + if (i == j) { + continue; + } auto& surface1 = m_surfaces[i]; auto& surface2 = m_surfaces[j]; for (Surface3dEdge& edge1 : surface1.edges) { @@ -265,6 +268,25 @@ void Polyhedron::performEdgeMatching() { } } } + + // special case to find edges that are used to "cut" in to a surface to remove an interior hole + // we allow these edges to double count the first surface since they bound the same surface on two sides + for (auto& surface : m_surfaces) { + auto& edges = surface.edges; + for (size_t i = 0; i < edges.size(); ++i) { + for (size_t j = 0; j < edges.size(); ++j) { + if (i == j) { + continue; + } + if ((edges[i].count() == 1) && (edges[j].count() == 1) && (edges[i] == edges[j]) && edges[i].reverseEqual(edges[j])) { + // appendSurface will allow use to check edge.count() later to check if count == 2. + // All edges must be count == 2 in an Enclosed Polyhedron + edges[i].appendSurface(surface); + edges[j].appendSurface(surface); + } + } + } + } } void Polyhedron::resetEdgeMatching() { diff --git a/src/utilities/geometry/Test/Polyhedron_GTest.cpp b/src/utilities/geometry/Test/Polyhedron_GTest.cpp index 8d88efb565f..601485006ab 100644 --- a/src/utilities/geometry/Test/Polyhedron_GTest.cpp +++ b/src/utilities/geometry/Test/Polyhedron_GTest.cpp @@ -128,6 +128,56 @@ TEST_F(GeometryFixture, Polyhedron_Titled_Roof) { EXPECT_DOUBLE_EQ(volume, zonePoly.calcDivergenceTheoremVolume()); } +TEST_F(GeometryFixture, Polyhedron_Box_With_Hole) { + + // This is a 10x10x1 box, with a 5x5x1 hole cut in the middle + + // We put extra vertices here to skew the calculate that Space::volume does + const Surface3d roof( + {{+10.0, +0.0, +1.0}, {+10.0, +10.0, +1.0}, {+0.0, +10.0, +1.0}, {+0.0, +0.0, +1.0}, + {+2.50, +2.5, +1.0}, {+2.50, +7.50, +1.0}, {+7.5, +7.50, +1.0}, {+7.5, +2.5, +1.0}, {+2.50, +2.5, +1.0}, {+0.0, +0.0, +1.0}}, "ROOF", 1); + EXPECT_FALSE(roof.isConvex()); + + const Surface3d east1({{+10.0, +0.0, +1.0}, {+10.0, +0.0, +0.0}, {+10.0, +10.0, +0.0}, {+10.0, +10.0, +1.0}}, "EAST1", 2); + EXPECT_TRUE(east1.isConvex()); + + const Surface3d east2({{+7.50, +7.50, +1.0}, {+7.50, +7.50, +0.0}, {+7.50, +2.5, +0.0},{+7.50, +2.5, +1.0}}, "EAST2", 7); + EXPECT_TRUE(east2.isConvex()); + + const Surface3d north1({{+10.0, +10.0, +1.0}, {+10.0, +10.0, +0.0}, {+0.0, +10.0, +0.0}, {+0.0, +10.0, +1.0}}, "NORTH1", 3); + EXPECT_TRUE(north1.isConvex()); + + const Surface3d north2({{+2.5, +7.50, +1.0}, {+2.5, +7.50, +0.0}, {+7.50, +7.50, +0.0}, {+7.50, +7.50, +1.0}}, "NORTH2", 8); + EXPECT_TRUE(north2.isConvex()); + + const Surface3d west1({{+0.0, +10.0, +1.0}, {+0.0, +10.0, +0.0}, {+0.0, +0.0, +0.0}, {+0.0, +0.0, +1.0}}, "WEST1", 4); + EXPECT_TRUE(west1.isConvex()); + + const Surface3d west2({{+2.5, +2.5, +1.0}, {+2.5, +2.5, +0.0}, {+2.5, +7.50, +0.0}, {+2.5, +7.50, +1.0}}, "WEST2", 9); + EXPECT_TRUE(west2.isConvex()); + + const Surface3d south1({{+0.0, +0.0, +1.0}, {+0.0, +0.0, +0.0}, {+10.0, +0.0, +0.0}, {+10.0, +0.0, +1.0}}, "SOUTH1", 5); + EXPECT_TRUE(south1.isConvex()); + + const Surface3d south2({{+7.5, +2.5, +1.0}, {+7.5, +2.5, +0.0}, {+2.5, +2.5, +0.0}, {+2.5, +2.5, +1.0}}, "SOUTH2", 10); + EXPECT_TRUE(south2.isConvex()); + + const Surface3d floor( + {{+0.0, +10.0, +0.0}, {+10.0, +10.0, +0.0}, {+10.0, +0.0, +0.0}, {+0.0, +0.0, +0.0}, + {+2.50, +2.5, +0.0}, {+7.50, +2.50, +0.0}, {+7.50, +7.5, +0.0}, {+2.50, +7.50, +0.0}, {+2.50, +2.5, +0.0}, {+0.0, +0.0, +0.0}}, "FLOOR", 6); + EXPECT_FALSE(floor.isConvex()); + + const Polyhedron zonePoly({south1, north1, east1, west1, roof, floor, south2, north2, east2, west2}); + + EXPECT_TRUE(zonePoly.isEnclosedVolume()); + EXPECT_TRUE(zonePoly.edgesNotTwo().empty()); + EXPECT_FALSE(zonePoly.hasAnySurfaceWithIncorrectOrientation()); + + constexpr double volume = 10.0 * 10.0 * 1.0 - 5.0 * 5.0 * 1.0; + EXPECT_DOUBLE_EQ(volume, zonePoly.polyhedronVolume()); + EXPECT_DOUBLE_EQ(volume, zonePoly.calcDivergenceTheoremVolume()); +} + TEST_F(GeometryFixture, Surface3d_Convexity) { const Point3d p0{}; for (size_t i = 3; i <= 16; ++i) { From eaef5132acefaabce98e91e7173347787592f7b2 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Sun, 29 Oct 2023 10:50:06 +0100 Subject: [PATCH 4/6] Minor udpate to a comment --- src/utilities/geometry/Polyhedron.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index 1b83adeac16..c2de558cb9e 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -269,7 +269,7 @@ void Polyhedron::performEdgeMatching() { } } - // special case to find edges that are used to "cut" in to a surface to remove an interior hole + // #5002 - special case to find edges that are used to "cut" in to a surface to remove an interior hole // we allow these edges to double count the first surface since they bound the same surface on two sides for (auto& surface : m_surfaces) { auto& edges = surface.edges; From f2c25372e25533c98c8015cb874769600bf779e1 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 30 Oct 2023 14:07:03 +0100 Subject: [PATCH 5/6] Use **Combinations** rather than **Permutations** when matching surfaces/edges Testbed / demo: https://godbolt.org/z/E7n88WP34 --- src/utilities/geometry/Polyhedron.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index c2de558cb9e..b2a2c3f4d91 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -111,6 +111,8 @@ std::ostream& operator<<(std::ostream& os, const Surface3dEdge& edge) { Surface3d::Surface3d(std::vector t_vertices, std::string t_name, size_t t_surfNum) : vertices(std::move(t_vertices)), name(std::move(t_name)), surfNum(t_surfNum) { + + edges.reserve(vertices.size()); for (auto it = vertices.begin(); it != vertices.end(); ++it) { auto itnext = std::next(it); @@ -240,11 +242,9 @@ void Polyhedron::performEdgeMatching() { m_hasAnySurfaceWithIncorrectOrientation = false; - for (size_t i = 0; i < m_surfaces.size(); ++i) { - for (size_t j = 0; j < m_surfaces.size(); ++j) { - if (i == j) { - continue; - } + // We use **Combinations** (rather than Permutations) to avoid traversing unnecessarily + for (size_t i = 0; i < m_surfaces.size() - 1; ++i) { + for (size_t j = i + 1; j < m_surfaces.size(); ++j) { auto& surface1 = m_surfaces[i]; auto& surface2 = m_surfaces[j]; for (Surface3dEdge& edge1 : surface1.edges) { @@ -273,11 +273,8 @@ void Polyhedron::performEdgeMatching() { // we allow these edges to double count the first surface since they bound the same surface on two sides for (auto& surface : m_surfaces) { auto& edges = surface.edges; - for (size_t i = 0; i < edges.size(); ++i) { - for (size_t j = 0; j < edges.size(); ++j) { - if (i == j) { - continue; - } + for (size_t i = 0; i < edges.size() - 1; ++i) { + for (size_t j = i + 1; j < edges.size(); ++j) { if ((edges[i].count() == 1) && (edges[j].count() == 1) && (edges[i] == edges[j]) && edges[i].reverseEqual(edges[j])) { // appendSurface will allow use to check edge.count() later to check if count == 2. // All edges must be count == 2 in an Enclosed Polyhedron From d746a33953ef84ac6cfca16c4f5581e663ab56ce Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 30 Oct 2023 15:25:11 +0100 Subject: [PATCH 6/6] Protect against empty vector... static_cast(-1) is a very large unsigned number. We'll just be iterating an integer once too many time, which is totally unoticable --- src/utilities/geometry/Polyhedron.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index b2a2c3f4d91..2afd1373132 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -243,7 +243,7 @@ void Polyhedron::performEdgeMatching() { m_hasAnySurfaceWithIncorrectOrientation = false; // We use **Combinations** (rather than Permutations) to avoid traversing unnecessarily - for (size_t i = 0; i < m_surfaces.size() - 1; ++i) { + for (size_t i = 0; i < m_surfaces.size(); ++i) { for (size_t j = i + 1; j < m_surfaces.size(); ++j) { auto& surface1 = m_surfaces[i]; auto& surface2 = m_surfaces[j]; @@ -273,7 +273,7 @@ void Polyhedron::performEdgeMatching() { // we allow these edges to double count the first surface since they bound the same surface on two sides for (auto& surface : m_surfaces) { auto& edges = surface.edges; - for (size_t i = 0; i < edges.size() - 1; ++i) { + for (size_t i = 0; i < edges.size(); ++i) { for (size_t j = i + 1; j < edges.size(); ++j) { if ((edges[i].count() == 1) && (edges[j].count() == 1) && (edges[i] == edges[j]) && edges[i].reverseEqual(edges[j])) { // appendSurface will allow use to check edge.count() later to check if count == 2.