Repository navigation
Conversation
tomcur
commented
Jul 17, 2026
tomcur
left a comment
tomcur
left a comment
Member
Author
There was a problem hiding this comment.
CI is failing due to cargo rdme. #222 should fix that. (And see #chatter > cargo rdme update has CI impacts.)
Chromaticity plus a luminance encodes a color. Mathematically the most straightforward coming from `xy` chromaticity is going to XYZ. As all our color spaces are relative to some reference white, we choose to represent it as `XyzD65`, i.e., with a reference white of D65 (as that's the most common reference white). The docs note to use `ColorSpace::convert_absolute` to convert this as an absolute color to another color space.
tomcur
force-pushed
the
chromaticity-to-color
branch
from
July 17, 2026 14:16
88d714c to
604d17a
Compare
waywardmonkeys
left a comment
waywardmonkeys
left a comment
Collaborator
There was a problem hiding this comment.
A small matter of asking for 1-2 more tests...
| /// assert!(color.iter().all(|component| (component - 1.).abs() < 1e-4)); | ||
| /// ``` | ||
| #[inline] | ||
| pub const fn with_luminance(self, luminance: f32) -> OpaqueColor<XyzD65> { |
Collaborator
There was a problem hiding this comment.
+1 to this returning OpaqueColor<XyzD65>!
| 1e-4, | ||
| ), | ||
| "`Chromaticity::D50.with_luminance(1)` converted without chromatic adaptation to ProPhoto RGB should match `ProphotoRgb::WHITE_COMPONENTS` (which has a D50 reference white)" | ||
| ); |
Collaborator
There was a problem hiding this comment.
All of the tests are with a luminance of 1.0, so we should add a test that is different:
let chromaticity = Chromaticity { x: 0.25, y: 0.4 };
assert_eq!(
chromaticity.with_luminance(0.2).components,
[0.125, 0.2, 0.175],
);And maybe one with a luminance of zero.
waywardmonkeys
approved these changes
Jul 17, 2026
| /// # Example | ||
| /// | ||
| /// ```rust | ||
| /// # use color::{Chromaticity, ColorSpace, OpaqueColor, ProphotoRgb, XyzD65}; |
Collaborator
There was a problem hiding this comment.
The import of OpaqueColor here is not needed, I think.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chromaticity plus a luminance encodes a color. Mathematically the most straightforward coming from
xychromaticity is going to XYZ. As all our color spaces are relative to some reference white, this chooses to represent it asXyzD65, i.e., with a reference white of D65 (as that's the most common reference white).The docs note to use
ColorSpace::convert_absoluteto convert this as an absolute color to another color space.