Repository: elalish/manifold (language: python)

Issue:
Odd failure creating and unioning multiple cylinders
In the process of simplifying down my test code for the performance regression (https://github.com/starseeker/manifold/blob/perf_test/extras/offset_test.cpp), I've hit an odd failure that I'm having trouble reproducing in isolation.  I'm running fine, and then I suddenly hit this error (enabling MANIFOLD_DEBUG doesn't seem to change anything):

```
Edge 333: 13254 vertices, 27144 triangles
Edges - manifold boolean op failure: Vec out of range
```

I've tried to dump the glb meshes producing this output, but something is wrong:  I can generate them, but for the Manifold generated by Cylinder all I'm getting is a mesh that contains 1 vertex:

```
./extras/convertFile cyl.glb 
1 vertices, 28 triangles
0 objects:
```

However, if I interrogate the cyl Manifold itself in the C++ code with methods like NumVert() and NumTri(), both the Manifold itself and a GetMeshGL version, there appears to be more data there:

Short Edge Cylinder counts (Manifold, pre bool): 16 vertices, 28 triangles
Short Edge Cylinder counts (GetMeshGL, pre bool) Manifold: 16 vertices, 28 triangles
Vec out of range
Short Edge Cylinder counts (GetMeshGL, post bool): 16 vertices, 28 triangles

Compared to the other Cylinders being generated this one is short (a little over 0.025) but if I try to create an isolated test with just the Cylinder call and a glb read of the other input mesh the test passes.  If I skip that one short cylinder the rest of the test runs to completion.

Am I using Cylinder incorrectly, or somehow mismanaging memory in such a way that I could be causing corruption?  I'm at a bit of a loss, particularly since things seem to complete without that individual cylinder but trying to isolate it for a test case isn't going very well...


Discussion / hints:
Additional context - running on Linux, specific invocation of the test from the build directory after building my perf_test branch with MANIFOLD_EXPORT=ON is:

```
./extras/offsetTest ../test/models/Generic_Twin_91.1.t0.glb
```

@elalish I'll boil things down more for the boolean complex test form, but ideally I'd like to know what I'm doing wrong rather than just punting and skipping the problem case...
Weird, maybe try to compile and run with address sanitizer? Is something corrupted?
Yeah, ideally boil down that failure case to its own `TEST` as well. I agree that's quite strange. I'm guessing your 1-vert mesh is just due to the exporter collapsing verts? I wouldn't trust MeshIO/Assimp very much. I take it you've checked that your volumes are positive (CCW winding)? 
New data point - ran with valgrind (quite slow) and that didn't report anything. (Grr.)
Is there a way to specify one test to run with manifold_test?  
```
--gtest_filter=xxx
```

It actually supports `-h` for help for a list of gtest flags :)
Hmm - may have spotted a bug in my code.
Ah - no, just a raw calculation instead of calling a method.  Phooey.
I tried the Cylinder method in BRL-CAD in lieu of the custom code I was using and I occasionally get the Vec out of range error there as well.  Once the other test is in I'll have another go at boiling down a reproduction.
Drilling in with the debugger, I'm seeing the following:

```
(gdb) print evec
$9 = {x = -0, y = -0, z = 0.025390625}
(gdb) print manifold::RotateUp(evec)
$10 = {x = {x = -nan(0x8000000000000), y = -nan(0x8000000000000), z = -nan(0x8000000000000)}, y = {
    x = -nan(0x8000000000000), y = -nan(0x8000000000000), z = -nan(0x8000000000000)}, z = {x = -nan(0x8000000000000), 
    y = -nan(0x8000000000000), z = -nan(0x8000000000000)}, w = {x = 0, y = 0, z = 0}}
```

Looks like the problem is {0, 0, N} vectors?
Oh wow, all this comes down to a bad `RotateUp` implementation? Ugh, that's embarrassing. Oh well, I was planning to remove that anyway - linalg has something more general. Guess I should make a PR.

Fix patch:
diff --git a/include/manifold/manifold.h b/include/manifold/manifold.h
index ffe8bef26..04de80c79 100644
--- a/include/manifold/manifold.h
+++ b/include/manifold/manifold.h
@@ -323,4 +323,50 @@ class Manifold {
   CsgLeafNode& GetCsgLeafNode() const;
 };
 /** @} */
