Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CURA-12218 fix scattered seam for round models #2160

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

wawanbreton
Copy link
Contributor

The previous rework of the score calculation removed the relative comparison between vertices scores to ensure the consistency. The new strategy performed very badly on shapes where multiple points would get a similar score, e.g. finding sharp corners on a cylinder. We now process a multi-pass scoring, so that all points having a very similar score at their main criterion will now go through a second pass with different criteria.

CURA-12218

CURA-12218
The previous rework of the score calculation removed the relative
comparison between vertices scores to ensure the consistency. The new
strategy performed very badly on shapes where multiple points would get
a similar score, e.g. finding sharp corners on a cylinder. We now
process a multi-pass scoring, so that all points having a very similar
score at their main criterion will now go through a second pass with
different criteria.
CURA-12218

Instead of sorting the whole scores list, which is very costly, pre-
calculate the best score and only keep elements that are close enough to
it.
@wawanbreton wawanbreton marked this pull request as ready for review October 29, 2024 14:29
CURA-12218
In order to restore the previous seam consistency even better, we now
use two different fallback passes, the first one working on Y only, and
the second one on X only. This way it really mimics the former behavior
and always aligns the seam even on very symmetric shapes.
Copy link
Member

@rburema rburema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 71ac23e Previous: 32a279d Ratio
SimplifyTestFixture/simplify_local 8.660006123440803 ns/iter 1.2392275564797144 ns/iter 6.99

This comment was automatically generated by workflow using github-action-benchmark.

CC: @nallath @jellespijker @wawanbreton @casperlamboo @saumyaj3 @HellAholic

@HellAholic HellAholic merged commit fb27df7 into 5.9 Nov 5, 2024
27 checks passed
@HellAholic HellAholic deleted the CURA-12218_fix-scattered-seam-for-round-models branch November 5, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants