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

Reimplement arc interpolation by yielding new InterpolationPoints #1097

Merged
merged 4 commits into from
Aug 22, 2020

Conversation

IgorYeremin
Copy link
Contributor

As suggested in #1092
Very ugly diff because of indentation, use &w=1 in github url or --ignore-space-change in git diff

@IgorYeremin IgorYeremin marked this pull request as ready for review August 21, 2020 16:07
@kliment
Copy link
Owner

kliment commented Aug 21, 2020

@volconst looks like this was your suggestion, and looks good to me. What do you think?

avg_move_normal_y = move_normal_y
for point in interpolate_arcs(gline, prev_gline):
current_pos = (point.current_x, point.current_y, point.current_z)
if not gline.extruding:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not understand why bother to define InperpolationPoint to immediately convert it to tuple current_pos on the next line instead of just returning tuple.

@volconst
Copy link
Collaborator

@kliment It's ok to commit.

@volconst
Copy link
Collaborator

Looks great, thank you, @IgorYeremin

@kliment kliment merged commit b515721 into kliment:master Aug 22, 2020
@kliment
Copy link
Owner

kliment commented Aug 22, 2020

Merged, thanks @IgorYeremin @volconst

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.

3 participants