From fdac21b8077c52aba761192a40839a298576ef54 Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Tue, 18 Apr 2023 12:57:00 +0200 Subject: [PATCH] Fix of SPE-1658 GH #9665 Crash at macOS when Orgnanic Support selected Reworked (again!) connecting of islands into a Z-graph. Implemented various heuristics to handle self-intersecting and mutually intersecting ExPolygons on the same layer. --- src/libslic3r/Layer.cpp | 369 ++++++++++++++++++--------- src/libslic3r/TriangleMeshSlicer.cpp | 10 + 2 files changed, 262 insertions(+), 117 deletions(-) diff --git a/src/libslic3r/Layer.cpp b/src/libslic3r/Layer.cpp index 7802fe983..199caf0d0 100644 --- a/src/libslic3r/Layer.cpp +++ b/src/libslic3r/Layer.cpp @@ -78,16 +78,22 @@ void Layer::make_slices() ClipperLib::ClipperOffset co; ClipperLib::Paths out2; - static constexpr const float delta = ClipperSafetyOffset; // *10.f; + // Top / bottom surfaces must overlap more than 2um to be chained into a Z graph. + // Also a larger offset will likely be more robust on non-manifold input polygons. + static constexpr const float delta = scaled(0.001); co.MiterLimit = scaled(3.); // Use the default zero edge merging distance. For this kind of safety offset the accuracy of normal direction is not important. // co.ShortestEdgeLength = delta * ClipperOffsetShortestEdgeFactor; + static constexpr const double accept_area_threshold_ccw = sqr(scaled(0.1 * delta)); + // Such a small hole should not survive the shrinkage, it should grow over + static constexpr const double accept_area_threshold_cw = sqr(scaled(0.2 * delta)); for (const ExPolygon &expoly : expolygons) { contours.clear(); co.Clear(); co.AddPath(expoly.contour.points, ClipperLib::jtMiter, ClipperLib::etClosedPolygon); co.Execute(contours, - delta); + size_t num_prev = out.size(); if (! contours.empty()) { holes.clear(); for (const Polygon &hole : expoly.holes) { @@ -109,13 +115,46 @@ void Layer::make_slices() clipper.Execute(ClipperLib::ctDifference, contours, ClipperLib::pftNonZero, ClipperLib::pftNonZero); } for (const auto &contour : contours) { - out.emplace_back(); - ClipperLib_Z::Path &path = out.back(); - path.reserve(contour.size()); - for (const Point &p : contour) - path.push_back({ p.x(), p.y(), isrc }); + bool accept = true; + // Trying to get rid of offset artifacts, that may be created due to numerical issues in offsetting algorithm + // or due to self-intersections in the source polygons. + //FIXME how reliable is it? Is it helpful or harmful? It seems to do more harm than good as it tends to punch holes + // into existing ExPolygons. +#if 0 + if (contour.size() < 8) { + // Only accept contours with area bigger than some threshold. + double a = ClipperLib::Area(contour); + // Polygon has to be bigger than some threshold to be accepted. + // Hole to be accepted has to have an area slightly bigger than the non-hole, so it will not happen due to rounding errors, + // that a hole will be accepted without its outer contour. + accept = a > 0 ? a > accept_area_threshold_ccw : a < - accept_area_threshold_cw; + } +#endif + if (accept) { + out.emplace_back(); + ClipperLib_Z::Path &path = out.back(); + path.reserve(contour.size()); + for (const Point &p : contour) + path.push_back({ p.x(), p.y(), isrc }); + } } } +#if 0 // #ifndef NDEBUG + // Test whether the expolygons in a single layer overlap. + Polygons test; + for (size_t i = num_prev; i < out.size(); ++ i) + test.emplace_back(ClipperZUtils::from_zpath(out[i])); + Polygons outside = diff(test, to_polygons(expoly)); + if (! outside.empty()) { + BoundingBox bbox(get_extents(expoly)); + bbox.merge(get_extents(test)); + SVG svg(debug_out_path("expolygons_to_zpaths_shrunk-self-intersections.svg").c_str(), bbox); + svg.draw(expoly, "blue"); + svg.draw(test, "green"); + svg.draw(outside, "red"); + } + assert(outside.empty()); +#endif // NDEBUG ++ isrc; } @@ -164,121 +203,36 @@ static void connect_layer_slices( if (polynode.Contour.size() >= 3) { // If there is an intersection point, it should indicate which contours (one from layer below, the other from layer above) intersect. // Otherwise the contour is fully inside another contour. - int32_t i = -1, j = -1; - for (int icontour = 0; icontour <= polynode.ChildCount(); ++ icontour) { - const ClipperLib_Z::Path &contour = icontour == 0 ? polynode.Contour : polynode.Childs[icontour - 1]->Contour; - if (contour.size() >= 3) { - for (const ClipperLib_Z::IntPoint &pt : contour) { - j = pt.z(); - if (j < 0) { - const auto &intersection = m_intersections[-j - 1]; - assert(intersection.first <= intersection.second); - if (intersection.second < m_offset_above) { - // Ignore intersection of polygons on the 1st layer. - assert(intersection.first >= m_offset_below); - j = i; - } else if (intersection.first >= m_offset_above) { - // Ignore intersection of polygons on the 2nd layer - assert(intersection.second < m_offset_end); - j = i; - } else { - std::tie(i, j) = m_intersections[-j - 1]; - assert(assert_intersection_valid(i, j)); - goto end; - } - } else if (i == -1) { - // First source contour of this expolygon was found. - i = j; - } else if (i != j) { - // Second source contour of this expolygon was found. - if (i > j) - std::swap(i, j); - assert(assert_intersection_valid(i, j)); - goto end; - } - } + auto [i, j] = this->find_top_bottom_contour_ids_strict(polynode); + bool found = false; + if (i < 0 && j < 0) { + // This should not happen. It may only happen if the source contours had just self intersections or intersections with contours at the same layer. + // We may safely ignore such cases where the intersection area is meager. + double a = ClipperLib_Z::Area(polynode.Contour); + if (a < sqr(scaled(0.001))) { + // Ignore tiny overlaps. They are not worth resolving. + } else { + // We should not ignore large cases. Try to resolve the conflict by a majority of references. + std::tie(i, j) = this->find_top_bottom_contour_ids_approx(polynode); + // At least top or bottom should be resolved. + assert(i >= 0 || j >= 0); } } - end: - bool found = false; - if (i == -1) { - // This should not happen. It may only happen if the source contours had just self intersections or intersections with contours at the same layer. - assert(false); - } else if (i == j) { - // The contour is completely inside another contour. - Point pt(polynode.Contour.front().x(), polynode.Contour.front().y()); - if (i < m_offset_above) { - // Index of an island below. Look-it up in the island above. - assert(i >= m_offset_below); - i -= m_offset_below; - for (int l = int(m_above.lslices_ex.size()) - 1; l >= 0; -- l) { - LayerSlice &lslice = m_above.lslices_ex[l]; - if (lslice.bbox.contains(pt) && m_above.lslices[l].contains(pt)) { - found = true; - j = l; - assert(i >= 0 && i < m_below.lslices_ex.size()); - assert(j >= 0 && j < m_above.lslices_ex.size()); - break; - } - } - //FIXME remove the following block one day, it should not be needed. - // The following shall not happen now as the source expolygons are being shrunk a bit before intersecting, - // thus each point of each intersection polygon should fit completely inside one of the original (unshrunk) expolygons. - assert(found); - if (!found) { - // The check above might sometimes fail when the polygons overlap only on points, which causes the clipper to detect no intersection. - // The problem happens rarely, mostly on simple polygons (in terms of number of points), but regardless of size! - // example of failing link on two layers, each with single polygon without holes. - // layer A = Polygon{(-24931238,-11153865),(-22504249,-8726874),(-22504249,11477151),(-23261469,12235585),(-23752371,12727276),(-25002495,12727276),(-27502745,10227026),(-27502745,-12727274),(-26504645,-12727274)} - // layer B = Polygon{(-24877897,-11100524),(-22504249,-8726874),(-22504249,11477151),(-23244827,12218916),(-23752371,12727276),(-25002495,12727276),(-27502745,10227026),(-27502745,-12727274),(-26504645,-12727274)} - // note that first point is not identical, and the check above picks (-24877897,-11100524) as the first contour point (polynode.Contour.front()). - // that point is sadly slightly outisde of the layer A, so no link is detected, eventhough they are overlaping "completely" - Polygon contour_poly(ClipperZUtils::from_zpath(polynode.Contour)); - BoundingBox contour_aabb{contour_poly.points}; - for (int l = int(m_above.lslices_ex.size()) - 1; l >= 0; --l) { - LayerSlice &lslice = m_above.lslices_ex[l]; - // it is potentially slow, but should be executed rarely - if (contour_aabb.overlap(lslice.bbox) && !intersection(Polygons{contour_poly}, m_above.lslices[l]).empty()) { - found = true; - j = l; - assert(i >= 0 && i < m_below.lslices_ex.size()); - assert(j >= 0 && j < m_above.lslices_ex.size()); - break; - } - } - } + if (j < 0) { + if (i < 0) { + // this->find_top_bottom_contour_ids_approx() shoudl have made sure this does not happen. + assert(false); } else { - // Index of an island above. Look-it up in the island below. - assert(j < m_offset_end); - j -= m_offset_above; - for (int l = int(m_below.lslices_ex.size()) - 1; l >= 0; -- l) { - LayerSlice &lslice = m_below.lslices_ex[l]; - if (lslice.bbox.contains(pt) && m_below.lslices[l].contains(pt)) { - found = true; - i = l; - assert(i >= 0 && i < m_below.lslices_ex.size()); - assert(j >= 0 && j < m_above.lslices_ex.size()); - break; - } - } - //FIXME remove the following block one day, it should not be needed. - // The following shall not happen now as the source expolygons are being shrunk a bit before intersecting, - // thus each point of each intersection polygon should fit completely inside one of the original (unshrunk) expolygons. - if (!found) { // Explanation for aditional check is above. - Polygon contour_poly(ClipperZUtils::from_zpath(polynode.Contour)); - BoundingBox contour_aabb{contour_poly.points}; - for (int l = int(m_below.lslices_ex.size()) - 1; l >= 0; --l) { - LayerSlice &lslice = m_below.lslices_ex[l]; - if (contour_aabb.overlap(lslice.bbox) && !intersection(Polygons{contour_poly}, m_below.lslices[l]).empty()) { - found = true; - i = l; - assert(i >= 0 && i < m_below.lslices_ex.size()); - assert(j >= 0 && j < m_above.lslices_ex.size()); - break; - } - } - } + assert(i >= m_offset_below && i < m_offset_above); + i -= m_offset_below; + j = this->find_other_contour_costly(polynode, m_above, j == -2); + found = j >= 0; } + } else if (i < 0) { + assert(j >= m_offset_above && j < m_offset_end); + j -= m_offset_above; + i = this->find_other_contour_costly(polynode, m_below, i == -2); + found = i >= 0; } else { assert(assert_intersection_valid(i, j)); i -= m_offset_below; @@ -329,6 +283,187 @@ static void connect_layer_slices( } private: + // Find the indices of the contour below & above for an expolygon created as an intersection of two expolygons, one below, the other above. + // Returns -1 if there is no point on the intersection refering bottom resp. top source expolygon. + // Returns -2 if the intersection refers to multiple source expolygons on bottom resp. top layers. + std::pair find_top_bottom_contour_ids_strict(const ClipperLib_Z::PolyNode &polynode) const + { + // If there is an intersection point, it should indicate which contours (one from layer below, the other from layer above) intersect. + // Otherwise the contour is fully inside another contour. + int32_t i = -1, j = -1; + auto process_i = [&i, &j](coord_t k) { + if (i == -1) + i = k; + else if (i >= 0) { + if (i != k) { + // Error: Intersection contour contains points of two or more source bottom contours. + i = -2; + if (j == -2) + // break + return true; + } + } else + assert(i == -2); + return false; + }; + auto process_j = [&i, &j](coord_t k) { + if (j == -1) + j = k; + else if (j >= 0) { + if (j != k) { + // Error: Intersection contour contains points of two or more source top contours. + j = -2; + if (i == -2) + // break + return true; + } + } else + assert(j == -2); + return false; + }; + for (int icontour = 0; icontour <= polynode.ChildCount(); ++ icontour) { + const ClipperLib_Z::Path &contour = icontour == 0 ? polynode.Contour : polynode.Childs[icontour - 1]->Contour; + if (contour.size() >= 3) { + for (const ClipperLib_Z::IntPoint &pt : contour) + if (coord_t k = pt.z(); k < 0) { + const auto &intersection = m_intersections[-k - 1]; + assert(intersection.first <= intersection.second); + if (intersection.first < m_offset_above ? process_i(intersection.first) : process_j(intersection.first)) + goto end; + if (intersection.second < m_offset_above ? process_i(intersection.second) : process_j(intersection.second)) + goto end; + } else if (k < m_offset_above ? process_i(k) : process_j(k)) + goto end; + } + } + end: + return { i, j }; + } + + // Find the indices of the contour below & above for an expolygon created as an intersection of two expolygons, one below, the other above. + // This variant expects that the source expolygon assingment is not unique, it counts the majority. + // Returns -1 if there is no point on the intersection refering bottom resp. top source expolygon. + // Returns -2 if the intersection refers to multiple source expolygons on bottom resp. top layers. + std::pair find_top_bottom_contour_ids_approx(const ClipperLib_Z::PolyNode &polynode) const + { + // 1) Collect histogram of contour references. + struct HistoEl { + int32_t id; + int32_t count; + }; + std::vector histogram; + { + auto increment_counter = [&histogram](const int32_t i) { + auto it = std::lower_bound(histogram.begin(), histogram.end(), i, [](auto l, auto r){ return l.id < r; }); + if (it == histogram.end() || it->id != i) + histogram.insert(it, HistoEl{ i, int32_t(1) }); + else + ++ it->count; + }; + for (int icontour = 0; icontour <= polynode.ChildCount(); ++ icontour) { + const ClipperLib_Z::Path &contour = icontour == 0 ? polynode.Contour : polynode.Childs[icontour - 1]->Contour; + if (contour.size() >= 3) { + for (const ClipperLib_Z::IntPoint &pt : contour) + if (coord_t k = pt.z(); k < 0) { + const auto &intersection = m_intersections[-k - 1]; + assert(intersection.first <= intersection.second); + increment_counter(intersection.first); + increment_counter(intersection.second); + } else + increment_counter(k); + } + } + assert(! histogram.empty()); + } + int32_t i = -1; + int32_t j = -1; + if (! histogram.empty()) { + // 2) Split the histogram to bottom / top. + auto mid = std::upper_bound(histogram.begin(), histogram.end(), m_offset_above, [](auto l, auto r){ return l < r.id; }); + // 3) Sort the bottom / top parts separately. + auto bottom_begin = histogram.begin(); + auto bottom_end = mid; + auto top_begin = mid; + auto top_end = histogram.end(); + std::sort(bottom_begin, bottom_end, [](auto l, auto r) { return l.count > r.count; }); + std::sort(top_begin, top_end, [](auto l, auto r) { return l.count > r.count; }); + double i_quality = 0; + double j_quality = 0; + if (bottom_begin != bottom_end) { + i = bottom_begin->id; + i_quality = std::next(bottom_begin) == bottom_end ? std::numeric_limits::max() : double(bottom_begin->count) / std::next(bottom_begin)->count; + } + if (top_begin != top_end) { + j = top_begin->id; + j_quality = std::next(top_begin) == top_end ? std::numeric_limits::max() : double(top_begin->count) / std::next(top_begin)->count; + } + // Expected to be called only if there are duplicate references to be resolved by the histogram. + assert(i >= 0 || j >= 0); + assert(i_quality < std::numeric_limits::max() || j_quality < std::numeric_limits::max()); + if (i >= 0 && i_quality < j_quality) { + // Force the caller to resolve the bottom references the costly but robust way. + assert(j >= 0); + // Twice the number of references for the best contour. + assert(j_quality >= 2.); + i = -2; + } else if (j >= 0) { + // Force the caller to resolve the top reference the costly but robust way. + assert(i >= 0); + // Twice the number of references for the best contour. + assert(i_quality >= 2.); + j = -2; + } + + } + return { i, j }; + } + + static int32_t find_other_contour_costly(const ClipperLib_Z::PolyNode &polynode, const Layer &other_layer, bool other_has_duplicates) + { + if (! other_has_duplicates) { + // The contour below is likely completely inside another contour above. Look-it up in the island above. + Point pt(polynode.Contour.front().x(), polynode.Contour.front().y()); + for (int i = int(other_layer.lslices_ex.size()) - 1; i >= 0; -- i) + if (other_layer.lslices_ex[i].bbox.contains(pt) && other_layer.lslices[i].contains(pt)) + return i; + // The following shall not happen now as the source expolygons are being shrunk a bit before intersecting, + // thus each point of each intersection polygon should fit completely inside one of the original (unshrunk) expolygons. + assert(false); + } + // The comment below may not be valid anymore, see the comment above. However the code is used in case the polynode contains multiple references + // to other_layer expolygons, thus the references are not unique. + // + // The check above might sometimes fail when the polygons overlap only on points, which causes the clipper to detect no intersection. + // The problem happens rarely, mostly on simple polygons (in terms of number of points), but regardless of size! + // example of failing link on two layers, each with single polygon without holes. + // layer A = Polygon{(-24931238,-11153865),(-22504249,-8726874),(-22504249,11477151),(-23261469,12235585),(-23752371,12727276),(-25002495,12727276),(-27502745,10227026),(-27502745,-12727274),(-26504645,-12727274)} + // layer B = Polygon{(-24877897,-11100524),(-22504249,-8726874),(-22504249,11477151),(-23244827,12218916),(-23752371,12727276),(-25002495,12727276),(-27502745,10227026),(-27502745,-12727274),(-26504645,-12727274)} + // note that first point is not identical, and the check above picks (-24877897,-11100524) as the first contour point (polynode.Contour.front()). + // that point is sadly slightly outisde of the layer A, so no link is detected, eventhough they are overlaping "completely" + Polygons contour_poly{ Polygon{ClipperZUtils::from_zpath(polynode.Contour)} }; + BoundingBox contour_aabb{contour_poly.front().points}; + int32_t i_largest = -1; + double a_largest = 0; + for (int i = int(other_layer.lslices_ex.size()) - 1; i >= 0; -- i) + if (contour_aabb.overlap(other_layer.lslices_ex[i].bbox)) + // it is potentially slow, but should be executed rarely + if (Polygons overlap = intersection(contour_poly, other_layer.lslices[i]); ! overlap.empty()) + if (other_has_duplicates) { + // Find the contour with the largest overlap. It is expected that the other overlap will be very small. + double a = area(overlap); + if (a > a_largest) { + a_largest = a; + i_largest = i; + } + } else { + // Most likely there is just one contour that overlaps, however it is not guaranteed. + i_largest = i; + break; + } + assert(i_largest >= 0); + return i_largest; + } + const std::vector> &m_intersections; Layer &m_below; Layer &m_above; diff --git a/src/libslic3r/TriangleMeshSlicer.cpp b/src/libslic3r/TriangleMeshSlicer.cpp index 329c1c4ca..460cd901e 100644 --- a/src/libslic3r/TriangleMeshSlicer.cpp +++ b/src/libslic3r/TriangleMeshSlicer.cpp @@ -1911,6 +1911,16 @@ std::vector slice_mesh_ex( this_mode == MeshSlicingParams::SlicingMode::EvenOdd ? ClipperLib::pftEvenOdd : this_mode == MeshSlicingParams::SlicingMode::PositiveLargestContour ? ClipperLib::pftPositive : ClipperLib::pftNonZero, &expolygons); + +#if 0 +//#ifndef _NDEBUG + // Test whether the expolygons in a single layer overlap. + for (size_t i = 0; i < expolygons.size(); ++ i) + for (size_t j = i + 1; j < expolygons.size(); ++ j) { + Polygons overlap = intersection(expolygons[i], expolygons[j]); + assert(overlap.empty()); + } +#endif #if 0 //#ifndef _NDEBUG for (const ExPolygon &ex : expolygons) {