More PolySet Refactoring (openscad#5029)

* Manifold constness fixes
* Added utility: PolySet::createEmpty()
* Renamed private members to have underscore suffix

src/RenderStatistic.cc

@@ -354,7 +354,7 @@ void StreamVisitor::visit(const PolySet& ps)
     assert(ps.getDimension() == 3);
     nlohmann::json geometryJson;
     geometryJson["dimensions"] = 3;
-    geometryJson["convex"] = ps.is_convex();
+    geometryJson["convex"] = ps.isConvex();
     geometryJson["facets"] = ps.numFacets();
     if (is_enabled(RenderStatistic::BOUNDING_BOX)) {
       geometryJson["bounding_box"] = getBoundingBox3(ps);

src/core/ImportNode.cc

@@ -211,7 +211,7 @@ std::unique_ptr<const Geometry> ImportNode::createGeometry() const
 #endif
   default:
     LOG(message_group::Error, "Unsupported file format while trying to import file '%1$s', import() at line %2$d", this->filename, loc.firstLine());
-    g = std::make_unique<PolySet>(3);
+    g = PolySet::createEmpty();
   }
 
   g->setConvexity(this->convexity);

src/core/primitives.cc

@@ -111,7 +111,7 @@ std::unique_ptr<const Geometry> CubeNode::createGeometry() const
     || this->y <= 0 || !std::isfinite(this->y)
     || this->z <= 0 || !std::isfinite(this->z)
     ) {
-    return std::make_unique<PolySet>(3, true);
+    return PolySet::createEmpty();
   }
 
   double x1, x2, y1, y2, z1, z2;
@@ -184,7 +184,7 @@ static std::shared_ptr<AbstractNode> builtin_cube(const ModuleInstantiation *ins
 std::unique_ptr<const Geometry> SphereNode::createGeometry() const
 {
   if (this->r <= 0 || !std::isfinite(this->r)) {
-    return std::make_unique<PolySet>(3, true);
+    return PolySet::createEmpty();
   }
 
   auto num_fragments = Calc::get_fragments_from_r(r, fn, fs, fa);
@@ -263,7 +263,7 @@ std::unique_ptr<const Geometry> CylinderNode::createGeometry() const
     || this->r2 < 0 || !std::isfinite(this->r2)
     || (this->r1 <= 0 && this->r2 <= 0)
     ) {
-    return std::make_unique<PolySet>(3, true);
+    return PolySet::createEmpty();
   }
 
   auto num_fragments = Calc::get_fragments_from_r(std::fmax(this->r1, this->r2), this->fn, this->fs, this->fa);
@@ -277,7 +277,7 @@ std::unique_ptr<const Geometry> CylinderNode::createGeometry() const
     z2 = this->h;
   }
 
-  auto polyset = std::make_unique<PolySet>(3, true);
+  auto polyset = std::make_unique<PolySet>(3, /*convex*/true);
   polyset->vertices.reserve(2 * num_fragments);
 
   generate_circle(std::back_inserter(polyset->vertices), r1, z1, num_fragments);
@@ -404,12 +404,17 @@ std::string PolyhedronNode::toString() const
 
 std::unique_ptr<const Geometry> PolyhedronNode::createGeometry() const
 {
-  auto p = std::make_unique<PolySet>(3);
+  auto p = PolySet::createEmpty();
   p->setConvexity(this->convexity);
   p->vertices=this->points;
   p->indices=this->faces;
-  for (auto &poly : p->indices)
+  p->isTriangular = true;
+  for (auto &poly : p->indices) {
     std::reverse(poly.begin(),poly.end());
+    if (p->isTriangular && poly.size() > 3) {
+      p->isTriangular = false;
+    }
+  }
   return p;
 }
 
@@ -474,6 +479,7 @@ static std::shared_ptr<AbstractNode> builtin_polyhedron(const ModuleInstantiatio
         }
         pointIndexIndex++;
       }
+      // FIXME: Print an error message if < 3 vertices are specified
       if (face.size() >= 3) {
         node->faces.push_back(std::move(face));
       }

src/geometry/GeometryEvaluator.cc

