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

Projects
None yet
2 participants
@byronwasti
Contributor

byronwasti commented Apr 5, 2018

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

fixes #11

@jamwaffles

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

Owner

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 jamwaffles:master Apr 5, 2018

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