Improve handling of invalid shape construction.

bindings/c/conv.cpp

@@ -96,6 +96,9 @@ ManifoldError to_c(manifold::Manifold::Error error) {
     case Manifold::Error::FaceIDWrongLength:
       e = MANIFOLD_FACE_ID_WRONG_LENGTH;
       break;
+    case Manifold::Error::InvalidConstruction:
+      e = MANIFOLD_INVALID_CONSTRUCTION;
+      break;
   };
   return e;
 }

bindings/c/include/types.h

@@ -74,6 +74,7 @@ typedef enum ManifoldError {
   MANIFOLD_TRANSFORM_WRONG_LENGTH,
   MANIFOLD_RUN_INDEX_WRONG_LENGTH,
   MANIFOLD_FACE_ID_WRONG_LENGTH,
+  MANIFOLD_INVALID_CONSTRUCTION,
 } ManifoldError;
 
 typedef enum ManifoldFillRule {

src/cross_section/src/cross_section.cpp

@@ -17,6 +17,7 @@
 #include <clipper2/clipper.h>
 
 #include <memory>
+#include <tuple>
 #include <vector>
 
 #include "clipper2/clipper.core.h"
@@ -236,21 +237,30 @@ C2::PathsD CrossSection::GetPaths() const {
  * @param center Set to true to shift the center to the origin.
  */
 CrossSection CrossSection::Square(const glm::vec2 dims, bool center) {
+  if (glm::length(dims) == 0.0f) {
+    return CrossSection();
+  }
+
+  // reverse winding if dimensions are negative
+  const bool neg = dims.x * dims.y < 0.0f;
+  const int start = neg ? 3 : 0;
+  const int step = neg ? -1 : 1;
+
   auto p = C2::PathD(4);
   if (center) {
-    auto w = dims.x / 2;
-    auto h = dims.y / 2;
-    p[0] = C2::PointD(w, h);
-    p[1] = C2::PointD(-w, h);
-    p[2] = C2::PointD(-w, -h);
-    p[3] = C2::PointD(w, -h);
+    const auto w = dims.x / 2;
+    const auto h = dims.y / 2;
+    p[start + step * 0] = C2::PointD(w, h);
+    p[start + step * 1] = C2::PointD(-w, h);
+    p[start + step * 2] = C2::PointD(-w, -h);
+    p[start + step * 3] = C2::PointD(w, -h);
   } else {
-    double x = dims.x;
-    double y = dims.y;
-    p[0] = C2::PointD(0.0, 0.0);
-    p[1] = C2::PointD(x, 0.0);
-    p[2] = C2::PointD(x, y);
-    p[3] = C2::PointD(0.0, y);
+    const double x = dims.x;
+    const double y = dims.y;
+    p[start + step * 0] = C2::PointD(0.0, 0.0);
+    p[start + step * 1] = C2::PointD(x, 0.0);
+    p[start + step * 2] = C2::PointD(x, y);
+    p[start + step * 3] = C2::PointD(0.0, y);
   }
   return CrossSection(C2::PathsD{p});
 }
@@ -263,6 +273,9 @@ CrossSection CrossSection::Square(const glm::vec2 dims, bool center) {
  * calculated by the static Quality defaults according to the radius.
  */
 CrossSection CrossSection::Circle(float radius, int circularSegments) {
+  if (radius <= 0.0f) {
+    return CrossSection();
+  }
   int n = circularSegments > 2 ? circularSegments
                                : Quality::GetCircularSegments(radius);
   float dPhi = 360.0f / n;

src/manifold/include/manifold.h

@@ -97,6 +97,7 @@ class Manifold {
     TransformWrongLength,
     RunIndexWrongLength,
     FaceIDWrongLength,
+    InvalidConstruction,
   };
   Error Status() const;
   int NumVert() const;
@@ -171,14 +172,11 @@ class Manifold {
  private:
   Manifold(std::shared_ptr<CsgNode> pNode_);
   Manifold(std::shared_ptr<Impl> pImpl_);
+  static Manifold Invalid();
 
   mutable std::shared_ptr<CsgNode> pNode_;
 
   CsgLeafNode& GetCsgLeafNode() const;
-
-  static int circularSegments_;
-  static float circularAngle_;
-  static float circularEdgeLength_;
 };
 /** @} */
 }  // namespace manifold

src/manifold/src/constructors.cpp

@@ -149,6 +149,9 @@ Manifold Manifold::Tetrahedron() {
  * @param center Set to true to shift the center to the origin.
  */
 Manifold Manifold::Cube(glm::vec3 size, bool center) {
+  if (glm::length(size) == 0.) {
+    return Invalid();
+  }
   auto cube = Manifold(std::make_shared<Impl>(Impl::Shape::Cube));
   cube = cube.Scale(size);
   if (center) cube = cube.Translate(-size / 2.0f);
@@ -170,6 +173,9 @@ Manifold Manifold::Cube(glm::vec3 size, bool center) {
  */
 Manifold Manifold::Cylinder(float height, float radiusLow, float radiusHigh,
                             int circularSegments, bool center) {
+  if (height <= 0.0f || radiusLow <= 0.0f) {
+    return Invalid();
+  }
   float scale = radiusHigh >= 0.0f ? radiusHigh / radiusLow : 1.0f;
   float radius = fmax(radiusLow, radiusHigh);
   int n = circularSegments > 2 ? circularSegments
@@ -195,6 +201,9 @@ Manifold Manifold::Cylinder(float height, float radiusLow, float radiusHigh,
  * calculated by the static Defaults.
  */
 Manifold Manifold::Sphere(float radius, int circularSegments) {
+  if (radius <= 0.0f) {
+    return Invalid();
+  }
   int n = circularSegments > 0 ? (circularSegments + 3) / 4
                                : Quality::GetCircularSegments(radius) / 4;
   auto pImpl_ = std::make_shared<Impl>(Impl::Shape::Octahedron);
@@ -226,6 +235,9 @@ Manifold Manifold::Extrude(const CrossSection& crossSection, float height,
                            int nDivisions, float twistDegrees,
                            glm::vec2 scaleTop) {
   auto polygons = crossSection.ToPolygons();
+  if (polygons.size() == 0 || height <= 0.0f) {
+    return Invalid();
+  }
 
   scaleTop.x = glm::max(scaleTop.x, 0.0f);
   scaleTop.y = glm::max(scaleTop.y, 0.0f);
@@ -307,6 +319,9 @@ Manifold Manifold::Extrude(const CrossSection& crossSection, float height,
 Manifold Manifold::Revolve(const CrossSection& crossSection,
                            int circularSegments) {
   auto polygons = crossSection.ToPolygons();
+  if (polygons.size() == 0) {
+    return Invalid();
+  }
 
   float radius = 0.0f;
   for (const auto& poly : polygons) {

src/manifold/src/manifold.cpp

@@ -71,6 +71,12 @@ Manifold::Manifold(std::shared_ptr<CsgNode> pNode) : pNode_(pNode) {}
 Manifold::Manifold(std::shared_ptr<Impl> pImpl_)
     : pNode_(std::make_shared<CsgLeafNode>(pImpl_)) {}
 
+Manifold Manifold::Invalid() {
+  auto pImpl_ = std::make_shared<Impl>();
+  pImpl_->status_ = Error::InvalidConstruction;
+  return Manifold(pImpl_);
+}
+
 Manifold& Manifold::operator=(const Manifold& other) {
   if (this != &other) {
     pNode_ = other.pNode_;

test/cross_section_test.cpp

@@ -28,6 +28,24 @@
 
 using namespace manifold;
 
+TEST(CrossSection, Square) {
+  auto sq = CrossSection::Square({5, 5});
+  auto neg_x_sq = CrossSection::Square({-5, 5});
+  auto neg_y_sq = CrossSection::Square({5, -5});
+  auto neg_xy_sq = CrossSection::Square({-5, -5});
+
+  auto cb = Manifold::Cube({5, 5, 5});
+  auto neg_x_cb = Manifold::Cube({-5, 5, 5});
+  auto neg_y_cb = Manifold::Cube({5, -5, 5});
+  auto neg_xy_cb = Manifold::Cube({-5, -5, 5});
+
+  auto cubes = cb + neg_x_cb + neg_y_cb + neg_xy_cb;
+  auto ex = Manifold::Extrude(sq + neg_x_sq + neg_y_sq + neg_xy_sq, 5);
+
+  // ensure semantics for negative dimensions for square and cube agree
+  EXPECT_FLOAT_EQ((cubes - ex).GetProperties().volume, 0.);
+}
+
 TEST(CrossSection, MirrorUnion) {
   auto a = CrossSection::Square({5., 5.}, true);
   auto b = a.Translate({2.5, 2.5});

test/manifold_test.cpp

@@ -14,6 +14,7 @@
 
 #include "manifold.h"
 
+#include "cross_section.h"
 #include "test.h"
 
 #ifdef MANIFOLD_EXPORT
@@ -505,3 +506,18 @@ TEST(Manifold, MirrorUnion) {
   EXPECT_FLOAT_EQ(vol_a * 2.75, result.GetProperties().volume);
   EXPECT_TRUE(a.Mirror(glm::vec3(0)).IsEmpty());
 }
+
+TEST(Manifold, Invalid) {
+  auto invalid = Manifold::Error::InvalidConstruction;
+  auto circ = CrossSection::Circle(10.);
+  auto empty_circ = CrossSection::Circle(-2.);
+  auto empty_sq = CrossSection::Square(glm::vec3(0.0f));
+
+  EXPECT_EQ(Manifold::Sphere(0).Status(), invalid);
+  EXPECT_EQ(Manifold::Cylinder(0, 5).Status(), invalid);
+  EXPECT_EQ(Manifold::Cylinder(2, -5).Status(), invalid);
+  EXPECT_EQ(Manifold::Cube(glm::vec3(0.0f)).Status(), invalid);
+  EXPECT_EQ(Manifold::Extrude(circ, 0.).Status(), invalid);
+  EXPECT_EQ(Manifold::Extrude(empty_circ, 10.).Status(), invalid);
+  EXPECT_EQ(Manifold::Revolve(empty_sq).Status(), invalid);
+}