search

Smithay / andrew

Convenient drawing of objects such as shapes, lines and text to buffers
MIT License
17 stars 7 forks source link

Implement line_drawing functionality inside of andrew #6

Closed chrisduerr closed 3 years ago

chrisduerr commented 3 years ago

I've recently sent a PR to the line_drawing crate (https://github.com/expenses/line_drawing/pull/17) about a week ago, which has made it pretty clear that the maintenance of it is insufficient. Since that introduces a fair bit of risk both now and in the future, which already shows up as duplicate dependencies in Alacritty, I'd propose to replace its use with a reimplementation in andrew.

Since these algorithms are so trivial, I think a cleanroom reimplementation shouldn't be a problem, if we want to circumvent having to include licensing notes. Of course the MIT license would allow us to just copy/paste the code into andrew and vendor it, but with how trivial this code is I feel like losing licensing control is likely not worth it. It should be possible to just completely remove line_drawing and all its dependencies, since there's no reason why we would need num_traits here as far as I can tell.

I'm curious how you're feeling about this @trimental, are you open to the idea of vendoring this code and would you prefer reimplemenation?

trimental commented 3 years ago

@chrisduerr I’m feeling that a clean reimplementation might be the right way to go. If andrews going to be drawing decorations it might as well do it’s job of reducing dependencies given line drawing is pretty simplistic. I’ll look into the algorithms used and try to come up with a reimplementation of it soon.

Matthias-Fauconneau commented 3 years ago

https://github.com/Matthias-Fauconneau/combustion/blob/master/src/plot.rs#L3-L40 might be of interest as a starting point for a reimplementation. Originally ported (via C++) from the algorithm in Wikipedia's "Xiaolin Wu's line algorithm" (which it is not). It is really just pixel stepping in one coordinate while tracking the other (a.k.a "Digital differential analyzer" 😱). Feel free to use this without any restrictions.

I could have used andrew/line_drawing but it doesn't handle the ends coverage correctly which I need to plot very dense polylines without any stippling errors.

chrisduerr commented 3 years ago

https://github.com/Matthias-Fauconneau/combustion/blob/master/src/plot.rs#L3-L40 might be of interest as a starting point for a reimplementation.

I'd recommend just looking at the wikipedia article tbh. That avoids potential licensing issues and it's really simple either way. In fact by the formatting of that link creates enough of an overhead that I wouldn't bother with trying to immitate that.

trimental commented 3 years ago

@chrisduerr https://github.com/Smithay/andrew/commit/312c78b8542dae5cff86e9a7f49c54cc066a2ce2 this should solve this problem. The re-implementation seems to be working well enough, thanks wikipedia! Might consider finally donating. Since nothing was changed api wise I've released it as 3.1 to keep dependencies down.

chrisduerr commented 3 years ago

Thanks for handling this so quickly, much appreciated!