-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
brim ears #4691
brim ears #4691
Conversation
Any thoughts on how to test this? |
Seems to me that the best way is to actually print something that's normally guaranteed to curl off of the bed, in ABS on untreated glass, playing with the brim settings to see the effect |
@VanessaE I mean for the code generation. Maybe inspecting the data structures? |
✅ Build Slic3r 1.3.0-master-2244 completed (commit 7aad202975 by @) |
You got me....... |
I forgot: you should check the tooltips & text. I'm not a good english speaker. |
✅ Build Slic3r 1.3.0-master-2245 completed (commit 4967b29cb9 by @) |
//create ear pattern | ||
coord_t size_ear = (scale_(this->config.brim_width.value) - flow.scaled_spacing()); | ||
Polygon point_round; | ||
point_round.points.push_back(Point(size_ear*1, 0*size_ear)); |
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.
This can be replaced with array of x,y coefficients ...
Also maybe angle + sin/cos may be used to generate the ear?
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.
yes. I used that because it's quick and easy to write & test. Also we don't need a big precision and this number of point is enough. Too much and the intersection takes ages.
What code do you propose?
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.
something like (untested):
for (int i = 0; i < POLY_SIDES; i++) {
float angle = 2 * PI / POLY_SIDES * i;
point_round.points.push_back(Point(size_ear * cos(angle), size_ear * sin(angle));
}
lines.erase(lines.begin() + best_idx); | ||
|
||
//connect if near enough | ||
if (lines_sorted.size() > 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.
this is always true
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.
Also, with some more tough, the if (previous == NULL) block should be put before the while.
//connect if near enough | ||
if (lines_sorted.size() > 1) { | ||
size_t idx = lines_sorted.size() - 2; | ||
bool connect = false; |
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 possible to check best_dist
first and skip testing distance again.
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.
?
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.
Closest distance between lines is already available in best_dist
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.
but i need to know where to do reverse(). So i need to store more information than just best_dist...
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.
Yes, the tests will be necessary. But testing best_dist can skip the tests altogether.
It's just minor optimization ...
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.
oh, i understand
double best_dist = -1; | ||
size_t best_idx = 0; | ||
while (lines.size() > 0) { | ||
if (previous == NULL) { |
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.
(this can be moved before while
, reducing nesting)
#4687