sharkdp / pastel

A command-line tool to generate, analyze, convert and manipulate colors
Apache License 2.0
5k stars 97 forks source link

Extract the distinct colors code into a reusable function. #92

Closed joshhansen closed 4 years ago

joshhansen commented 4 years ago

I want to use pastel's algorithm for finding a set of distinct colors in an application I'm writing, but found there was no convenient interface for this without re-implementing the algorithm myself. I moved the distinct colors annealing algorithm from the relevant cli command module into the distinct module at the root:

pub fn distinct_colors(
    count: usize,
    distance_metric: DistanceMetric,
    verbose: bool,
    arrange_colors: bool
) -> Result<(Vec<Color>, DistanceResult)> { ... }

It returns the vec of colors (optionally rearranged) as well as the DistanceResult struct. The cli now calls this function.

Please also note that this required moving the error module up to the root so distinct could access it.

sharkdp commented 4 years ago

Thank you very much for your contribution.

I agree it would be nice if the "distinct colors" feature would be usable more easily from pastel the library. However, I don't want to move all of the printing, debugging and error handling code into the library.

What exactly are you missing from the library? Probably mostly the "recipe" on how long to run the simulated annealing?

rivy commented 4 years ago

I just saw this in passing and created a small refactor.

Would rivy/rust.pastel@a62a7326 be a satisfactory solution?

joshhansen commented 4 years ago

@sharkdp Mostly I don't want to implement the algorithm myself. I was about to copy-paste a bunch of code and then thought, why don't we refactor this and make this already-useful algorithm more easily accessible. I could have copied your distinct code but why not provide an off-the-shelf solution so people don't have to do that?

@rivy I haven't tried it yet but at first glance that looks good to me. One thing I am also wanting but hadn't mentioned yet (and have been working on myself) is the ability to provide a few already-known colors which the algorithm needs to work around. For the game I'm working on, I want to provide the algorithm with the color of the background tiles (green for land, blue for ocean, etc.) and have the algorithm automatically choose distinct colors for the different players which will be different from the background colors, so the units remain easy to see. That's mostly a matter of allowing certain elements in the colors vector to be held fixed while the others vary around them. I'll see if I can get your code adapted to that purpose soon.

sharkdp commented 4 years ago

@joshhansen as for the latter point, see #88

sharkdp commented 4 years ago

@rivy Yes, that looks better to me. Happy to review it in detail if you want to send a PR

rivy commented 4 years ago

No response for @joshhansen so far ... so, I'll create an alternate PR.

joshhansen commented 4 years ago

@rivy What is the question?

88 does look like what I was talking about for allowing fixed colors, btw.

rivy commented 4 years ago

@joshhansen , I wasn't sure if you'd want to modify your PR based on the refactor that I presented.

So, I went ahead and created a fully-fleshed out alternate PR (#95).

joshhansen commented 4 years ago

@rivy I apologize I didn't communicate it better, but I felt your PR did a better job isolating the core functionality whereas mine still had some hard-coded logging outputs mixed up in it. And as your PR also appears to address my desire for invariant colors, I fully support it and will now close this one as being obsolete. Thank you for your efforts!