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
Implement Color Operations for Color
#13285
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I am starting to really like the enum approach. I think this is going to be very expressive for our users who are able to work with Color
rather than a specific space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the end results here: it provides a great balance between correctness, performance and usability.
@bushrat011899 you may need to tweak this to account for the removal of ClampColor. |
Head branch was pushed to by a user without write access
2d73b3c
to
c140f9f
Compare
Delegates to internal type when possible, otherwise uses `ChosenColorSpace` as an intermediary. This _will_ double convert, but this is considered an acceptable compromise since use of specific colour types in performance critical colour operations is already encouraged. `ChosenColorSpace` is `Oklcha` since it's perceptually uniform while supporting all required operations, and in my opinion is the "best" for this task. Using different spaces for different operations will make documenting this double-conversion behaviour more challenging. Call out explicitly why `Oklcha` is used for operations requiring conversion.
c140f9f
to
f2e9c13
Compare
@alice-i-cecile should be good to go now! |
Objective
Color
type #13214Solution
Delegates to internal type when possible, otherwise uses
ChosenColorSpace
as an intermediary. This will double convert, but this is considered an acceptable compromise since use of specific colour types in performance critical colour operations is already encouraged.ChosenColorSpace
isOklcha
since it's perceptually uniform while supporting all required operations, and in my opinion is the "best" for this task. Using different spaces for different operations will make documenting this double-conversion behaviour more challenging.Testing
Changes straightforward enough to not require testing beyond current CI in my opinion.
Changelog
Color
:Luminance
Hue
Mix
EuclideanDistance
ClampColor
Color
explaining the behaviour of these operations (possible conversion, etc.)