From ffe59a870f6895f813c5bf36befa9dd050e60978 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Fri, 22 Nov 2024 09:02:31 +0100 Subject: [PATCH 1/4] Do not use fallback criterion for random/shortest CURA-12324 When using random and shortest seam strategy, we don't care at all about the seam being aligned vertically, thus skip the fallback criterion which have a tendency of placing the seam at a certain position, which in case of random is really unexpected. --- include/PathOrderOptimizer.h | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/include/PathOrderOptimizer.h b/include/PathOrderOptimizer.h index a2e917fae8..2d339e79f8 100644 --- a/include/PathOrderOptimizer.h +++ b/include/PathOrderOptimizer.h @@ -784,17 +784,20 @@ class PathOrderOptimizer best_candidate_finder.appendCriteriaPass(main_criteria_pass); // ########## Step 2: add fallback passes for criteria with very similar scores (e.g. corner on a cylinder) - const AABB path_bounding_box(points); + if (path.seam_config_.type_ == EZSeamType::SHARPEST_CORNER) + { + const AABB path_bounding_box(points); - { // First fallback strategy is to take points on the back-most position - auto fallback_criterion = std::make_shared(points, path_bounding_box.max_, DistanceScoringCriterion::DistanceType::YOnly); - constexpr double outsider_delta_threshold = 0.01; - best_candidate_finder.appendSingleCriterionPass(fallback_criterion, outsider_delta_threshold); - } + { // First fallback strategy is to take points on the back-most position + auto fallback_criterion = std::make_shared(points, path_bounding_box.max_, DistanceScoringCriterion::DistanceType::YOnly); + constexpr double outsider_delta_threshold = 0.01; + best_candidate_finder.appendSingleCriterionPass(fallback_criterion, outsider_delta_threshold); + } - { // Second fallback strategy, in case we still have multiple points that are aligned on Y (e.g. cube), take the right-most point - auto fallback_criterion = std::make_shared(points, path_bounding_box.max_, DistanceScoringCriterion::DistanceType::XOnly); - best_candidate_finder.appendSingleCriterionPass(fallback_criterion); + { // Second fallback strategy, in case we still have multiple points that are aligned on Y (e.g. cube), take the right-most point + auto fallback_criterion = std::make_shared(points, path_bounding_box.max_, DistanceScoringCriterion::DistanceType::XOnly); + best_candidate_finder.appendSingleCriterionPass(fallback_criterion); + } } // ########## Step 3: apply the criteria to find the vertex with the best global score From 6b20571fe08811e8f000bc48d023297ee1f901b6 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Fri, 22 Nov 2024 09:06:30 +0100 Subject: [PATCH 2/4] Fix wrong scoring tendancy CURA-12324 When processing the last criterion pass, we used to keep all the best candidates in regard to the scoring threshold, then the first one of them would be returned, which is not always the one with the actual best score. Now we just early-out with the best candidate in case of the last pass. This is also more efficient. --- include/utils/scoring/BestElementFinder.h | 3 ++- src/utils/scoring/BestElementFinder.cpp | 29 ++++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/include/utils/scoring/BestElementFinder.h b/include/utils/scoring/BestElementFinder.h index e2ede975d5..07daf055d2 100644 --- a/include/utils/scoring/BestElementFinder.h +++ b/include/utils/scoring/BestElementFinder.h @@ -15,7 +15,7 @@ class ScoringCriterion; /*! * This class implements an algorithm to find an element amongst a list, regarding one or multiple scoring criteria. The - * criteria are implemented by subclassing the CriterionScoring class. It is also possible to setup multiple passes of + * criteria are implemented by subclassing the CriterionScoring class. It is also possible to set up multiple passes of * criteria. Thus, if the first pass gives a few best candidates that are too close to each other, we keep only the best * candidates and process a second pass with different criteria, and so on until we have a single outsider, or no more * criteria. @@ -60,6 +60,7 @@ class BestElementFinder * Once we have calculated the global scores of each element for this pass, we calculate the score difference * between the best candidate and the following ones. If the following ones have a score close enough to the * best, within this threshold, they will also be considered outsiders and will be run for the next pass. + * This value will be ignored for the last pass. */ double outsider_delta_threshold{ 0.0 }; }; diff --git a/src/utils/scoring/BestElementFinder.cpp b/src/utils/scoring/BestElementFinder.cpp index ecd63e89cb..1b116f62b7 100644 --- a/src/utils/scoring/BestElementFinder.cpp +++ b/src/utils/scoring/BestElementFinder.cpp @@ -5,6 +5,8 @@ #include +#include + #include "utils/scoring/ScoringCriterion.h" namespace cura @@ -33,11 +35,11 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand const auto begin = best_candidates.begin(); auto end = best_candidates.end(); - // Now run the criteria passes until we have a single outsider or no more cirteria - for (const CriteriaPass& criteria_pass : criteria_) + // Now run the criteria passes until we have a single outsider or no more criteria + for (const auto& [pass_index, criteria_pass] : criteria_ | ranges::views::enumerate) { // For each element, reset score, process each criterion and apply weights to get the global score - double best_score = 0.0; + auto best_candidate_iterator = end; for (auto iterator = begin; iterator != end; ++iterator) { iterator->score = 0.0; @@ -47,7 +49,22 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand iterator->score += weighed_criterion.criterion->computeScore(iterator->candidate_index) * weighed_criterion.weight; } - best_score = std::max(best_score, iterator->score); + if (best_candidate_iterator == end || iterator->score > best_candidate_iterator->score) + { + best_candidate_iterator = iterator; + } + } + + if (best_candidate_iterator == end) + { + // Something got wrong, we have not best candidate + return std::nullopt; + } + + // Early out for last pass, just keep the best candidate + if (pass_index == criteria_.size() - 1) + { + return best_candidate_iterator->candidate_index; } // Skip candidates that have a score too far from the actual best one @@ -55,9 +72,9 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand end = std::remove_if( begin, end, - [&best_score, &delta_threshold](const Candidate& candidate) + [&best_candidate_iterator, &delta_threshold](const Candidate& candidate) { - return best_score - candidate.score > delta_threshold; + return best_candidate_iterator->score - candidate.score > delta_threshold; }); if (std::distance(begin, end) == 1) From 0abecdc2e61595302f10471c271ef676ab2b0154 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Thu, 28 Nov 2024 10:35:38 +0100 Subject: [PATCH 3/4] Fix comment CURA-12324 --- src/utils/scoring/BestElementFinder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/scoring/BestElementFinder.cpp b/src/utils/scoring/BestElementFinder.cpp index 1b116f62b7..c0f79cd042 100644 --- a/src/utils/scoring/BestElementFinder.cpp +++ b/src/utils/scoring/BestElementFinder.cpp @@ -57,7 +57,7 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand if (best_candidate_iterator == end) { - // Something got wrong, we have not best candidate + // Something went wrong, we don't have a best candidate return std::nullopt; } From bb2b6810fcbb1078b8a6b21575f21c9ef031be56 Mon Sep 17 00:00:00 2001 From: Casper Lamboo Date: Thu, 28 Nov 2024 11:55:10 +0100 Subject: [PATCH 4/4] Update npm-package.yml --- .github/workflows/npm-package.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/npm-package.yml b/.github/workflows/npm-package.yml index 239334ff4d..7285375380 100644 --- a/.github/workflows/npm-package.yml +++ b/.github/workflows/npm-package.yml @@ -86,6 +86,7 @@ jobs: - name: Set npm config run: | + cd CuraEngineJS npm run install_curaengine npm ci npm publish @@ -97,4 +98,4 @@ jobs: run: | conan remove "cura_private_data/*" --force conan remove "fdm_materials/*" --force - conan upload "*" -r cura --all -c \ No newline at end of file + conan upload "*" -r cura --all -c