Improve handling of invalid shape construction. (#385)

Add `InvalidConstruction` to `Manifold::Error` and return empty manifolds with it set when manifold construction is attempted with invalid parameters

- zero / negative dimensions for Cube
- zero / negative radius for Sphere
- zero / negative height for Extrude
- empty CrossSection for Extrude/Revolve

Also, Circle and Square have been updated to return empty CrossSections for similar arguments.

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 {

bindings/wasm/bindings.cpp

@@ -201,7 +201,8 @@ EMSCRIPTEN_BINDINGS(whatever) {
       .value("MergeIndexOutOfBounds", Manifold::Error::MergeIndexOutOfBounds)
       .value("TransformWrongLength", Manifold::Error::TransformWrongLength)
       .value("RunIndexWrongLength", Manifold::Error::RunIndexWrongLength)
-      .value("FaceIDWrongLength", Manifold::Error::FaceIDWrongLength);
+      .value("FaceIDWrongLength", Manifold::Error::FaceIDWrongLength)
+      .value("InvalidConstruction", Manifold::Error::InvalidConstruction);
 
   value_object<Box>("box").field("min", &Box::min).field("max", &Box::max);
 

bindings/wasm/bindings.js

@@ -230,6 +230,8 @@ Module.setup = function() {
         break;
       case Module.status.FaceIDWrongLength.value:
         message = 'Face ID vector has wrong length';
+      case Module.status.InvalidConstruction.value:
+        message = 'Manifold constructed with invalid parameters';
     }
 
     const base = Error.apply(this, [message, ...args]);

src/cross_section/src/cross_section.cpp

@@ -14,20 +14,6 @@
 
 #include "cross_section.h"
 
-#include <clipper2/clipper.h>
-
-#include <memory>
-#include <vector>
-
-#include "clipper2/clipper.core.h"
-#include "clipper2/clipper.engine.h"
-#include "clipper2/clipper.offset.h"
-#include "glm/ext/matrix_float3x2.hpp"
-#include "glm/ext/vector_float2.hpp"
-#include "glm/geometric.hpp"
-#include "glm/glm.hpp"
-#include "public.h"
-
 using namespace manifold;
 
 namespace {
@@ -230,23 +216,28 @@ C2::PathsD CrossSection::GetPaths() const {
 
 /**
  * Constructs a square with the given XY dimensions. By default it is
- * positioned in the first quadrant, touching the origin.
+ * positioned in the first quadrant, touching the origin. If any dimensions in
+ * size are negative, or if all are zero, an empty Manifold will be returned.
  *
  * @param size The X, and Y dimensions of the square.
  * @param center Set to true to shift the center to the origin.
  */
-CrossSection CrossSection::Square(const glm::vec2 dims, bool center) {
+CrossSection CrossSection::Square(const glm::vec2 size, bool center) {
+  if (size.x < 0.0f || size.y < 0.0f || glm::length(size) == 0.0f) {
+    return CrossSection();
+  }
+
   auto p = C2::PathD(4);
   if (center) {
-    auto w = dims.x / 2;
-    auto h = dims.y / 2;
+    const auto w = size.x / 2;
+    const auto h = size.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);
   } else {
-    double x = dims.x;
-    double y = dims.y;
+    const double x = size.x;
+    const double y = size.y;
     p[0] = C2::PointD(0.0, 0.0);
     p[1] = C2::PointD(x, 0.0);
     p[2] = C2::PointD(x, y);
@@ -263,6 +254,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

@@ -143,12 +143,17 @@ Manifold Manifold::Tetrahedron() {
 
 /**
  * Constructs a unit cube (edge lengths all one), by default in the first
- * octant, touching the origin.
+ * octant, touching the origin. If any dimensions in size are negative, or if
+ * all are zero, an empty Manifold will be returned.
  *
  * @param size The X, Y, and Z dimensions of the box.
  * @param center Set to true to shift the center to the origin.
  */
 Manifold Manifold::Cube(glm::vec3 size, bool center) {
+  if (size.x < 0.0f || size.y < 0.0f || size.z < 0.0f ||
+      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 +175,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 +203,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 +237,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 +321,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,13 @@
 
 using namespace manifold;
 
+TEST(CrossSection, Square) {
+  auto a = Manifold::Cube({5, 5, 5});
+  auto b = Manifold::Extrude(CrossSection::Square({5, 5}), 5);
+
+  EXPECT_FLOAT_EQ((a - b).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,19 @@ 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::Cube({-1, 1, 1}).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);
+}