From a6f5fe7bea91e4b89e9a462df1fec2d371b1a30d Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Mon, 23 Sep 2019 11:58:39 +0200 Subject: [PATCH] Fix arrange crash with incorrect geometries. Guard the case with tests. --- .../libnest2d/backends/clipper/geometries.hpp | 19 ++-- .../include/libnest2d/geometry_traits.hpp | 2 +- .../include/libnest2d/placers/nfpplacer.hpp | 73 +------------- .../selections/selection_boilerplate.hpp | 2 +- src/libnest2d/tests/test.cpp | 98 +++++++++++++------ src/libslic3r/Arrange.cpp | 18 ++-- 6 files changed, 92 insertions(+), 120 deletions(-) diff --git a/src/libnest2d/include/libnest2d/backends/clipper/geometries.hpp b/src/libnest2d/include/libnest2d/backends/clipper/geometries.hpp index 56330e15e..57da6ec12 100644 --- a/src/libnest2d/include/libnest2d/backends/clipper/geometries.hpp +++ b/src/libnest2d/include/libnest2d/backends/clipper/geometries.hpp @@ -81,17 +81,16 @@ inline void offset(PolygonImpl& sh, TCoord distance, const PolygonTag using ClipperLib::etClosedPolygon; using ClipperLib::Paths; - // If the input is not at least a triangle, we can not do this algorithm - if(sh.Contour.size() <= 3 || - std::any_of(sh.Holes.begin(), sh.Holes.end(), - [](const PathImpl& p) { return p.size() <= 3; }) - ) throw GeometryException(GeomErr::OFFSET); - - ClipperOffset offs; Paths result; - offs.AddPath(sh.Contour, jtMiter, etClosedPolygon); - offs.AddPaths(sh.Holes, jtMiter, etClosedPolygon); - offs.Execute(result, static_cast(distance)); + + try { + ClipperOffset offs; + offs.AddPath(sh.Contour, jtMiter, etClosedPolygon); + offs.AddPaths(sh.Holes, jtMiter, etClosedPolygon); + offs.Execute(result, static_cast(distance)); + } catch (ClipperLib::clipperException &) { + throw GeometryException(GeomErr::OFFSET); + } // Offsetting reverts the orientation and also removes the last vertex // so boost will not have a closed polygon. diff --git a/src/libnest2d/include/libnest2d/geometry_traits.hpp b/src/libnest2d/include/libnest2d/geometry_traits.hpp index 827e2d8ba..72e239a70 100644 --- a/src/libnest2d/include/libnest2d/geometry_traits.hpp +++ b/src/libnest2d/include/libnest2d/geometry_traits.hpp @@ -1144,7 +1144,7 @@ inline bool isInside(const TBGuest& ibb, const TBHost& box, auto minY = getY(box.minCorner()); auto maxY = getY(box.maxCorner()); - return iminX > minX && imaxX < maxX && iminY > minY && imaxY < maxY; + return iminX >= minX && imaxX <= maxX && iminY >= minY && imaxY <= maxY; } template diff --git a/src/libnest2d/include/libnest2d/placers/nfpplacer.hpp b/src/libnest2d/include/libnest2d/placers/nfpplacer.hpp index 1a341d691..686857a87 100644 --- a/src/libnest2d/include/libnest2d/placers/nfpplacer.hpp +++ b/src/libnest2d/include/libnest2d/placers/nfpplacer.hpp @@ -3,9 +3,6 @@ #include -// For caching nfps -#include - // For parallel for #include #include @@ -76,55 +73,6 @@ inline void enumerate( } -namespace __itemhash { - -using Key = size_t; - -template -Key hash(const _Item& item) { - using Point = TPoint; - using Segment = _Segment; - - static const int N = 26; - static const int M = N*N - 1; - - std::string ret; - auto& rhs = item.rawShape(); - auto& ctr = sl::contour(rhs); - auto it = ctr.begin(); - auto nx = std::next(it); - - double circ = 0; - while(nx != ctr.end()) { - Segment seg(*it++, *nx++); - Radians a = seg.angleToXaxis(); - double deg = Degrees(a); - int ms = 'A', ls = 'A'; - while(deg > N) { ms++; deg -= N; } - ls += int(deg); - ret.push_back(char(ms)); ret.push_back(char(ls)); - circ += std::sqrt(seg.template sqlength()); - } - - it = ctr.begin(); nx = std::next(it); - - while(nx != ctr.end()) { - Segment seg(*it++, *nx++); - auto l = int(M * std::sqrt(seg.template sqlength()) / circ); - int ms = 'A', ls = 'A'; - while(l > N) { ms++; l -= N; } - ls += l; - ret.push_back(char(ms)); ret.push_back(char(ls)); - } - - return std::hash()(ret); -} - -template -using Hash = std::unordered_map>; - -} - namespace placers { template @@ -529,17 +477,9 @@ class _NofitPolyPlacer: public PlacerBoilerplate<_NofitPolyPlacer; - using ItemKeys = std::vector<__itemhash::Key>; - // Norming factor for the optimization function const double norm_; - // Caching calculated nfps - __itemhash::Hash nfpcache_; - - // Storing item hash keys - ItemKeys item_keys_; - public: using Pile = nfp::Shapes; @@ -636,15 +576,12 @@ public: private: using Shapes = TMultiShape; - using ItemRef = std::reference_wrapper; - using ItemWithHash = const std::pair; - Shapes calcnfp(const ItemWithHash itsh, Lvl) + Shapes calcnfp(const Item &trsh, Lvl) { using namespace nfp; Shapes nfps(items_.size()); - const Item& trsh = itsh.first; // ///////////////////////////////////////////////////////////////////// // TODO: this is a workaround and should be solved in Item with mutexes @@ -678,12 +615,11 @@ private: template - Shapes calcnfp( const ItemWithHash itsh, Level) + Shapes calcnfp(const Item &trsh, Level) { // Function for arbitrary level of nfp implementation using namespace nfp; Shapes nfps; - const Item& trsh = itsh.first; auto& orb = trsh.transformedShape(); bool orbconvex = trsh.isContourConvex(); @@ -849,8 +785,6 @@ private: remlist.insert(remlist.end(), remaining.from, remaining.to); } - size_t itemhash = __itemhash::hash(item); - if(items_.empty()) { setInitialPosition(item); best_overfit = overfit(item.transformedShape(), bin_); @@ -875,7 +809,7 @@ private: // it is disjunct from the current merged pile placeOutsideOfBin(item); - nfps = calcnfp({item, itemhash}, Lvl()); + nfps = calcnfp(item, Lvl()); auto iv = item.referenceVertex(); @@ -1112,7 +1046,6 @@ private: if(can_pack) { ret = PackResult(item); - item_keys_.emplace_back(itemhash); } else { ret = PackResult(best_overfit); } diff --git a/src/libnest2d/include/libnest2d/selections/selection_boilerplate.hpp b/src/libnest2d/include/libnest2d/selections/selection_boilerplate.hpp index 2df9a26c3..36fec7164 100644 --- a/src/libnest2d/include/libnest2d/selections/selection_boilerplate.hpp +++ b/src/libnest2d/include/libnest2d/selections/selection_boilerplate.hpp @@ -43,7 +43,7 @@ protected: Placer p{bin}; p.configure(pcfg); - if (!p.pack(cpy)) it = c.erase(it); + if (itm.area() <= 0 || !p.pack(cpy)) it = c.erase(it); else it++; } } diff --git a/src/libnest2d/tests/test.cpp b/src/libnest2d/tests/test.cpp index 4a6691415..5ba28228a 100644 --- a/src/libnest2d/tests/test.cpp +++ b/src/libnest2d/tests/test.cpp @@ -40,7 +40,7 @@ struct NfpImpl } } -std::vector& prusaParts() { +static std::vector& prusaParts() { static std::vector ret; if(ret.empty()) { @@ -51,7 +51,7 @@ std::vector& prusaParts() { return ret; } -TEST(BasicFunctionality, Angles) +TEST(GeometryAlgorithms, Angles) { using namespace libnest2d; @@ -109,7 +109,7 @@ TEST(BasicFunctionality, Angles) } // Simple test, does not use gmock -TEST(BasicFunctionality, creationAndDestruction) +TEST(Nesting, ItemCreationAndDestruction) { using namespace libnest2d; @@ -572,26 +572,74 @@ TEST(GeometryAlgorithms, convexHull) { } -TEST(GeometryAlgorithms, NestTest) { +TEST(Nesting, NestPrusaPartsShouldFitIntoTwoBins) { + + // Get the input items and define the bin. std::vector input = prusaParts(); - - libnest2d::nest(input, Box(250000000, 210000000), [](unsigned cnt) { - std::cout << "parts left: " << cnt << std::endl; + auto bin = Box(250000000, 210000000); + + // Do the nesting. Check in each step if the remaining items are less than + // in the previous step. (Some algorithms can place more items in one step) + size_t pcount = input.size(); + libnest2d::nest(input, bin, [&pcount](unsigned cnt) { + ASSERT_TRUE(cnt < pcount); + pcount = cnt; }); - + + // Get the number of logical bins: search for the max binId... auto max_binid_it = std::max_element(input.begin(), input.end(), [](const Item &i1, const Item &i2) { return i1.binId() < i2.binId(); }); - - size_t bins = max_binid_it == input.end() ? 0 : max_binid_it->binId() + 1; - ASSERT_EQ(bins, 2u); - + auto bins = size_t(max_binid_it == input.end() ? 0 : + max_binid_it->binId() + 1); + + // For prusa parts, 2 bins should be enough... + ASSERT_LE(bins, 2u); + + // All parts should be processed by the algorithm ASSERT_TRUE( std::all_of(input.begin(), input.end(), [](const Item &itm) { return itm.binId() != BIN_ID_UNSET; })); + + // Gather the items into piles of arranged polygons... + using Pile = TMultiShape; + std::vector piles(bins); + + for (auto &itm : input) + piles[size_t(itm.binId())].emplace_back(itm.transformedShape()); + + // Now check all the piles, the bounding box of each pile should be inside + // the defined bin. + for (auto &pile : piles) { + auto bb = sl::boundingBox(pile); + ASSERT_TRUE(sl::isInside(bb, bin)); + } +} + +TEST(Nesting, NestEmptyItemShouldBeUntouched) { + auto bin = Box(250000000, 210000000); // dummy bin + + std::vector items; + items.emplace_back(Item{}); // Emplace empty item + items.emplace_back(Item{0, 200, 0}); // Emplace zero area item + + libnest2d::nest(items, bin); + + for (auto &itm : items) ASSERT_EQ(itm.binId(), BIN_ID_UNSET); +} + +TEST(Nesting, NestLargeItemShouldBeUntouched) { + auto bin = Box(250000000, 210000000); // dummy bin + + std::vector items; + items.emplace_back(Rectangle{250000001, 210000001}); // Emplace large item + + libnest2d::nest(items, bin); + + ASSERT_EQ(items.front().binId(), BIN_ID_UNSET); } namespace { @@ -966,26 +1014,20 @@ using Ratio = boost::rational; } -TEST(RotatingCalipers, MinAreaBBCClk) { - auto u = [](ClipperLib::cInt n) { return n*1000000; }; - PolygonImpl poly({ {u(0), u(0)}, {u(4), u(1)}, {u(2), u(4)}}); +//TEST(GeometryAlgorithms, MinAreaBBCClk) { +// auto u = [](ClipperLib::cInt n) { return n*1000000; }; +// PolygonImpl poly({ {u(0), u(0)}, {u(4), u(1)}, {u(2), u(4)}}); - long double arearef = refMinAreaBox(poly); - long double area = minAreaBoundingBox(poly).area(); +// long double arearef = refMinAreaBox(poly); +// long double area = minAreaBoundingBox(poly).area(); - ASSERT_LE(std::abs(area - arearef), 500e6 ); -} +// ASSERT_LE(std::abs(area - arearef), 500e6 ); +//} -TEST(RotatingCalipers, AllPrusaMinBB) { - // /size_t idx = 0; +TEST(GeometryAlgorithms, MinAreaBBWithRotatingCalipers) { long double err_epsilon = 500e6l; for(ClipperLib::Path rinput : PRINTER_PART_POLYGONS) { - // ClipperLib::Path rinput = PRINTER_PART_POLYGONS[idx]; - // rinput.pop_back(); - // std::reverse(rinput.begin(), rinput.end()); - - // PolygonImpl poly(removeCollinearPoints(rinput, 1000000)); PolygonImpl poly(rinput); long double arearef = refMinAreaBox(poly); @@ -993,8 +1035,6 @@ TEST(RotatingCalipers, AllPrusaMinBB) { long double area = cast(bb.area()); bool succ = std::abs(arearef - area) < err_epsilon; - // std::cout << idx++ << " " << (succ? "ok" : "failed") << " ref: " -// << arearef << " actual: " << area << std::endl; ASSERT_TRUE(succ); } @@ -1011,8 +1051,6 @@ TEST(RotatingCalipers, AllPrusaMinBB) { bool succ = std::abs(arearef - area) < err_epsilon; - // std::cout << idx++ << " " << (succ? "ok" : "failed") << " ref: " -// << arearef << " actual: " << area << std::endl; ASSERT_TRUE(succ); } diff --git a/src/libslic3r/Arrange.cpp b/src/libslic3r/Arrange.cpp index 52168c929..20dfd8926 100644 --- a/src/libslic3r/Arrange.cpp +++ b/src/libslic3r/Arrange.cpp @@ -618,19 +618,21 @@ void arrange(ArrangePolygons & arrangables, items.reserve(arrangables.size()); // Create Item from Arrangeable - auto process_arrangeable = - [](const ArrangePolygon &arrpoly, std::vector &outp) + auto process_arrangeable = [](const ArrangePolygon &arrpoly, + std::vector & outp) { - Polygon p = arrpoly.poly.contour; - const Vec2crd & offs = arrpoly.translation; - double rotation = arrpoly.rotation; + Polygon p = arrpoly.poly.contour; + const Vec2crd &offs = arrpoly.translation; + double rotation = arrpoly.rotation; if (p.is_counter_clockwise()) p.reverse(); clppr::Polygon clpath(Slic3rMultiPoint_to_ClipperPath(p)); - - auto firstp = clpath.Contour.front(); - clpath.Contour.emplace_back(firstp); + + if (!clpath.Contour.empty()) { + auto firstp = clpath.Contour.front(); + clpath.Contour.emplace_back(firstp); + } outp.emplace_back(std::move(clpath)); outp.back().rotation(rotation);