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

Fix line algorithm implementation #13

Merged
merged 4 commits into from Apr 5, 2018

Conversation

@byronwasti
Copy link
Contributor

@byronwasti byronwasti commented Apr 5, 2018

This implements the Bresenham's line algorithm from the wikipedia page with some slight modifications.

fixes #11

Copy link
Member

@jamwaffles jamwaffles left a comment

Thanks for taking a crack at this! A couple of things:

  1. Can you rustfmt your code please?

  2. It would be good to have at least a primitive unit test for this code, just to catch any glaring problems. I wrote a crappy one for my fix which passes when pasted into this PR. It's not the greatest test, but it should hopefully catch any regressions in the future.

byronwasti added 3 commits Apr 5, 2018
@byronwasti
Copy link
Contributor Author

@byronwasti byronwasti commented Apr 5, 2018

I've added tests for each octant. I'm not sure if its the best way to test things, but it should catch regressions.

@jamwaffles
Copy link
Member

@jamwaffles jamwaffles commented Apr 5, 2018

Looks good! I like your approach to the testing, much better than mine as it actually checks the line. Thanks for fixing the lines!

@jamwaffles jamwaffles merged commit dbbd91b into embedded-graphics:master Apr 5, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants