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

Ranges for CIE color spaces #10

Closed
sidred opened this issue Jan 13, 2016 · 7 comments
Closed

Ranges for CIE color spaces #10

sidred opened this issue Jan 13, 2016 · 7 comments
Milestone

Comments

@sidred
Copy link
Contributor

sidred commented Jan 13, 2016

For XYZ, both http://www.easyrgb.com/ and http://www.brucelindbloom.com/index.html?ColorCalculator.html show z as 1.088830 for srgb(1,1,1).

The is_valid() method for xyz assumes a range of [0-1].

For Lab, both http://www.easyrgb.com/ and http://www.brucelindbloom.com/index.html?ColorCalculator.html have values above 1 and also negative. For example, srgb(1,0.5,1) -> lab(72.087 ,65.181, -42.228)

The is_valid() method for lab assumes a range of [0-1].

Not sure what the correct range is. Opening this issue for tracking this.

@sidred sidred changed the title Ranges for XYZ color space Ranges for CIE color spaces Jan 13, 2016
@Ogeon
Copy link
Owner

Ogeon commented Jan 13, 2016

Thank you for pointing this out. Lab has normally a range of L: 0 - 100, a: -127 - 128, b: -127 - 128, as I understand it, which is adapted to an 8 bit storage format. I have chosen to normalize these to 0 - 1 for L and -1 - 1 for a and b. The reason for this is the same as in Rgb, to make calculations (especially multiplication) easier. Other common ranges for these are 0 - 100 and -100 - 100, respectively.

The Xyz situation, on the other hand, is probably a mistake from my side. These should be corrected in the best possible way.

@Ogeon
Copy link
Owner

Ogeon commented Jan 13, 2016

This answer on mathematica.stackexchange.com suggests that the ranges of XYZ depends on the white point. Maybe we should just remove the upper limits for now.

@sidred
Copy link
Contributor Author

sidred commented Jan 13, 2016

Since the struct values are all public, we should try to use existing conventions where possible. This will ensure less surprises for the end users of the library. I'll try to look at ImageMagik, OpenCV and GIMP sources to see what they use.

Can you explain how normalizing makes multiplication easier? I am not sure I understand what you are trying to say.

@Ogeon
Copy link
Owner

Ogeon commented Jan 13, 2016

I have to admit that multiplying two L_a_b* values is not so intuitive, compared to RGB, so it may not matter as much. The general thought can be illustrated with light and reflection in RGB space. If you have white light (rgb(1, 1, 1)) that should be reflected from a red surface (rgb(1, 0, 0)) you want the resulting light to also be red. Having an upper limit >1 makes this operation require additional computations to scale the resulting color down by the white point.

An other reason for keeping as many of the color spaces' limits within 0-1 was to also avoid having different limits for every color space. This may not be as useful as I thought, because of the nature of some color spaces like XYZ. The question is, though, what the best limits for L_a_b* are. 0 - 100 and -100 - 100 seem to be the "official" limits, while 0 - 100 and -127 - 128 are common for storage purposes.

I couldn't find out what ImageMagick does, but it looks like OpenCV rescales each component to 0 - 255 and GIMP doesn't seem to have official support, but the decomposition tool seems to do the same as OpenCV.

@sidred
Copy link
Contributor Author

sidred commented Jan 14, 2016

I have found an official document for the color spaces CIE Technical Report Colorimetry 3rd Edition.

I will use the transformations and ranges from this for LAB, LCH, XYZ and LUV color spaces. No normalization.

XYZ will have a range of [0 - 100 (approx) ] and not [0 - 1 (approx) ] as before.

Lab will have [0 - 128] range.

@Ogeon
Copy link
Owner

Ogeon commented Jan 14, 2016

XYZ is based on measurements, so those values may actually be "real", but the values of L_a_b* are "artificial", just like in RGB or HSV, so I see no reason why we can't normalize them to keep it consistent with the rest of the color spaces. I would gladly accept power-of-two based values if we were working with u8, but these are floating point calculations, so I prefer to use the ranges [0%, 100%] and [-100%, 100%], but normalized as [0, 1] and [-1, 1], for as much as possible. The only exceptions I intended to make was for angular measurements and I'm also willing to do it with XYZ if those makes more sense than the normalized values.

@Ogeon
Copy link
Owner

Ogeon commented Jan 22, 2016

I have been thinking about repurposing the ColorSpace trait as a Limited trait, since that's basically what it is. Some color spaces, like XYZ, doesn't seem to have a well defined upper limit, so these should maybe not be treated in such a way.

@Ogeon Ogeon added this to the 0.2.0 milestone Jan 25, 2016
homu added a commit that referenced this issue Jan 28, 2016
Fix or relax some color ranges and clamping

The `ColorSpace` trait has been repurposed as a trait for checking limits and clamping, and is now called `Limited`. The limits of `Xyz` has been changed to use the white point as its maximum (closing #10), and the maximum of `Lch.chroma` has been removed, since it's not very well defined. The ranges of `Lch` stays as they are, for the sake of consistency.

Extra clamping is also added when converting RGB colors to `u8` based pixel representation, to prevent float shenanigans when multiplying values with 255.0. It's impossible to know what a type `T` will do when it's asked to convert `255.12312312` or whatever to `u8`. This closes #19.

This is a breaking change, since it changes the ranges of some color spaces and renames the `ColorSpace` trait to `Limited`.
@Ogeon Ogeon closed this as completed Jan 28, 2016
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

No branches or pull requests

2 participants