Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EuclideanDistance trait #288

Closed
Ogeon opened this issue Jul 12, 2022 Discussed in #275 · 5 comments · Fixed by #316
Closed

Add EuclideanDistance trait #288

Ogeon opened this issue Jul 12, 2022 Discussed in #275 · 5 comments · Fixed by #316

Comments

@Ogeon
Copy link
Owner

Ogeon commented Jul 12, 2022

Discussed in #275

Originally posted by okaneco March 6, 2022
I find myself rewriting boilerplate often enough that maybe it'd be nice to have upstream. The ColorDifference trait exists but I don't always need the expensive CIEDE2000 calculation. Sometimes I'd like a more simple Euclidean distance/norm or the squared distance for comparing to a tolerance value.

This is a sketch of what I had in mind.

fn distance_squared(&self, other: &Self) -> T {
    (self.red - other.red).powi(2)
        + (self.green - other.green).powi(2)
        + (self.blue - other.blue).powi(2)
}

fn distance(&self, other: &Self) -> T {
    ((self.red - other.red).powi(2)
        + (self.green - other.green).powi(2)
        + (self.blue - other.blue).powi(2))
    .sqrt()
}

Perhaps this can be added to the ColorDifference trait?

There may be other "general enough" functions that people find themselves re-implementing similar to these which would carry their weight as additions to the library.

@okaneco
Copy link
Contributor

okaneco commented Jul 13, 2022

I tried working on this a bit but ran into a stumbling block for cylindrical color spaces. How should the various RGB Hue spaces be treated, LcH, etc.? Would their implementations of the trait be omitted, would they be converted to the Cartesian coordinates (RGB, Lab, etc.), or something else? There may be other exceptions but this question made me put it on hold and forget about it until this issue.

I think it'd be nice to get a better idea of what to do for all the color spaces to make it easier for implementing.

@Ogeon
Copy link
Owner Author

Ogeon commented Jul 13, 2022

Thanks a lot for giving this a try! I think it makes sense to simply skip anything with polar coordinates. At least for Hsv, Hsl and Hwb that map weirdly to Rgb. I prefer to draw the line where it becomes subjective or stops being well defined. I have had a though of making Cartesian versions of them too, since it's useful for interpolating nicely, and those could implement this trait. I have nothing concrete in mind but maybe something like Cartesian<PolarColorType, Shape> or CartesianHsv<Shape> (etc.) that allows it to be interpreted as cylindrical, conical or bi-conical.

Some of the other spaces do have clear Cartesian counterparts (Lch and Lab are 1:1 the same space in that sense) and could be converted. Maybe that's fine after all, since it will behave in a much more expected way than those who don't map 1:1 to anything.

@Ogeon
Copy link
Owner Author

Ogeon commented Apr 23, 2023

I'm implementing this as part of some general color difference improvements in #316. Let me know if there's anything I missed that you were about to add, if you want.

@okaneco
Copy link
Contributor

okaneco commented Apr 23, 2023

Thanks so much for working on this, looks great. I think that covers most use cases I had in mind.

Since then, I came across this paper where the HyAB (hybrid model of |delta L*| + Euclidean(a*, b*)) color distance metric is presented in Equation 3 on page 4. It seems like it could be a good addition to Lab because it separates the effect of luma and chroma/hue in color difference calculation while remaining computationally simple. Unfortunately, the Lch Manhattan distance and hybrid models require calculating the expensive factors in the DE2000 calculation and perform slightly worse overall than HyAB, so they probably wouldn't be used as much and seem like less of a priority.
http://markfairchild.org/PDFs/PAP40.pdf

@Ogeon
Copy link
Owner Author

Ogeon commented Apr 23, 2023

Interesting! It looks like HyAB would be a simple and good addition. I'll skip it in the current PR, though, just to not rush it in. Would you like to open an issue for it? Otherwise, I'll do it and link to your comment. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants