Allow bridges to wrap around corners.

Makefile.am

@@ -77,7 +77,7 @@ EXTRA_DIST = millproject
 
 check_PROGRAMS = voronoi_tests eulerian_paths_tests segmentize_tests tsp_solver_tests units_tests \
                  available_drills_tests gerberimporter_tests options_tests path_finding_tests \
-                 autoleveller_tests common_tests backtrack_tests trim_paths_tests
+                 autoleveller_tests common_tests backtrack_tests trim_paths_tests outline_bridges_tests
 voronoi_tests_SOURCES = voronoi.hpp voronoi.cpp voronoi_tests.cpp boost_unit_test.cpp
 eulerian_paths_tests_SOURCES = eulerian_paths_tests.cpp eulerian_paths.hpp geometry_int.hpp boost_unit_test.cpp  bg_operators.hpp bg_operators.cpp bg_helpers.hpp bg_helpers.cpp eulerian_paths.cpp segmentize.cpp merge_near_points.cpp
 segmentize_tests_SOURCES = segmentize_tests.cpp segmentize.cpp segmentize.hpp merge_near_points.cpp merge_near_points.hpp boost_unit_test.cpp bg_helpers.hpp bg_helpers.cpp eulerian_paths.cpp bg_operators.hpp bg_operators.cpp
@@ -92,6 +92,7 @@ autoleveller_tests_SOURCES = autoleveller_tests.cpp autoleveller.hpp autolevelle
 common_tests_SOURCES = common.hpp common.cpp common_tests.cpp boost_unit_test.cpp
 backtrack_tests_SOURCES = backtrack.hpp backtrack.cpp backtrack_tests.cpp boost_unit_test.cpp bg_helpers.hpp bg_helpers.cpp eulerian_paths.hpp eulerian_paths.cpp segmentize.hpp segmentize.cpp merge_near_points.hpp merge_near_points.cpp bg_operators.hpp bg_operators.cpp
 trim_paths_tests_SOURCES = trim_paths.hpp trim_paths.cpp trim_paths_tests.cpp boost_unit_test.cpp bg_helpers.hpp bg_helpers.cpp eulerian_paths.hpp eulerian_paths.cpp segmentize.hpp segmentize.cpp merge_near_points.hpp merge_near_points.cpp bg_operators.hpp bg_operators.cpp
+outline_bridges_tests_SOURCES = outline_bridges_tests.cpp outline_bridges.hpp outline_bridges.cpp bg_operators.hpp bg_operators.cpp bg_helpers.hpp bg_helpers.cpp eulerian_paths.hpp eulerian_paths.cpp segmentize.hpp segmentize.cpp boost_unit_test.cpp merge_near_points.hpp merge_near_points.cpp
 
 TESTS = $(check_PROGRAMS)
 

ngc_exporter.cpp