@@ -256,7 +256,7 @@ std::unique_ptr<Geometry> GeometryEvaluator::applyHull3D(const AbstractNode& nod
 {
   Geometry::Geometries children = collectChildren3D(node);
 
-  auto P = std::make_unique<PolySet>(3);
+  auto P = PolySet::createEmpty();
   return applyHull(children);
 }
 
@@ -704,18 +704,18 @@ Response GeometryEvaluator::visit(State& state, const TransformNode& node)
             std::shared_ptr<Polygon2d> newpoly;
             if (res.isConst()) {
               newpoly = std::make_shared<Polygon2d>(*polygons);
-	    }
+            }
             else {
               newpoly = std::dynamic_pointer_cast<Polygon2d>(res.ptr());
-	    }
+            }
 
             Transform2d mat2;
             mat2.matrix() <<
               node.matrix(0, 0), node.matrix(0, 1), node.matrix(0, 3),
               node.matrix(1, 0), node.matrix(1, 1), node.matrix(1, 3),
               node.matrix(3, 0), node.matrix(3, 1), node.matrix(3, 3);
             newpoly->transform(mat2);
-	    // FIXME: We lose the transform if we copied a const geometry above. Probably similar issue in multiple places
+            // FIXME: We lose the transform if we copied a const geometry above. Probably similar issue in multiple places
             // A 2D transformation may flip the winding order of a polygon.
             // If that happens with a sanitized polygon, we need to reverse
             // the winding order for it to be correct.
@@ -842,29 +842,29 @@ static void add_slice(PolySetBuilder &builder, const Polygon2d& poly,
       // unless at top for a 0-scaled axis (which can create 0 thickness "ears")
       if (splitfirst xor any_zero) {
         builder.appendPoly({
-		Vector3d(curr1[0], curr1[1], h1),
-		Vector3d(curr2[0], curr2[1], h2),
-		Vector3d(prev1[0], prev1[1], h1)
-		});
+                Vector3d(curr1[0], curr1[1], h1),
+                Vector3d(curr2[0], curr2[1], h2),
+                Vector3d(prev1[0], prev1[1], h1)
+                });
         if (!any_zero || (any_non_zero && prev2 != curr2)) {
           builder.appendPoly({
-		Vector3d(prev2[0], prev2[1], h2),
-		Vector3d(prev1[0], prev1[1], h1),
-		Vector3d(curr2[0], curr2[1], h2)
-	  });
+                Vector3d(prev2[0], prev2[1], h2),
+                Vector3d(prev1[0], prev1[1], h1),
+                Vector3d(curr2[0], curr2[1], h2)
+          });
         }
       } else {
         builder.appendPoly({
-		Vector3d(curr1[0], curr1[1], h1),
-		Vector3d(prev2[0], prev2[1], h2),
-		Vector3d(prev1[0], prev1[1], h1)
-	});
+                Vector3d(curr1[0], curr1[1], h1),
+                Vector3d(prev2[0], prev2[1], h2),
+                Vector3d(prev1[0], prev1[1], h1)
+        });
         if (!any_zero || (any_non_zero && prev2 != curr2)) {
           builder.appendPoly({
-		Vector3d(curr1[0], curr1[1], h1),
-		Vector3d(curr2[0], curr2[1], h2),
-		Vector3d(prev2[0], prev2[1], h2)
-	  });	
+                Vector3d(curr1[0], curr1[1], h1),
+                Vector3d(curr2[0], curr2[1], h2),
+                Vector3d(prev2[0], prev2[1], h2)
+          });        
         }
       }
       prev1 = curr1;
@@ -1041,7 +1041,7 @@ static std::unique_ptr<Geometry> extrudePolygon(const LinearExtrudeNode& node, c
   if (isConvex && non_linear) isConvex = unknown;
   PolySetBuilder builder(0, 0, 3, isConvex);
   builder.setConvexity(node.convexity);
-  if (node.height <= 0) return std::make_unique<PolySet>(3);
+  if (node.height <= 0) return PolySet::createEmpty();
 
   size_t slices;
   if (node.has_slices) {
@@ -1312,16 +1312,16 @@ static std::unique_ptr<Geometry> rotatePolygon(const RotateExtrudeNode& node, co
 
       for (size_t i = 0; i < o.vertices.size(); ++i) {
         builder.appendPoly({
-		rings[j % 2][(i + 1) % o.vertices.size()],
-		rings[(j + 1) % 2][(i + 1) % o.vertices.size()],
-		rings[j % 2][i]
-	});		
+                rings[j % 2][(i + 1) % o.vertices.size()],
+                rings[(j + 1) % 2][(i + 1) % o.vertices.size()],
+                rings[j % 2][i]
+        });                
 
         builder.appendPoly({
-		rings[(j + 1) % 2][(i + 1) % o.vertices.size()],
-		rings[(j + 1) % 2][i],
-		rings[j % 2][i]
-	});
+                rings[(j + 1) % 2][(i + 1) % o.vertices.size()],
+                rings[(j + 1) % 2][i],
+                rings[j % 2][i]
+        });
       }
     }
   }