+
+/** @defgroup Debug
+ *  @brief Debugging features
+ *
+ * The features require compiler flags to be enabled. Assertions are enabled
+ * with the MANIFOLD_DEBUG flag and then controlled with ExecutionParams.
+ *  @{
+ */
+#ifdef MANIFOLD_DEBUG
+inline std::string ToString(const Manifold::Error& error) {
+  switch (error) {
+    case Manifold::Error::NoError:
+      return "No Error";
+    case Manifold::Error::NonFiniteVertex:
+      return "Non Finite Vertex";
+    case Manifold::Error::NotManifold:
+      return "Not Manifold";
+    case Manifold::Error::VertexOutOfBounds:
+      return "Vertex Out Of Bounds";
+    case Manifold::Error::PropertiesWrongLength:
+      return "Properties Wrong Length";
+    case Manifold::Error::MissingPositionProperties:
+      return "Missing Position Properties";
+    case Manifold::Error::MergeVectorsDifferentLengths:
+      return "Merge Vectors Different Lengths";
+    case Manifold::Error::MergeIndexOutOfBounds:
+      return "Merge Index Out Of Bounds";
+    case Manifold::Error::TransformWrongLength:
+      return "Transform Wrong Length";
+    case Manifold::Error::RunIndexWrongLength:
+      return "Run Index Wrong Length";
+    case Manifold::Error::FaceIDWrongLength:
+      return "Face ID Wrong Length";
+    case Manifold::Error::InvalidConstruction:
+      return "Invalid Construction";
+    default:
+      return "Unkown Error";
+  };
+}
+
+inline std::ostream& operator<<(std::ostream& stream,
+                                const Manifold::Error& error) {
+  return stream << ToString(error);
+}
+#endif
+/** @} */
 }  // namespace manifold