@@ -153,32 +153,29 @@ void NGC_Exporter::cutter_milling(std::ofstream& of, shared_ptr<Cutter> cutter,
 
     auto current_bridge = bridges.cbegin();
 
+    bool in_bridge = false;
+    // Start at 1 because the caller already moved to the start.
     for (size_t current = 1; current < path.size(); current++) {
-      if (current_bridge != bridges.cend() && *current_bridge == current && z >= cutter->bridges_height) {
-        // We're about to cut to the start of a bridge but there is no need to
-        // stop there so let's just mill right across it.
-        current += 2;
+      while (current_bridge != bridges.cend() && *current_bridge < current - 1) {
         current_bridge++;
       }
       // We are now cutting to current.
       // Is this a bridge cut?
       auto is_bridge_cut = current_bridge != bridges.cend() && *current_bridge == current-1;
-      if (is_bridge_cut) {
-        // We're about to make a bridge cut so we need to go up.  (If we didn't
-        // need to, we would have skipped over it earlier.)
+      if (is_bridge_cut && z < cutter->bridges_height && !in_bridge) {
+        // We're about to make a bridge cut so we need to go up.
         of << "G00 Z" << cutter->bridges_height * cfactor << '\n';
+        in_bridge = true;
+      } else if (!is_bridge_cut && in_bridge) {
+        // Now plunge back down if needed.
+        of << "G01 Z" << z * cfactor << " F" << cutter->vertfeed * cfactor << '\n';
+        of << "G01 F" << cutter->feed * cfactor << '\n';
+        in_bridge = false;
       }
-
+      
       // Now cut horizontally.
       of << "G01 X" << (path.at(current).x() - xoffsetTot) * cfactor
          << " Y"    << (path.at(current).y() - yoffsetTot) * cfactor << '\n';
-
-      // Now plunge back down if needed.
-      if (is_bridge_cut) {
-        of << "G01 Z" << z * cfactor << " F" << cutter->vertfeed * cfactor << '\n';
-        of << "G01 F" << cutter->feed * cfactor << '\n';
-        current_bridge++; // We're done with this bridge.
-      }
     }
   }
 }

outline_bridges.cpp

@@ -19,62 +19,141 @@
 
 #include "outline_bridges.hpp"
 
-#include <boost/assign/list_of.hpp>
-#include <boost/geometry/algorithms/distance.hpp>
-#include <cmath>
-#include <queue>
-#include <set>
+#include "bg_operators.hpp"
 #include <map>
+#include <list>
+#include <set>
 
 using std::vector;
 using std::pair;
 using std::set;
 using std::map;
+using std::list;
+
+namespace boost { namespace geometry { namespace model { namespace d2 {
+
+static inline bool operator<(const list<point_type_fp>::const_iterator& lhs,
+                             const list<point_type_fp>::const_iterator& rhs) {
+  return *lhs < *rhs;
+}
+}}}}
+
+namespace outline_bridges {
 
 //This function returns the intermediate point between p0 and p1.
-//With position=0 it returns p0, with position=1 it returns p1,
-//with values between 0 and 1 it returns the relative position between p0 and p1
-point_type_fp intermediatePoint(point_type_fp p0, point_type_fp p1, double position) {
+//With position=0 it returns p0, with position=1 it returns p1, with
+//values between 0 and 1 it returns the relative position between p0 and p1
+static point_type_fp intermediatePoint(point_type_fp p0, point_type_fp p1, double position) {
   return point_type_fp(p0.x() + (p1.x() - p0.x()) * position,
                        p0.y() + (p1.y() - p0.y()) * position);
 }
 
+/* Find where to insert a point that is offset distance away from the
+ * point at path[index].  Return the index of the newly inserted
+ * point. */
+static list<point_type_fp>::const_iterator insertPoint(list<point_type_fp>& path, list<point_type_fp>::const_iterator p, double offset) {
+  if (offset == 0) {
+    return path.insert(p, *p);
+  }
+  if (offset < 0) {
+    // Backwards.
+    if (p != path.cbegin()) {
+      auto d = bg::distance(*p, *(std::prev(p)));
+      if (offset < -d) {
+        // Need to go back beyond the point.
+        return insertPoint(path, std::prev(p), offset + d);
+      } else {
+        auto new_point = intermediatePoint(*p, *(std::prev(p)), -offset/d);
+        return path.insert(p, new_point);
+      }
+    } else {
+      // index is 0.
+      if (*p == path.back()) {
+        return insertPoint(path, std::prev(path.cend()), offset);
+      } else {
+        // Not a ring so just insert at the start. This might make a
+        // bridge that is too short in the rare case where the outline
+        // isn't a loop.
+        return path.insert(path.begin(), *p);
+      }
+    }
+  } else {
+    // Forward.
+    if (p != std::prev(path.cend())) {
+      auto d = bg::distance(*p, *(std::next(p)));
+      if (offset > d) {
+        // Need to go forward beyond the point.
+        return insertPoint(path, std::next(p), offset - d);
+      } else {
+        auto new_point = intermediatePoint(*p, *std::next(p), offset/d);
+        return path.insert(std::next(p), new_point);
+      }
+    } else {
+      // index is at the end.
+      if (*p == path.front()) {
+        return insertPoint(path, path.cbegin(), offset);
+      } else {
+        // Not enough room so just insert at the end.
+        return path.insert(path.cend(), *p);
+      }
+    }
+  }
+}
+
 /* This function takes the segments where the bridges must be built (in the form
  * of set<size_t>, see findBridgeSegments), inserts them in the path and
  * returns an array containing the indexes of each bridge's start.
  */
-vector<size_t> insertBridges(linestring_type_fp& path, const set<size_t>& bridges, double length) {
-  vector<size_t> chosenSegments(bridges.cbegin(), bridges.cend());
-  path.reserve(path.size() + chosenSegments.size() * 2);  //Just to avoid unnecessary reallocations
-  std::sort(chosenSegments.begin(), chosenSegments.end()); //Sort it (lower index -> higher index)
+static vector<size_t> insertBridges(linestring_type_fp& path, const set<size_t>& bridges, double length) {
+  list<point_type_fp> path_list(path.cbegin(), path.cend());
+  set<list<point_type_fp>::const_iterator> bridge_pointers;
+  {
+    auto p = path_list.cbegin();
+    size_t i = 0;
+    for (; p != path_list.cend(); i++, p++) {
+      if (bridges.count(i) > 0) {
+        bridge_pointers.insert(p);
+      }
+    }
+  }
 
-  vector<size_t> output;
-  for(size_t i = 0; i < chosenSegments.size(); i++) {
-    //Each time we insert a bridge all following indexes have a offset of 2, we
-    auto segment_index = chosenSegments[i] + 2 * i;
-    auto segment_start = path.at(segment_index);
-    auto segment_end = path.at(segment_index + 1);
+  set<list<point_type_fp>::const_iterator> bridge_segments;
+  for(const auto& bridge_pointer : bridge_pointers) {
+    auto segment_start = *bridge_pointer;
+    auto segment_end = *std::next(bridge_pointer);
     auto segment_length = bg::distance(segment_start, segment_end);
-    //Insert the bridges in the path
-    path.insert(path.begin() + segment_index + 1,
-                { intermediatePoint(segment_start,
-                                    segment_end,
-                                    0.5 - (length / segment_length) / 2),
-                  intermediatePoint(segment_start,
-                                    segment_end,
-                                    0.5 + (length / segment_length) / 2) });
-    output.push_back(segment_index + 1);    //Add the bridges indexes to output
+    //Insert the bridges in the path.
+    auto bridge_start = insertPoint(path_list, bridge_pointer, segment_length/2 - length/2);
+    auto bridge_end = insertPoint(path_list, bridge_pointer, segment_length/2 + length/2);
+    for (auto seg = bridge_start; seg != bridge_end; seg++) {
+      if (seg == path_list.cend()) {
+        seg = path_list.cbegin();
+      }
+      bridge_segments.insert(seg);
+    }
   }
 
+  linestring_type_fp new_path;
+  vector<size_t> output;
+  {
+    int i = 0;
+    for (auto p = path_list.cbegin(); p != path_list.cend(); p++, i++) {
+      new_path.push_back(*p);
+      if (bridge_segments.count(p) > 0) {
+        output.push_back(i);
+      }
+    }
+  }
+  path = new_path;
   return output;
 }
 
 // Computes the distance between the two closest points in the clique.  The
 // locations of the elements of the clique are in the locations input.  The two
 // points that are closest together are set in closest.
-static inline double min_clique_distance(const set<size_t>& clique,
-                                         const map<size_t, point_type_fp>& locations,
-                                         vector<size_t>& closest) {
+static double min_clique_distance(const set<size_t>& clique,
+                                  const map<size_t, point_type_fp>& locations,
+                                  vector<size_t>& closest) {
   auto current_score = std::numeric_limits<double>::infinity();
   for (auto i = clique.cbegin(); i != clique.cend(); i++) {
     for (auto j = std::next(i); j != clique.cend(); j++) {
@@ -89,8 +168,9 @@ static inline double min_clique_distance(const set<size_t>& clique,
   return current_score;
 }
 
-// Computes the minimum of the distances from the given points to all the other
-// points in the clique, not including the same point because it would be distance 0.
+// Computes the minimum of the distances from the given point to all
+// the other points in the clique, not including the same point
+// because it would be distance 0.
 static inline vector<double> min_distance_to_clique(point_type_fp point,
                                                     const vector<size_t>& excluded_points,
                                                     const set<size_t>& clique,
@@ -113,27 +193,23 @@ static inline vector<double> min_distance_to_clique(point_type_fp point,
 
 /* This function finds the segments for putting bridges.
  *
- * It starts by finding the longest segments.  Then it iteratively picks one
- * segment at a time to try to replace elsewhere, searching for a swap that will
- * maximize the minimum distance between segments.  It continues until no
- * improvement can be made or until one second has passed.  It may return fewer
- * than the "number" requested if not enough place can be found to place
- * bridges.
+ * It starts by finding the longest segments.  Then it iteratively
+ * picks one segment at a time to try to replace elsewhere, searching
+ * for a swap that will maximize the minimum distance between
+ * segments.  It continues until no improvement can be made.  It may
+ * return fewer than the "number" requested if not enough place can be
+ * found to place bridges.
  *
  * Returns the positions in path of the segments that need to be modified.
  */
-set<size_t> findBridgeSegments(const linestring_type_fp& path, size_t number, double length) {
+static set<size_t> findBridgeSegments(const linestring_type_fp& path, size_t number) {
   if (number < 1) {
     return {};
   }
 
   // All the potential bridge segments and their locations.
   map<size_t, point_type_fp> candidates;
   for (size_t i = 0; i < path.size() - 1; i++) {
-    auto current_distance = bg::distance(path.at(i), path.at(i+1));
-    if (current_distance < length) {
-      continue;
-    }
     candidates[i] = intermediatePoint(path.at(i),
                                       path.at(i + 1),
                                       0.5);
@@ -181,11 +257,12 @@ set<size_t> findBridgeSegments(const linestring_type_fp& path, size_t number, do
     }
     output.erase(swap_from);
     output.insert(swap_to);
-    best_score = min_clique_distance(output, candidates, closest);
   } while(true);
   return output;
 }
 
-vector<size_t> outline_bridges::makeBridges(linestring_type_fp& path, size_t number, double length) {
-  return insertBridges(path, findBridgeSegments(path, number, length), length);
+vector<size_t> makeBridges(linestring_type_fp& path, size_t number, double length) {
+  return insertBridges(path, findBridgeSegments(path, number), length);
 }
+
+} // namespace outline_bridges

outline_bridges_tests.cpp

@@ -0,0 +1,60 @@
+#define BOOST_TEST_MODULE outline_bridges tests
+#include <boost/test/unit_test.hpp>
+
+#include <vector>
+
+#include "geometry.hpp"
+#include "bg_operators.hpp"
+
+#include "outline_bridges.hpp"
+
+using namespace std;
+using outline_bridges::makeBridges;
+
+BOOST_AUTO_TEST_SUITE(outline_bridges_tests)
+
+BOOST_AUTO_TEST_CASE(box) {
+  linestring_type_fp path{{0,0}, {0,10}, {10,10}, {10,0}, {0,0}};
+  auto ret = makeBridges(path, 4, 2);
+
+  vector<size_t> expected_ret{1,4,7,10};
+  linestring_type_fp expected_path{
+    {0,0},
+    {0,4},
+    {0,6},
+    {0,10},
+    {4,10},
+    {6,10},
+    {10,10},
+    {10,6},
+    {10,4},
+    {10,0},
+    {6,0},
+    {4,0},
+    {0,0},
+  };
+  BOOST_CHECK_EQUAL(ret, expected_ret);
+  BOOST_CHECK_EQUAL(path, expected_path);
+}
+
+BOOST_AUTO_TEST_CASE(rectangle) {
+  linestring_type_fp path{{0,0}, {0,1}, {10,1}, {10,0}, {0,0}};
+  auto ret = makeBridges(path, 2, 2);
+
+  vector<size_t> expected_ret{0,1,3,4,5,7,8};
+  linestring_type_fp expected_path{
+    {0,0},
+    {0,1},
+    {0.5,1},
+    {9.5,1},
+    {10,1},
+    {10,0},
+    {9.5,0},
+    {0.5,0},
+    {0,0},
+  };
+  BOOST_CHECK_EQUAL(ret, expected_ret);
+  BOOST_CHECK_EQUAL(path, expected_path);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

surface_vectorial.cpp

@@ -929,10 +929,14 @@ void Surface_vectorial::add_mask(shared_ptr<Surface_vectorial> surface) {
 // milling for thermal reliefs.  The voronoi is the shape that
 // encloses the input and outside which we have no need to mill
 // because that will be handled by another call to this function.  The
-// diameter is the diameter of the tool and the overlap is by how much each pass should overlap the prevoius pass.  Steps is
-// how many passes to do, including the first pass.  If do_voronoi is
-// true then isolation should be done from the voronoi region inward
-// instead of from the trace outward.  The offset is how far to kee away from any trace, useful if the milling bit has some diameter that it is guaranteed to mill but also some slop that causes it to sometimes mill beyond its diameter.
+// diameter is the diameter of the tool and the overlap is by how much
+// each pass should overlap the prevoius pass.  Steps is how many
+// passes to do, including the first pass.  If do_voronoi is true then
+// isolation should be done from the voronoi region inward instead of
+// from the trace outward.  The offset is how far to kee away from any
+// trace, useful if the milling bit has some diameter that it is
+// guaranteed to mill but also some slop that causes it to sometimes
+// mill beyond its diameter.
 vector<multi_polygon_type_fp> Surface_vectorial::offset_polygon(
     const optional<polygon_type_fp>& input,
     const polygon_type_fp& voronoi_polygon,