JuliaAttic / Color.jl

Basic color manipulation utilities.
Other
47 stars 21 forks source link

Added code for additional color difference formulae #28

Closed glennsweeney closed 10 years ago

glennsweeney commented 10 years ago

A whole slew of color difference formulae for those who just can't be satisfied with Delta E 2000. ;)

Some have industry applications; some are antique but still used. I will try to look up sources and include them soon.

You'll notice that colordiff survived untouched; it is just an alias for colordiff_2000 now. See line 84.

BFD has a feature to allow giving a white point for the LAB->XYZ and XYZ->LAB transform that happens internally. I left it undocumented to avoid confusion.

StefanKarpinski commented 10 years ago

Any time I see a bunch of functions with really similar names, I get pretty suspicious that this should be done with dispatch instead. Perhaps by providing color difference types and then having the color difference type as an optional argument to colordiff?

StefanKarpinski commented 10 years ago

Having color difference objects also naturally provides a place for parameters to the difference computation to live, which is another indication that it's a better way to express this.

glennsweeney commented 10 years ago

Ugh. I'm still a C programmer at heart, sorry. I'll see what I can do for that; thanks for the input.

Any suggestions on what the objects could look like to be more coherent with the Julia mentality?

StefanKarpinski commented 10 years ago

No worries – C is the desert island programming language, after all :-)

They should be something like this:

abstract ColorMetric

immutable CIEDE2000 <: ColorMetric end

immutable DeltaE94 <: ColorMetric
  kl::Int
  kc::Int
  kh::Int
  DeltaE94(; kl::Integer=1, kc::Integer=1, kh::Integer=1) = new(kl, kc, kh)
end

immutable JPC79 <: ColorMetric
...

Later you would do something like this:

colordiff(ai::ColorValue, bi::ColorValue, m::ColorMetric) = colordiff(ai, bi, CIEDE2000())

function colordiff(ai::ColorValue, bi::ColorValue, m::CIEDE2000)
  ...
end

function colordiff(ai::ColorValue, bi::ColorValue, m::DeltaE94)
  # use m.kl, m.kc, m.kh values as parameters
  ...
end

Those parameter names could probably be better since they're now externally exposed.

StefanKarpinski commented 10 years ago

Any progress? Does that approach make sense?

glennsweeney commented 10 years ago

Yes, it does make sense! Sorry, it's approaching finals week - things are gonna move at a snail's pace for me for a bit. I'll try to get a look at this and put the changes in soon as I can.

StefanKarpinski commented 10 years ago

No worries, just thought I'd ask.

glennsweeney commented 10 years ago

Here's an updated version. let me know what you think. I also took the liberty of deleting a .swp file that had snuck into the repository with 0d7e39fb5ee930931ed8dff53ca45ac234ef6089. Left it as a separate commit for transparency.

StefanKarpinski commented 10 years ago

Looks good to me but @dcjones should take a look and merge if it's good.

dcjones commented 10 years ago

Nice work @glenn-sweeney.

dcjones commented 10 years ago

The one issue I noticed is this:

julia> colordiff(color("red"), color("blue"), DE_BFD())
ERROR: no method convert(Type{XYZ}, RGB, XYZ)
 in colordiff at /Users/dcjones/.julia/v0.3/Color/src/differences.jl:298

That's just exposing an existing issue though. We have a white point parameter when converting from e.g. LAB to XYZ, but not from RGB to XYZ. So if it makes sense, we should add the same parameter to every convert(::Type{XYZ}, ...) function.

glennsweeney commented 10 years ago

I've been noticing some problems as well; for instance, there is no conversion with a white point from XYZ to LCHab or similar. If we want to provide this support, it'll just mean adding white point control through the entire conversion function set. Special care will need to be taken with examples like RGB - right now we only support sRGB, which uses the D65 white point only.

On Sun, Jun 1, 2014 at 12:29 PM, Daniel Jones notifications@github.com wrote:

The one issue I noticed is this:

julia> colordiff(color("red"), color("blue"), DE_BFD())ERROR: no method convert(Type{XYZ}, RGB, XYZ) in colordiff at /Users/dcjones/.julia/v0.3/Color/src/differences.jl:298

That's just exposing an existing issue though. We have a white point parameter when converting from e.g. LAB to XYZ, but not from RGB to XYZ. So if it makes sense, we should add the same parameter to every convert(::Type{XYZ}, ...) function.

— Reply to this email directly or view it on GitHub https://github.com/JuliaLang/Color.jl/pull/28#issuecomment-44781849.

dcjones commented 10 years ago

Yeah, a more general RGB implementation would be good. I'd be best if we can do that without forcing people to care about the white point, etc, and default to sRGB.

glennsweeney commented 10 years ago

It might be worth introducing a type like conversionparams or something that can house relevant customized conversion information, such as a white point, and provide it as an optional parameter to any conversion. That way "longer" conversions through multiple steps could have access to necessary custom data. For instance, if you were converting from LMS to LCHab, it could provide white point information to the intermediate XYZ->LAB step.

This allows a bit of "futureproofing" as we accumulate more metadata needed for certain steps, but when not provided, could always revert to defaults. It could also become the backbone of a general RGB transformation; if nothing is specified, assume the sRGB tone function and D65 white point.

Just a thought, if there's a better way to structure this, I'm happy to consider that instead.