parasyte / pixels

A tiny hardware-accelerated pixel frame buffer. 🦀
https://docs.rs/pixels
MIT License
1.82k stars 123 forks source link

Replace `line_drawing` with `clipline` #381

Closed nxsaken closed 1 year ago

nxsaken commented 1 year ago

Replaced line_drawing with my clipline crate which implements a variation of Bresenham's algorithm with built-in pixel-perfect clipping.

parasyte commented 1 year ago

Does this crate offer better performance or something? It has no dependencies, which might be enough of an improvement to make this change worth it.

But I would strongly prefer an external iterator interface over the callback. I have written a little about why in another issue. There really isn't any reason to not let the caller write the loop. And enforcing an internal iteration is a form of inversion of control which I am firmly against on principle.

If you switch it to external iteration, I don't see any reason not to make the change. Assuming it doesn't cause performance regressions or anything.

nxsaken commented 1 year ago

Yes, this crate clips the line before the tight loop (in constant time), allowing you to skip the bounds checks when indexing into the pixel buffer. It also skips Bresenham's algorithm for vertical and horizontal lines. I haven't had experience writing non-trivial Iterators, so I avoided doing it for this crate - but I'll look into it. Thank you for sharing the discussion and the talk!

nxsaken commented 1 year ago

I implemented the iterator(s) and updated the examples. You can either iterate over the Clipline enum itself, or match on it and move the loop(s) inside its arms, which will remove the overhead of the match from every iteration. I decided not to do that to keep the examples concise.

parasyte commented 1 year ago

That looks very good! Thank you for making that change. I know it's a lot to ask, but I also believe it is a marked improvement for usability.

I'll let the CI run, and it should inform you if there is anything that needs to be addressed.

nxsaken commented 1 year ago

Thank you for your feedback, that was valuable! It's my first crate, so I'm extra grateful for the opportunity to learn :)