halostatue / color

Color tools for Ruby.
Other
133 stars 41 forks source link

Adds closest_match functions for color matching among RGB colors. #5

Closed armahillo closed 10 years ago

armahillo commented 10 years ago

Given an RGB object instantiation, you can pass an array of other RGB objects to the "closest_match" method, and it will identify the which of the colors is the closest match, based on L_a_b* analysis color distance. (This uses the CIE94 formula. There is a newer CIEDE2000 formula, but it uses some math functions I'm not sure how to do in Ruby. CIE94 is pretty solid.)

I had to add a to_lab function to handle the initial conversion. It's pretty basic, just returning a Hash (keys: :L, :a, :b) with the calculated values.

There is test coverage for all created methods.

halostatue commented 10 years ago

Looks mostly good; a few problems—and some of this will probably need to be clarified in the Contributing file.

  1. Please rebase your commit and drop the .gemspec modification. If you need to modify dependencies, that belongs in the Rakefile. There's also a spurious reference to a color_match.rb file that I can't find anywhere.
  2. The version of color.rb you've targeted still supports Ruby 1.8 and you've used Ruby 1.9-only Hash styles. I have to consider whether dropping Ruby 1.8 support is worth it for this particular release (much the same as I did with mime-types).
  3. There are lots of magic numbers in this—please provide better documentation to the functions, possibly with links to where the functions were found from in order to help people learn how this works (including myself ;).

I think we can get this pulled in with those things fixed.

armahillo commented 10 years ago

@halostatue Definitely! Will do that tonight. 1) No dependency changes necessary -- Sorry, I must have overlooked that part in the contributing doc. :) 2) I'll go ahead and convert to Hash-rocket syntax, no problem 3) Would it be ok to cite a URL that explains the formula in more depth? I've drawn from a number of sources that have the algebra all laid out with a great explanation. I would be happy to provide more documentation for the steps in the calculations though.

halostatue commented 10 years ago

Re: 1) I don't think it's in the contributing doc, but it should be. It's probably something I need to fix.

Re: 3) URLs are perfectly acceptable for reference material—I've done that with a lot of the methods that do other calculations. I would like better documentation on the methods on how they are used. The only method name I'm not 100% sold on after the quick glance I did last night is the bang version where you calculate the Just Noticeable Difference—color_match vs color_match! doesn't really scream that difference to me. I don't have a solution yet, and I probably won't have a solution until later tonight…

armahillo commented 10 years ago

OK I just modified things and pushed up the changes. Please see commit #194e6a7

armahillo commented 10 years ago

Is there anything else you need me to do to conform to the coding standards for this gem, or am I all set here?

halostatue commented 10 years ago

Sorry I didn't get back to you—been a little busy. I think it's much better; the only thing I think I miscommunicated is that I wasn't suggesting that #closest_match! needed to be deleted, only renamed—the idea of a closest match that defaults to a Just Noticeable Difference isn't a bad one, but the name didn't feel right.

My (artless) suggestion would be something like #just_noticeable_match.

armahillo commented 10 years ago

Not a problem -- I was just following up.

I got rid of closest_match! because it really didn't make sense to have a separate method that acted so similarly, even if it was a bang. No offense or anything taken there -- it was a good suggestion. :)

halostatue commented 10 years ago

Fair enough. I probably won't be able to make this real for a couple of weeks, but I won't forget it.

armahillo commented 10 years ago

OK sounds good. That Travis CI is wicked handy!

halostatue commented 10 years ago

I needed to make tweaks to your code, but it has been merged at ce6013b. Thanks for your contribution.

Nakilon commented 2 years ago

Wikipedia says:

There are a number of color distance formulae that attempt to use color spaces like HSV with the hue as a circle, placing the various colors within a three-dimensional space of either a cylinder or cone, but most of these are just modifications of RGB; without accounting for differences in human color perception, they will tend to be on par with a simple Euclidean metric.

How can the author call it "just a modification of RGB" without even clarifying the shape of the space. In my opinion people hate HSV unreasonably. So far the bicone Euclidean works for me well:

https://github.com/Nakilon/colorutils/blob/f8727837f58669610931008eb18a9fd2695da13b/lib/colorutils.rb#L18-L26

halostatue commented 2 years ago

Hi, @Nakilon. I’m sort of coming back to this codebase to start working on an updated release and would love to have some additional contributors who are more familiar with the colour math involved. This would be definitely focused on a new release with modern support (Ruby 2.7 or later explicitly supported; anything else would be accidental).

  1. Would you be interested in contributing to this update (possibly the utils functions you have already implemented)?
  2. Would you be interested in reviewing the codebase overall, because I think that the more that I look at this, the current API and calculations may be subtly wrong.

Eventually, I am looking to transition development to someone who deals with colour calculations and manipulation on a regular basis. That hasn’t been me since 2008 or so, when I handed off PDF::Writer.