diff --git a/src/boolean_result.cpp b/src/boolean_result.cpp
index 4e68f1b24..5b10dc463 100644
--- a/src/boolean_result.cpp
+++ b/src/boolean_result.cpp
@@ -669,23 +669,23 @@ Manifold::Impl Boolean3::Result(OpType op) const {
   const int c2 = op == OpType::Add ? 1 : 0;
   const int c3 = op == OpType::Intersect ? 1 : -1;
 
+  if (inP_.status_ != Manifold::Error::NoError) {
+    auto impl = Manifold::Impl();
+    impl.status_ = inP_.status_;
+    return impl;
+  }
+  if (inQ_.status_ != Manifold::Error::NoError) {
+    auto impl = Manifold::Impl();
+    impl.status_ = inQ_.status_;
+    return impl;
+  }
+
   if (inP_.IsEmpty()) {
-    if (inP_.status_ != Manifold::Error::NoError ||
-        inQ_.status_ != Manifold::Error::NoError) {
-      auto impl = Manifold::Impl();
-      impl.status_ = Manifold::Error::InvalidConstruction;
-      return impl;
-    }
     if (!inQ_.IsEmpty() && op == OpType::Add) {
       return inQ_;
     }
     return Manifold::Impl();
   } else if (inQ_.IsEmpty()) {
-    if (inQ_.status_ != Manifold::Error::NoError) {
-      auto impl = Manifold::Impl();
-      impl.status_ = Manifold::Error::InvalidConstruction;
-      return impl;
-    }
     if (op == OpType::Intersect) {
       return Manifold::Impl();
     }
diff --git a/src/csg_tree.cpp b/src/csg_tree.cpp
index 3e6cbe777..bfeccbaab 100644
--- a/src/csg_tree.cpp
+++ b/src/csg_tree.cpp
@@ -185,7 +185,7 @@ Manifold::Impl CsgLeafNode::Compose(
   for (auto &node : nodes) {
     if (node->pImpl_->status_ != Manifold::Error::NoError) {
       Manifold::Impl impl;
-      impl.status_ = Manifold::Error::InvalidConstruction;
+      impl.status_ = node->pImpl_->status_;
       return impl;
     }
     double nodeOldScale = node->pImpl_->bBox_.Scale();
diff --git a/src/impl.cpp b/src/impl.cpp
index 12eb7bdb9..ace902456 100644
--- a/src/impl.cpp
+++ b/src/impl.cpp
@@ -513,6 +513,10 @@ Manifold::Impl Manifold::Impl::Transform(const mat3x4& transform_) const {
     result.status_ = status_;
     return result;
   }
+  if (!all(la::isfinite(transform_))) {
+    result.MarkFailure(Error::NonFiniteVertex);
+    return result;
+  }
   result.collider_ = collider_;
   result.meshRelation_ = meshRelation_;
   result.precision_ = precision_;
diff --git a/src/meshIO/meshIO.cpp b/src/meshIO/meshIO.cpp
index 090931416..3c74412ca 100644
--- a/src/meshIO/meshIO.cpp
+++ b/src/meshIO/meshIO.cpp
@@ -55,7 +55,7 @@ aiScene* CreateScene(const ExportOptions& options) {
 
   scene->mRootNode = new aiNode();
   scene->mRootNode->mNumMeshes = 1;
-  scene->mRootNode->mMeshes = new uint[scene->mRootNode->mNumMeshes];
+  scene->mRootNode->mMeshes = new uint32_t[scene->mRootNode->mNumMeshes];
   scene->mRootNode->mMeshes[0] = 0;
 
   scene->mMeshes[0]->mPrimitiveTypes = aiPrimitiveType_TRIANGLE;
@@ -251,7 +251,7 @@ void ExportMesh(const std::string& filename, const MeshGL& mesh,
   for (size_t i = 0; i < mesh_out->mNumFaces; ++i) {
     aiFace& face = mesh_out->mFaces[i];
     face.mNumIndices = 3;
-    face.mIndices = new uint[face.mNumIndices];
+    face.mIndices = new uint32_t[face.mNumIndices];
     for (int j : {0, 1, 2}) face.mIndices[j] = mesh.triVerts[3 * i + j];
   }
 
diff --git a/src/properties.cpp b/src/properties.cpp
index ab42c597b..f20cf0300 100644
--- a/src/properties.cpp
+++ b/src/properties.cpp
@@ -126,16 +126,12 @@ struct UpdateProperties {
 
 struct CheckHalfedges {
   VecView<const Halfedge> halfedges;
-  VecView<const vec3> vertPos;
 
   bool operator()(size_t edge) const {
     const Halfedge halfedge = halfedges[edge];
     if (halfedge.startVert == -1 || halfedge.endVert == -1) return true;
     if (halfedge.pairedHalfedge == -1) return false;
 
-    if (!std::isfinite(vertPos[halfedge.startVert][0])) return false;
-    if (!std::isfinite(vertPos[halfedge.endVert][0])) return false;
-
     const Halfedge paired = halfedges[halfedge.pairedHalfedge];
     bool good = true;
     good &= paired.pairedHalfedge == static_cast<int>(edge);
@@ -199,7 +195,7 @@ namespace manifold {
 bool Manifold::Impl::IsManifold() const {
   if (halfedge_.size() == 0) return true;
   return all_of(countAt(0_uz), countAt(halfedge_.size()),
-                CheckHalfedges({halfedge_, vertPos_}));
+                CheckHalfedges({halfedge_}));
 }
 
 /**
diff --git a/src/utils.h b/src/utils.h
index b7e43c955..f3e84a919 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -230,21 +230,6 @@ inline int CCW(vec2 p0, vec2 p1, vec2 p2, double tol) {
     return area > 0 ? 1 : -1;
 }
 
-/**
- * This 4x3 matrix can be used as an input to Manifold.Transform() to turn an
- * object. Turns along the shortest path from given up-vector to (0, 0, 1).
- *
- * @param up The vector to be turned to point upwards. Length does not matter.
- */
-inline mat3x4 RotateUp(vec3 up) {
-  up = la::normalize(up);
-  const vec3 axis = la::cross(up, {0, 0, 1});
-  double angle = la::asin(la::length(axis));
-  if (la::dot(up, {0, 0, 1}) < 0) angle = kPi - angle;
-  const quat q = la::rotation_quat(la::normalize(axis), angle);
-  return mat3x4(la::qmat(q), vec3());
-}
-
 inline mat4 Mat4(mat3x4 a) {
   return mat4({a[0], 0}, {a[1], 0}, {a[2], 0}, {a[3], 1});
 }


Test patch:
diff --git a/test/boolean_complex_test.cpp b/test/boolean_complex_test.cpp
index 61aea86f4..ae39e53bf 100644
--- a/test/boolean_complex_test.cpp
+++ b/test/boolean_complex_test.cpp
@@ -15,7 +15,6 @@
 #ifdef MANIFOLD_CROSS_SECTION
 #include "manifold/cross_section.h"
 #endif
-#include "../src/utils.h"  // For RotateUp
 #include "manifold/manifold.h"
 #include "manifold/polygon.h"
 #include "test.h"
@@ -1053,13 +1052,8 @@ TEST(BooleanComplex, SimpleOffset) {
     vec3 vpos(seeds.vertProperties[3 * i + 0], seeds.vertProperties[3 * i + 1],
               seeds.vertProperties[3 * i + 2]);
     Manifold vsph = sph.Translate(vpos);
-    if (!vsph.NumTri()) continue;
     c += vsph;
-    // See above discussion
-    EXPECT_EQ(c.Status(), Manifold::Error::NoError);
   }
-  // See above discussion
-  // EXPECT_EQ(c.Status(), Manifold::Error::NoError);
   // Edge Cylinders
   for (size_t i = 0; i < edges.size(); i++) {
     vec3 ev1 = vec3(seeds.vertProperties[3 * edges[i].first + 0],
@@ -1071,20 +1065,12 @@ TEST(BooleanComplex, SimpleOffset) {
     vec3 edge = ev2 - ev1;
     double len = la::length(edge);
     if (len < std::numeric_limits<float>::min()) continue;
-    // TODO - workaround, shouldn't be necessary
-    if (len < 0.03) continue;
     manifold::Manifold origin_cyl = manifold::Manifold::Cylinder(len, 1, 1, 8);
     vec3 evec(-1 * edge.x, -1 * edge.y, edge.z);
-    manifold::Manifold rotated_cyl =
-        origin_cyl.Transform(manifold::RotateUp(evec));
-    manifold::Manifold right = rotated_cyl.Translate(ev1);
-    if (!right.NumTri()) continue;
+    quat q = rotation_quat(normalize(evec), vec3(0, 0, 1));
+    manifold::Manifold right = origin_cyl.Transform({la::qmat(q), ev1});
     c += right;
-    // See above discussion
-    EXPECT_EQ(c.Status(), Manifold::Error::NoError);
   }
-  // See above discussion
-  // EXPECT_EQ(c.Status(), Manifold::Error::NoError);
   // Triangle Volumes
   for (size_t i = 0; i < seeds.NumTri(); i++) {
     int eind[3];
@@ -1098,6 +1084,7 @@ TEST(BooleanComplex, SimpleOffset) {
     vec3 a = ev[0] - ev[2];
     vec3 b = ev[1] - ev[2];
     vec3 n = la::normalize(la::cross(a, b));
+    if (!all(isfinite(n))) continue;
     // Extrude the points above and below the plane of the triangle
     vec3 pnts[6];
     for (int j = 0; j < 3; j++) pnts[j] = ev[j] + n;
@@ -1123,13 +1110,12 @@ TEST(BooleanComplex, SimpleOffset) {
     for (int j = 0; j < 24; j++)
       tri_m.triVerts.insert(tri_m.triVerts.end(), faces[j]);
     manifold::Manifold right(tri_m);
-    if (!right.NumTri()) continue;
     c += right;
     // See above discussion
     EXPECT_EQ(c.Status(), Manifold::Error::NoError);
   }
   // See above discussion
-  // EXPECT_EQ(c.Status(), Manifold::Error::NoError);
+  EXPECT_EQ(c.Status(), Manifold::Error::NoError);
 }
 
 #endif
diff --git a/test/samples_test.cpp b/test/samples_test.cpp
index dc1dd6548..4e9cede51 100644
--- a/test/samples_test.cpp
+++ b/test/samples_test.cpp
@@ -17,7 +17,6 @@
 #ifdef MANIFOLD_CROSS_SECTION
 #include "manifold/cross_section.h"
 #endif
-#include "../src/utils.h"
 #include "manifold/polygon.h"
 #include "test.h"
 
@@ -147,7 +146,8 @@ TEST(Samples, TetPuzzle) {
 
   Manifold puzzle2 = puzzle.Rotate(0, 0, 180);
   EXPECT_TRUE((puzzle ^ puzzle2).IsEmpty());
-  puzzle = puzzle.Transform(RotateUp({1, -1, -1}));
+  quat q = rotation_quat(normalize(vec3(1, -1, -1)), vec3(0, 0, 1));
+  puzzle = puzzle.Transform({la::qmat(q), vec3()});
 #ifdef MANIFOLD_EXPORT
   if (options.exportModels) ExportMesh("tetPuzzle.glb", puzzle.GetMeshGL(), {});
 #endif
diff --git a/test/smooth_test.cpp b/test/smooth_test.cpp
index 90e433d7d..083616eee 100644
--- a/test/smooth_test.cpp
+++ b/test/smooth_test.cpp
@@ -14,7 +14,6 @@
 
 #include <algorithm>
 
-#include "../src/utils.h"
 #ifdef MANIFOLD_CROSS_SECTION
 #include "manifold/cross_section.h"
 #endif
@@ -324,8 +323,7 @@ TEST(Smooth, Torus) {
         vec3 tan(v.y, -v.x, 0);
         tan *= la::dot(tan, edge) < 0 ? -1.0 : 1.0;
         tangent = CircularTangent(tan, edge);
-      } else if (std::abs(la::determinant(mat2(vec2(v), vec2(edge)))) <
-                 kTolerance) {
+      } else if (std::abs(la::determinant(mat2(vec2(v), vec2(edge)))) < 1e-5) {
         const double theta = std::asin(v.z);
         vec2 xy(v);
         const double r = la::length(xy);