From 2cdc9c2f52d4b290718cc161ca1df3d7eb6ab5c3 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 5 Sep 2023 09:31:49 +0200 Subject: [PATCH 1/5] Fix #4957 - Put back the copy ctor for SWIG for Point3d, Vector3d, Plane and Transformation --- src/utilities/geometry/Plane.hpp | 10 +++++----- src/utilities/geometry/Point3d.hpp | 10 +++++----- src/utilities/geometry/Transformation.hpp | 10 +++++----- src/utilities/geometry/Vector3d.hpp | 10 +++++----- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/utilities/geometry/Plane.hpp b/src/utilities/geometry/Plane.hpp index 8166c2e6887..64e95c4ed80 100644 --- a/src/utilities/geometry/Plane.hpp +++ b/src/utilities/geometry/Plane.hpp @@ -33,12 +33,12 @@ class UTILITIES_API Plane /// throws openstudio::Exception if cannot compute plane for these points. Plane(const std::vector& points); - // Copy and move operators are implicitly declared (Rule of 1) + // Copy and move operators are implicitly declared (Rule of 1), but we want the copy ctor for SWIG so we have to define all of them // There's no need to check if the length of the normal is zero since we never allow another plane to not satisfy this condition - // Plane(const Plane& other) = default; - // Plane(Plane&& other) = default; - // Plane& operator=(const Plane&) = default; - // Plane& operator=(Plane&&) = default; + Plane(const Plane& other) = default; + Plane(Plane&& other) noexcept = default; + Plane& operator=(const Plane&) = default; + Plane& operator=(Plane&&) noexcept = default; // ~Plane() noexcept = default; /// get the outward normal of this plane diff --git a/src/utilities/geometry/Point3d.hpp b/src/utilities/geometry/Point3d.hpp index 46f0cfe6094..7bd1a0dcc68 100644 --- a/src/utilities/geometry/Point3d.hpp +++ b/src/utilities/geometry/Point3d.hpp @@ -27,11 +27,11 @@ class UTILITIES_API Point3d /// constructor with x, y, z Point3d(double x, double y, double z); - // Copy and move operators are implicitly declared - // Point3d(const Point3d& other) = default; - // Point3d(Point3d&& other) = default; - // Point3d& operator=(const Point3d&) = default; - // Point3d& operator=(Point3d&&) = default; + // Copy and move operators are implicitly declared, but we want the copy ctor for SWIG so we have to define all of them + Point3d(const Point3d& other) = default; + Point3d(Point3d&& other) noexcept = default; + Point3d& operator=(const Point3d&) = default; + Point3d& operator=(Point3d&&) noexcept = default; // ~Point3d() noexcept = default; /// get x diff --git a/src/utilities/geometry/Transformation.hpp b/src/utilities/geometry/Transformation.hpp index b336d1168c5..9f1e82f6f9e 100644 --- a/src/utilities/geometry/Transformation.hpp +++ b/src/utilities/geometry/Transformation.hpp @@ -35,11 +35,11 @@ class UTILITIES_API Transformation /// constructor from storage, asserts vector is size 16 Transformation(const Vector& vector); - // Copy and move operators are implicitly declared (Rule of 1) - // Transformation(const Transformation& other) = default; - // Transformation(Transformation&& other) = default; - // Transformation& operator=(const Transformation&) = default; - // Transformation& operator=(Transformation&&) = default; + // Copy and move operators are implicitly declared (Rule of 1), but we want the copy ctor for SWIG so we have to define all of them + Transformation(const Transformation& other) = default; + Transformation(Transformation&& other) noexcept = default; + Transformation& operator=(const Transformation&) = default; + Transformation& operator=(Transformation&&) noexcept = default; // ~Transformation() noexcept = default; /// rotation about origin defined by axis and angle (radians) diff --git a/src/utilities/geometry/Vector3d.hpp b/src/utilities/geometry/Vector3d.hpp index b036f01dfbc..f85b0d447cb 100644 --- a/src/utilities/geometry/Vector3d.hpp +++ b/src/utilities/geometry/Vector3d.hpp @@ -24,11 +24,11 @@ class UTILITIES_API Vector3d /// constructor with x, y, z Vector3d(double x, double y, double z); - // Copy and move operators are implicitly declared (Rule of 1) - // Vector3d(const Vector3d& other) = default; - // Vector3d(Vector3d&& other) = default; - // Vector3d& operator=(const Vector3d&) = default; - // Vector3d& operator=(Vector3d&&) = default; + // Copy and move operators are implicitly declared (Rule of 1), but we want the copy ctor for SWIG so we have to define all of them + Vector3d(const Vector3d& other) = default; + Vector3d(Vector3d&& other) noexcept = default; + Vector3d& operator=(const Vector3d&) = default; + Vector3d& operator=(Vector3d&&) noexcept = default; // ~Vector3d() noexcept = default; /// get x From 1b93313345f9507f457ee92ff2381d35f6d5db33 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 5 Sep 2023 09:34:37 +0200 Subject: [PATCH 2/5] Fix a use after move in Surface3d --- 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 bdf3293b574..0d81402f7c6 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -118,7 +118,7 @@ Surface3d::Surface3d(std::vector t_vertices, std::string t_name, size_t itnext = std::begin(vertices); } - edges.emplace_back(*it, *itnext, t_name, t_surfNum); + edges.emplace_back(*it, *itnext, name, surfNum); } } From 2601399e55ba17636317ad42f8a7b62fb7e8dc3a Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 5 Sep 2023 09:35:17 +0200 Subject: [PATCH 3/5] Add Polyhedron::surface3ds for inspection / easier debugging --- src/utilities/geometry/Polyhedron.cpp | 4 ++++ src/utilities/geometry/Polyhedron.hpp | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index 0d81402f7c6..d603e67314c 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -492,4 +492,8 @@ double Polyhedron::calcDivergenceTheoremVolume() const { return volume; } +std::vector Polyhedron::surface3ds() const { + return m_surfaces; +} + } // namespace openstudio diff --git a/src/utilities/geometry/Polyhedron.hpp b/src/utilities/geometry/Polyhedron.hpp index fa8baf738d9..643ae82d456 100644 --- a/src/utilities/geometry/Polyhedron.hpp +++ b/src/utilities/geometry/Polyhedron.hpp @@ -138,6 +138,8 @@ class UTILITIES_API Polyhedron * proportion of conflicted edges / total number of edges */ std::vector findSurfacesWithIncorrectOrientation() const; + std::vector surface3ds() const; + protected: void performEdgeMatching(); void resetEdgeMatching(); From 9c8001da9080a75f63cdcf9adc03a956b13201ac Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 5 Sep 2023 09:58:16 +0200 Subject: [PATCH 4/5] Re-add copy ctor for Polygon3d for consistency --- src/utilities/geometry/Polygon3d.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/utilities/geometry/Polygon3d.hpp b/src/utilities/geometry/Polygon3d.hpp index fdabe3f2d30..d85c9729abb 100644 --- a/src/utilities/geometry/Polygon3d.hpp +++ b/src/utilities/geometry/Polygon3d.hpp @@ -27,11 +27,11 @@ class UTILITIES_API Polygon3d // Constructs a polygon with an outer path and one or more inner paths Polygon3d(const Point3dVector& outerPath, const Point3dVectorVector& innerPaths); - // Copy and move operators are implicitly declared (Rule of 1) - // Polygon3d(const Polygon3d& other) = default; - // Polygon3d(Polygon3d&& other) = default; - // Polygon3d& operator=(const Polygon3d&) = default; - // Polygon3d& operator=(Polygon3d&&) = default; + // Copy and move operators are implicitly declared (Rule of 1), but we want the copy ctor for SWIG so we have to define all of them + Polygon3d(const Polygon3d& other) = default; + Polygon3d(Polygon3d&& other) noexcept = default; + Polygon3d& operator=(const Polygon3d&) = default; + Polygon3d& operator=(Polygon3d&&) noexcept = default; // ~Polygon3d() noexcept = default; // Assigns an outer path for the polygon From f9d1e88d69954fcb3c4937048cd609f18bd0e553 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 5 Sep 2023 09:59:36 +0200 Subject: [PATCH 5/5] Add an ostream operator<< for Surface3d for convenience too. --- src/utilities/geometry/Geometry.i | 8 ++++++++ src/utilities/geometry/Polyhedron.cpp | 17 +++++++++++++++-- src/utilities/geometry/Polyhedron.hpp | 1 + 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/utilities/geometry/Geometry.i b/src/utilities/geometry/Geometry.i index 9a1d18f693a..bcbdf86ab06 100644 --- a/src/utilities/geometry/Geometry.i +++ b/src/utilities/geometry/Geometry.i @@ -169,6 +169,14 @@ } } +%extend openstudio::Surface3d { + std::string __str__() const { + std::ostringstream os; + os << *self; + return os.str(); + } +} + %extend openstudio::Transformation { std::string __str__() const { diff --git a/src/utilities/geometry/Polyhedron.cpp b/src/utilities/geometry/Polyhedron.cpp index d603e67314c..71fb420ab5b 100644 --- a/src/utilities/geometry/Polyhedron.cpp +++ b/src/utilities/geometry/Polyhedron.cpp @@ -104,8 +104,8 @@ Vector3d Surface3dEdge::asVector() const { } std::ostream& operator<<(std::ostream& os, const Surface3dEdge& edge) { - os << "Surface3dEdge: start=" << edge.start() << ", end=" << edge.end() << ", count=" << edge.count() - << ", firstSurface=" << edge.firstSurfaceName(); + os << "Surface3dEdge: start=" << edge.start() << ", end=" << edge.end() << ", count=" << edge.count() << ", firstSurface='" + << edge.firstSurfaceName() << "'"; return os; } @@ -122,6 +122,19 @@ Surface3d::Surface3d(std::vector t_vertices, std::string t_name, size_t } } +std::ostream& operator<<(std::ostream& os, const Surface3d& surface3d) { + os << "Surface3d "; + if (!surface3d.name.empty()) { + os << "'" << surface3d.name << "' "; + } + os << "= [\n"; + for (const auto& pt : surface3d.vertices) { + os << " " << pt << ",\n"; + } + os << "]"; + return os; +} + bool Surface3d::operator<(const Surface3d& rhs) const { return this->name < rhs.name; } diff --git a/src/utilities/geometry/Polyhedron.hpp b/src/utilities/geometry/Polyhedron.hpp index 643ae82d456..df93c8ae26e 100644 --- a/src/utilities/geometry/Polyhedron.hpp +++ b/src/utilities/geometry/Polyhedron.hpp @@ -166,6 +166,7 @@ using PolyhedronVector = std::vector; /// ostream operator UTILITIES_API std::ostream& operator<<(std::ostream& os, const Surface3dEdge& edge); +UTILITIES_API std::ostream& operator<<(std::ostream& os, const Surface3d& surface3d); } // namespace openstudio