-
-
Notifications
You must be signed in to change notification settings - Fork 913
CURA-12833 improve bridge lines direction #2266
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
Conversation
… res CURA-12833 Very effective, but extremely slow...
…idge-lines-direction
To fix compiling on mac CURA-12833
CURA-12361 We now negatively account for the line parts that are fully supported, because in some cases the angle would generate a few lonely lines on supported areas.
CURA-12833 This makes sure the bridging lines will anchor as deep as possible inside the mesh, instead of sometimes generating a tiny concentric area on the borders.
| case BridgeStatus::Supported: | ||
| bridge_status = leaving_skin ? BridgeStatus::Outside : BridgeStatus::Anchored; | ||
| // Negatively account for fully supported lines to avoid lonely line parts over the supported areas | ||
| add_segment_score_weight = -0.1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -0.1 value seems a bit arbitrary (I even thought it might be a typo for -1.0 at first; until I took a closer look at what it replaced and why you might want to use a double at all...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is very much arbitrary indeed... The previous version was mostly working, with an equivalent score of 0, compared to scores 1.0 and -1.0 for other lines. But on one edge case I had to add a negative value for this type of line. I tried with -1.0, which worked but felt a bit wrong. -0.1 also worked and feels less harmful. If there are other annoying edge cases, we can stil adjust it further, but so far all the tested cases work as expected.
rburema
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Reviewed new changes only.) LGTM!
This PR contains a few independent changes:
writeSomethingmethods, but instead a consistent API for any geometric type and user-provided attributes to display surface, outlines and or/vertices at will.CURA-12833
Requires Ultimaker/Cura#21163