Perimeter loop : continuous mode for perimeters #4695
Conversation
…nto a big one) - add perimeter loop seam option : rear => it put the externals ones on -y and the internal ones on +y, if possible.
|
I will want some general tests of its functionality in the c++ tests before I can merge. |
|
✅ Build Slic3r 1.3.0-master-2249 completed (commit 11a1a48f22 by @) |
|
https://dl.slic3r.org/dev/linux/branches/Slic3r-1.3.1-dev-11a1a48-PR4695-x86_64.AppImage Linux AppImage for people wanting to play with it. There should be a windows build and osx build as well. |
|
|
||
| /// Projection of a point onto the polygon. | ||
| /// return the parameter if nothing can be find. | ||
| Point MultiPoint::point_projection(const Point &point) const { |
There was a problem hiding this comment.
Slic3r/xs/src/libslic3r/Point.cpp
Line 256 in d5a2e1b
Or use
projection_onto to find correct point
| PerimeterGenerator::_get_nearest_point(const PerimeterGeneratorLoops &children, ExtrusionLoop &myPolylines, const coord_t dist_cut, const coord_t max_dist) const { | ||
| //find best points of intersections | ||
| PerimeterIntersectionPoint intersect; | ||
| intersect.distance = 0x7FFFFFFF; // ! assumption on intersect type & max value |
There was a problem hiding this comment.
decltype and numeric_limits may help here
| const Point &nearest_p = child.polygon.points[child.polygon.closest_point_index(p)]; | ||
| const double dist = nearest_p.distance_to(p); | ||
| //Try to find a point in the far side, aligning them | ||
| if (dist + dist_cut / 20 < intersect.distance || |
| //Try to find a point in the far side, aligning them | ||
| if (dist + dist_cut / 20 < intersect.distance || | ||
| (config->perimeter_loop_seam.value == spRear && (intersect.idx_polyline_outter <0 || p.y > intersect.outter_best.y) | ||
| && dist <= max_dist && intersect.distance + dist_cut / 20)) { |
There was a problem hiding this comment.
what does intersect.distance + dist_cut / 20 mean?
| intersect.child_best = nearest_p; | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
a lot of repeated code in this branch, it should be quite easy to merge
| } | ||
| } | ||
| if (idx_before == (size_t)-1) { | ||
| std::cout << "ERROR: idx_before can't be finded\n"; |
| if (length_poly_1 < length_trim_1) { | ||
| length_trim_2 = length_trim_1 + length_trim_2 - length_poly_1; | ||
| } | ||
| if (length_poly_2 < length_trim_1) { |
There was a problem hiding this comment.
is length_trim_1 correct?
| length_poly_2 = inner_end->polyline.length(); | ||
| length_trim_1 = inner_child_spacing / 2; | ||
| length_trim_2 = inner_child_spacing / 2; | ||
| if (length_poly_1 < length_trim_1) { |
There was a problem hiding this comment.
duplicate code, may be moved into function
| //travel_path_begin[2].extruder_id = -1; | ||
| Line line(outer_start->polyline.points.back(), inner_start->polyline.points.front()); | ||
| Point p_dist_cut_extrude = (line.b - line.a); | ||
| p_dist_cut_extrude.x = (coord_t)(p_dist_cut_extrude.x * ((double)max_width_extrusion) / (line.length() * 2)); |
There was a problem hiding this comment.
scale()
Also - longer line is cut by smaller amount, is that desirable?
There was a problem hiding this comment.
Sorry, the code is fine ...
But the normalization is very well hidden ..
It may be better to switch to coordf_t, normalize and scale
| travel_path_begin[0].polyline.append(inner_start->polyline.points.front()); | ||
| } | ||
| dist_travel = inner_end->polyline.points.back().distance_to(outer_end->polyline.points.front()); | ||
| if (dist_travel > max_width_extrusion*1.5 && this->config->fill_density.value > 0) { |
|
I'm going to need a test or two for regressions before I can merge. I think it should be sufficient to check that the perimeter is a single polyline. |
|
I'm a bit full of work just right now. |
|
I think this features is super good. SImilar functionality exists in other slicer and the resultas are really good. If needed, I offer my help in testing the functionality. |
It merges the loops into a big one
also add perimeter loop seam option : rear => it put the externals ones on -y and the internal ones on +y, if possible.
see #4552
thanks to @gotthalmseder for sponsoring this feature.