Skip to content

Commit

Permalink
Fix reverse order for perimeters (also for first layer with brim)
Browse files Browse the repository at this point in the history
  • Loading branch information
@supermerill
supermerill committed Jan 8, 2024
1 parent fd948b5 commit 1b4d227
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 90 deletions.
5 changes: 5 additions & 0 deletions src/libslic3r/ExtrusionEntityCollection.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ class ExtrusionEntityCollection : public ExtrusionEntity
return *this;
}
~ExtrusionEntityCollection() override { clear(); }
// move all entitites from src into this
void append_move_from(ExtrusionEntityCollection &src) {
m_entities.insert(m_entities.end(), src.m_entities.begin(), src.m_entities.end());
src.m_entities.clear();
}

/// Operator to convert and flatten this collection to a single vector of ExtrusionPaths.
explicit operator ExtrusionPaths() const;
Expand Down
129 changes: 54 additions & 75 deletions src/libslic3r/PerimeterGenerator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,25 @@
#endif

namespace Slic3r {

//struct CollectionSimplifyVisitor : public ExtrusionVisitor
//{
// ExtrusionEntityCollection *previous = nullptr;
// virtual void default_use(ExtrusionEntity& entity) override {};
// virtual void use(ExtrusionEntityCollection &coll) override
// {
// if (previous && coll.entities().size() == 1) {
// previous->append_move_from(coll);
// } else {
// ExtrusionEntityCollection *old_previous = previous;
// previous = &coll;
// for (size_t i = 0; i < coll.entities().size(); ++i) { coll.set_entities()[i]->visit(*this); }
// previous = old_previous;
// }
// }
//};


PerimeterGeneratorLoops get_all_Childs(PerimeterGeneratorLoop loop) {
PerimeterGeneratorLoops ret;
for (PerimeterGeneratorLoop &child : loop.children) {
Expand Down Expand Up @@ -348,8 +367,10 @@ ProcessSurfaceResult PerimeterGenerator::process_arachne(int& loop_number, const
std::unordered_map<const Arachne::ExtrusionLine*, size_t> map_extrusion_to_idx;
for (size_t idx = 0; idx < all_extrusions.size(); idx++)
map_extrusion_to_idx.emplace(all_extrusions[idx], idx);

auto extrusions_constrains = Arachne::WallToolPaths::getRegionOrder(all_extrusions, this->config->external_perimeters_first);

//TODO: order extrusion for contour/hole separatly
bool reverse_order = this->config->external_perimeters_first || this->object_config->brim_width.value > 0 || this->object_config->brim_width_interior.value > 0;
auto extrusions_constrains = Arachne::WallToolPaths::getRegionOrder(all_extrusions, reverse_order);
for (auto [before, after] : extrusions_constrains) {
auto after_it = map_extrusion_to_idx.find(after);
++blocked[after_it->second];
Expand All @@ -360,7 +381,7 @@ ProcessSurfaceResult PerimeterGenerator::process_arachne(int& loop_number, const
Point current_position = all_extrusions.empty() ? Point::Zero() : all_extrusions.front()->junctions.front().p; // Some starting position.
std::vector<PerimeterGeneratorArachneExtrusion> ordered_extrusions; // To store our result in. At the end we'll std::swap.
ordered_extrusions.reserve(all_extrusions.size());

while (ordered_extrusions.size() < all_extrusions.size()) {
size_t best_candidate = 0;
double best_distance_sqr = std::numeric_limits<double>::max();
Expand Down Expand Up @@ -1598,6 +1619,19 @@ ProcessSurfaceResult PerimeterGenerator::process_classic(int& loop_number, const
#if _DEBUG
peri_entities.visit(LoopAssertVisitor{});
#endif
// remove the un-needed top collection if only one child.
//peri_entities.visit(CollectionSimplifyVisitor{});
if (peri_entities.entities().size() == 1) {
if (ExtrusionEntityCollection *coll_child = dynamic_cast<ExtrusionEntityCollection *>(
peri_entities.set_entities().front());
coll_child != nullptr) {
peri_entities.set_can_sort_reverse(coll_child->can_sort(), coll_child->can_reverse());
peri_entities.append_move_from(*coll_child);
peri_entities.remove(0);
}
}


//{
// static int aodfjiaqsdz = 0;
// std::stringstream stri;
Expand All @@ -1613,69 +1647,6 @@ ProcessSurfaceResult PerimeterGenerator::process_classic(int& loop_number, const
// svg.Close();
//}


// if brim will be printed, reverse the order of perimeters so that
// we continue inwards after having finished the brim
// be careful to not print thin walls before perimeters (gapfill will be added after so don't worry for them)
// TODO: add test for perimeter order
const bool brim_first_layer = this->layer->id() == 0 && (this->object_config->brim_width.value > 0 || this->object_config->brim_width_interior.value > 0);
if (this->config->external_perimeters_first || brim_first_layer) {
if (this->config->external_perimeters_nothole.value || brim_first_layer) {
if (this->config->external_perimeters_hole.value || brim_first_layer) {
//reverse only not-thin wall
ExtrusionEntityCollection coll2;
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if ( (loop->is_loop() && loop->role() != erThinWall)) {
coll2.append(*loop);
}
}
coll2.reverse();
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if (!((loop->is_loop() && loop->role() != erThinWall))) {
coll2.append(*loop);
}
}
//note: this hacky thing is possible because coll2.entities contains in fact peri_entities's entities
//if you does peri_entities = coll2, you'll delete peri_entities's entities() and then you have nothing.
peri_entities = std::move(coll2);
} else {
//reverse only not-hole perimeters
ExtrusionEntityCollection coll2;
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if ((loop->is_loop() && loop->role() != erThinWall) && !(((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) != 0) {
coll2.append(*loop);
}
}
coll2.reverse();
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if (!((loop->is_loop() && loop->role() != erThinWall) && !(((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) != 0)) {
coll2.append(*loop);
}
}
//note: this hacky thing is possible because coll2.entities contains in fact entities's entities
//if you does peri_entities = coll2, you'll delete peri_entities's entities and then you have nothing.
peri_entities = std::move(coll2);
}
} else if (this->config->external_perimeters_hole.value) {
//reverse the hole, and put them in first place.
ExtrusionEntityCollection coll2;
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if ((loop->is_loop() && loop->role() != erThinWall) && (((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) != 0) {
coll2.append(*loop);
}
}
coll2.reverse();
for (const ExtrusionEntity* loop : peri_entities.entities()) {
if (!((loop->is_loop() && loop->role() != erThinWall) && (((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) != 0)) {
coll2.append(*loop);
}
}
//note: this hacky thing is possible because coll2.entities contains in fact peri_entities's entities
//if you does peri_entities = coll2, you'll delete peri_entities's entities and then you have nothing.
peri_entities = std::move(coll2);
}

}
// append perimeters for this slice as a collection
if (!peri_entities.empty()) {
//move it, to avoid to clone evrything and then delete it
Expand Down Expand Up @@ -2651,6 +2622,13 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_loops(
if (idx.first >= loops.size())
better_chain.push_back(idx);
}

// if brim will be printed, reverse the order of perimeters so that
// we continue inwards after having finished the brim
const bool reverse_contour = (this->layer->id() == 0 && this->object_config->brim_width.value > 0) ||
(this->config->external_perimeters_first.value && this->config->external_perimeters_nothole.value);
const bool reverse_hole = (this->layer->id() == 0 && this->object_config->brim_width_interior.value > 0) ||
(this->config->external_perimeters_first.value && this->config->external_perimeters_hole.value);

//move from coll to coll_out and getting children of each in the same time. (deep first)
for (const std::pair<size_t, bool> &idx : better_chain) {
Expand Down Expand Up @@ -2691,28 +2669,30 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_loops(
assert(thin_walls.empty());
ExtrusionEntityCollection children = this->_traverse_loops(loop.children, thin_walls, has_overhang ? 1 : count_since_overhang < 0 ? -1 : (count_since_overhang+1));
coll[idx.first] = nullptr;
if (loop.is_contour) {
if ((loop.is_contour && !reverse_contour) || (!loop.is_contour && reverse_hole)) {
//note: this->layer->id() % 2 == 1 already taken into account in the is_steep_overhang compute (to save time).
if (loop.is_steep_overhang && this->layer->id() % 2 == 1)
// if contour: reverse if steep_overhang & odd. if hole: the opposite
if ((loop.is_steep_overhang && this->layer->id() % 2 == 1) == loop.is_contour)
eloop->make_clockwise();
else
eloop->make_counter_clockwise();
//ensure that our children are printed before us
if (!children.empty()) {
ExtrusionEntityCollection print_child_beforeplz;
print_child_beforeplz.set_can_sort_reverse(false, false);
if (children.entities().size() > 1) {
if (children.entities().size() > 1 && (children.can_reverse() || children.can_sort())) {
print_child_beforeplz.append(children);
} else {
print_child_beforeplz.append(std::move(children.entities()));
print_child_beforeplz.append_move_from(children);
}
print_child_beforeplz.append(*eloop);
coll_out.append(std::move(print_child_beforeplz));
} else {
coll_out.append(*eloop);
}
} else {
if (loop.is_steep_overhang && this->layer->id() % 2 == 1)
// if hole: reverse if steep_overhang & odd. if contour: the opposite
if ((loop.is_steep_overhang && this->layer->id() % 2 == 1) != loop.is_contour)
eloop->make_counter_clockwise();
else
eloop->make_clockwise();
Expand All @@ -2721,10 +2701,10 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_loops(
ExtrusionEntityCollection print_child_beforeplz;
print_child_beforeplz.set_can_sort_reverse(false, false);
print_child_beforeplz.append(*eloop);
if (children.entities().size() > 1) {
if (children.entities().size() > 1 && (children.can_reverse() || children.can_sort())) {
print_child_beforeplz.append(children);
} else {
print_child_beforeplz.append(std::move(children.entities()));
print_child_beforeplz.append_move_from(children);
}
coll_out.append(std::move(print_child_beforeplz));
} else {
Expand Down Expand Up @@ -2778,7 +2758,7 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_extrusions(std::vector<P
ClipperLib_Z::Path extrusion_path;
extrusion_path.reserve(extrusion->size());
for (const Arachne::ExtrusionJunction& ej : extrusion->junctions) {
//remove duplicate poitns from arachne
//remove duplicate points from arachne
if(extrusion_path.empty() ||
(ej.p.x() != extrusion_path.back().x() || ej.p.y() != extrusion_path.back().y()))
extrusion_path.emplace_back(ej.p.x(), ej.p.y(), ej.w);
Expand Down Expand Up @@ -3016,7 +2996,6 @@ void PerimeterGenerator::_merge_thin_walls(ExtrusionEntityCollection &extrusions
current_loop = last_loop;
}
virtual void use(ExtrusionEntityCollection &collection) override {
collection.set_can_sort_reverse(true, true);
//for each loop? (or other collections)
for (ExtrusionEntity *entity : collection.entities())
entity->visit(*this);
Expand Down
16 changes: 4 additions & 12 deletions src/libslic3r/Print.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void Print::clear()
}

// Called by Print::apply().
// This method only accepts PrintConfig option keys.
// This method only accepts PrintConfig option keys. Not PrintObjectConfig or PrintRegionConfig, go to PrintObject for these
bool Print::invalidate_state_by_config_options(const ConfigOptionResolver& /* new_config */, const std::vector<t_config_option_key> &opt_keys)
{
if (opt_keys.empty())
Expand All @@ -67,13 +67,14 @@ bool Print::invalidate_state_by_config_options(const ConfigOptionResolver& /* ne
"avoid_crossing_top",
"bed_shape",
"bed_temperature",
"chamber_temperature",
"before_layer_gcode",
"between_objects_gcode",
"bridge_acceleration",
"bridge_internal_acceleration",
"bridge_fan_speed",
"bridge_internal_fan_speed",
"brim_acceleration",
"chamber_temperature",
"colorprint_heights",
"complete_objects_sort",
"cooling",
Expand Down Expand Up @@ -230,16 +231,7 @@ bool Print::invalidate_state_by_config_options(const ConfigOptionResolver& /* ne
} else if (steps_ignore.find(opt_key) != steps_ignore.end()) {
// These steps have no influence on the G-code whatsoever. Just ignore them.
} else if (
opt_key == "brim_inside_holes"
|| opt_key == "brim_width"
|| opt_key == "brim_width_interior"
|| opt_key == "brim_ears"
|| opt_key == "brim_ears_detection_length"
|| opt_key == "brim_ears_max_angle"
|| opt_key == "brim_ears_pattern"
|| opt_key == "brim_per_object"
|| opt_key == "brim_separation"
|| opt_key == "complete_objects_one_skirt"
opt_key == "complete_objects_one_skirt"
|| opt_key == "draft_shield"
|| opt_key == "min_skirt_length"
|| opt_key == "ooze_prevention"
Expand Down
10 changes: 8 additions & 2 deletions src/libslic3r/PrintObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,8 +1086,6 @@ bool PrintObject::invalidate_state_by_config_options(
invalidated |= m_print->invalidate_step(psGCodeExport);
} else if (
opt_key == "brim_inside_holes"
|| opt_key == "brim_width"
|| opt_key == "brim_width_interior"
|| opt_key == "brim_ears"
|| opt_key == "brim_ears_detection_length"
|| opt_key == "brim_ears_max_angle"
Expand All @@ -1097,6 +1095,14 @@ bool PrintObject::invalidate_state_by_config_options(
invalidated |= m_print->invalidate_step(psSkirtBrim);
// Brim is printed below supports, support invalidates brim and skirt.
steps.emplace_back(posSupportMaterial);
} else if (
opt_key == "brim_width"
|| opt_key == "brim_width_interior") {
invalidated |= m_print->invalidate_step(psSkirtBrim);
// these two may change the ordering of first layer perimeters
steps.emplace_back(posPerimeters);
// Brim is printed below supports, support invalidates brim and skirt.
steps.emplace_back(posSupportMaterial);
} else {
// for legacy, if we can't handle this option let's invalidate all steps
this->invalidate_all_steps();
Expand Down
2 changes: 1 addition & 1 deletion src/libslic3r/ShortestPath.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ std::vector<std::pair<size_t, bool>> chain_segments_greedy2(SegmentEndPointFunc
std::vector<std::pair<size_t, bool>> chain_extrusion_entities(std::vector<ExtrusionEntity*> &entities, const Point *start_near)
{
auto segment_end_point = [&entities](size_t idx, bool first_point) -> const Point& { return first_point ? entities[idx]->first_point() : entities[idx]->last_point(); };
auto could_reverse = [&entities](size_t idx) { const ExtrusionEntity *ee = entities[idx]; return ee->is_loop() || ee->can_reverse(); };
auto could_reverse = [&entities](size_t idx) { const ExtrusionEntity *ee = entities[idx]; return ee->can_reverse(); };
std::vector<std::pair<size_t, bool>> out = chain_segments_greedy_constrained_reversals<Point, decltype(segment_end_point), decltype(could_reverse)>(segment_end_point, could_reverse, entities.size(), start_near);
for (std::pair<size_t, bool> &segment : out) {
ExtrusionEntity *ee = entities[segment.first];
Expand Down

0 comments on commit 1b4d227

Please sign in to comment.