@@ -1351,7 +1351,7 @@ Response GeometryEvaluator::visit(State& state, const RotateExtrudeNode& node)
         geometry = applyToChildren2D(node, OpenSCADOperator::UNION);
       }
       if (geometry) {
-	geom = rotatePolygon(node, *geometry);
+        geom = rotatePolygon(node, *geometry);
       }
     } else {
       geom = smartCacheGet(node, false);
@@ -1404,8 +1404,8 @@ std::shared_ptr<const Geometry> GeometryEvaluator::projectionNoCut(const Project
     // project chgeom -> polygon2d
     if (auto chPS = PolySetUtils::getGeometryAsPolySet(chgeom)) {
       if (auto poly = PolySetUtils::project(*chPS)) {
-	bounds.extend(poly->getBoundingBox());
-	tmp_geom.push_back(std::move(poly));
+        bounds.extend(poly->getBoundingBox());
+        tmp_geom.push_back(std::move(poly));
       }
     }
   }
@@ -1478,7 +1478,7 @@ Response GeometryEvaluator::visit(State& state, const CgalAdvNode& node)
         geom = res.constptr();
         // If we added convexity, we need to pass it on
         if (geom && geom->getConvexity() != node.convexity) {
-	  std::shared_ptr<Geometry> editablegeom;
+          std::shared_ptr<Geometry> editablegeom;
           // If we got a const object, make a copy
           if (res.isConst()) editablegeom = geom->copy();
           else editablegeom = res.ptr();
@@ -1561,13 +1561,13 @@ Response GeometryEvaluator::visit(State& state, const RoofNode& node)
     if (!isSmartCached(node)) {
       const auto polygon2d = applyToChildren2D(node, OpenSCADOperator::UNION);
       if (polygon2d) {
-	std::unique_ptr<Geometry> roof;
+        std::unique_ptr<Geometry> roof;
         try {
           roof = roofOverPolygon(node, *polygon2d);
         } catch (RoofNode::roof_exception& e) {
           LOG(message_group::Error, node.modinst->location(), this->tree.getDocumentPath(),
               "Skeleton computation error. " + e.message());
-          roof = std::make_unique<PolySet>(3);
+          roof = PolySet::createEmpty();
         }
         assert(roof);
         geom = std::move(roof);

src/geometry/PolySet.cc

@@ -46,7 +46,7 @@
 
  */
 
-PolySet::PolySet(unsigned int dim, boost::tribool convex) : dim(dim), convex(convex)
+PolySet::PolySet(unsigned int dim, boost::tribool convex) : dim_(dim), convex_(convex)
 {
 }
 
@@ -58,7 +58,7 @@ std::string PolySet::dump() const
 {
   std::ostringstream out;
   out << "PolySet:"
-      << "\n dimensions:" << this->dim
+      << "\n dimensions:" << dim_
       << "\n convexity:" << this->convexity
       << "\n num polygons: " << indices.size()
       << "\n polygons data:";
@@ -74,12 +74,12 @@ std::string PolySet::dump() const
 
 BoundingBox PolySet::getBoundingBox() const
 {
-  if (this->bbox.isNull()) {
+  if (bbox_.isNull()) {
     for (const auto& v : vertices) {
-      this->bbox.extend(v);
+      bbox_.extend(v);
     }
   }
-  return this->bbox;
+  return bbox_;
 }
 
 size_t PolySet::memsize() const
@@ -102,12 +102,12 @@ void PolySet::transform(const Transform3d& mat)
     for (auto& p : this->indices) {
       std::reverse(p.begin(), p.end());
   }
-  this->bbox.setNull();
+  bbox_.setNull();
 }
 
-bool PolySet::is_convex() const {
-  if (convex || this->isEmpty()) return true;
-  if (!convex) return false;
+bool PolySet::isConvex() const {
+  if (convex_ || this->isEmpty()) return true;
+  if (!convex_) return false;
   return PolySetUtils::is_approximately_convex(*this);
 }
 

src/geometry/PolySet.h

@@ -23,7 +23,7 @@ class PolySet : public Geometry
   size_t memsize() const override;
   BoundingBox getBoundingBox() const override;
   std::string dump() const override;
-  unsigned int getDimension() const override { return this->dim; }
+  unsigned int getDimension() const override { return dim_; }
   bool isEmpty() const override { return indices.empty(); }
   std::unique_ptr<Geometry> copy() const override;
 
@@ -32,12 +32,14 @@ class PolySet : public Geometry
   void transform(const Transform3d& mat) override;
   void resize(const Vector3d& newsize, const Eigen::Matrix<bool, 3, 1>& autosize) override;
 
-  bool is_convex() const;
-  boost::tribool convexValue() const { return this->convex; }
+  bool isConvex() const;
+  boost::tribool convexValue() const { return convex_; }
   bool isTriangular = false;
 
+  static std::unique_ptr<PolySet> createEmpty() { return std::make_unique<PolySet>(3); }
+
 private:
-  unsigned int dim;
-  mutable boost::tribool convex;
-  mutable BoundingBox bbox;
+  unsigned int dim_;
+  mutable boost::tribool convex_;
+  mutable BoundingBox bbox_;
 };

src/geometry/PolySetUtils.cc

@@ -164,7 +164,7 @@ std::shared_ptr<const PolySet> getGeometryAsPolySet(const std::shared_ptr<const
       }
       LOG(message_group::Error, "Nef->PolySet failed.");
     }
-    return std::make_unique<PolySet>(3);
+    return PolySet::createEmpty();
   }
   if (auto hybrid = std::dynamic_pointer_cast<const CGALHybridPolyhedron>(geom)) {
     return hybrid->toPolySet();

src/geometry/boolean_utils.cc

@@ -137,7 +137,7 @@ std::shared_ptr<const Geometry> applyMinkowski(const Geometry::Geometries& child
         else if (nef && nef->p3->is_simple()) CGALUtils::convertNefToPolyhedron(*nef->p3, poly);
         else throw 0;
 
-        if ((ps && ps->is_convex()) ||
+        if ((ps && ps->isConvex()) ||
             (!ps && CGALUtils::is_weakly_convex(poly))) {
           PRINTDB("Minkowski: child %d is convex and %s", i % (ps?"PolySet":"Nef"));
           P[i].push_back(poly);

src/geometry/cgal/cgalutils-applyops-hybrid-minkowski.cc

@@ -66,7 +66,7 @@ std::shared_ptr<const Geometry> applyMinkowskiHybrid(const Geometry::Geometries&
           else throw 0;
         } else throw 0;
 
-        if ((ps && ps->is_convex()) ||
+        if ((ps && ps->isConvex()) ||
             (!ps && CGALUtils::is_weakly_convex(*poly))) {
           PRINTDB("Minkowski: child %d is convex and %s", i % (ps?"PolySet":"Hybrid"));
           P[i].push_back(poly);

src/geometry/cgal/cgalutils-hybrid.cc

@@ -49,7 +49,7 @@ std::shared_ptr<CGALHybridPolyhedron> createHybridPolyhedronFromPolySet(const Po
   std::vector<Vector3d> points3d;
   psq.quantizeVertices(&points3d);
   auto ps_tri = PolySetUtils::tessellate_faces(psq);
-  if (ps_tri->is_convex()) {
+  if (ps_tri->isConvex()) {
     using K = CGAL::Epick;
     // Collect point cloud
     std::vector<K::Point_3> points(points3d.size());
@@ -70,7 +70,7 @@ std::shared_ptr<CGALHybridPolyhedron> createHybridPolyhedronFromPolySet(const Po
   if (createMeshFromPolySet(*ps_tri, *mesh)) {
     assert(false && "Error from createMeshFromPolySet");
   }
-  if (!ps_tri->is_convex()) {
+  if (!ps_tri->isConvex()) {
     if (isClosed(*mesh)) {
       // Note: PMP::orient can corrupt models and cause cataclysmic memory leaks
       // (try testdata/scad/3D/issues/issue1105d.scad for instance), but

src/geometry/cgal/cgalutils.cc

@@ -46,7 +46,7 @@ std::unique_ptr<CGAL_Nef_polyhedron> createNefPolyhedronFromPolySet(const PolySe
   std::vector<Vector3d> points3d;
   psq.quantizeVertices(&points3d);
   auto ps_tri = PolySetUtils::tessellate_faces(psq);
-  if (ps_tri->is_convex()) {
+  if (ps_tri->isConvex()) {
     using K = CGAL::Epick;
     // Collect point cloud
     std::vector<K::Point_3> points(points3d.size());
@@ -375,7 +375,7 @@ std::unique_ptr<PolySet> createPolySetFromNefPolyhedron3(const CGAL::Nef_polyhed
     LOG(message_group::Error, "Non-manifold mesh created: %1$d unconnected edges", unconnected2);
   }
 
-  auto polyset = std::make_unique<PolySet>(3);
+  auto polyset = PolySet::createEmpty();
   polyset->vertices.reserve(verts.size());
   for (const auto& v : verts) {
     polyset->vertices.emplace_back(v.cast<double>());