Implement [ok]lab, [ok]lch from color-4 spec. #309
Conversation
|
Removed the |
src/color.rs
Outdated
| /// The b-axis component. | ||
| pub b: f32, | ||
| /// The alpha component. | ||
| pub alpha: u8, |
There was a problem hiding this comment.
I think maybe storing the alpha as an f32 might be better long term (avoids having different computed / animated representations). Wdyt?
There was a problem hiding this comment.
That would make things easier yes. I think I'm going to do it with RGBA.alpha as well. Right now the size of the struct doesn't matter (we'll see if this is an issue) and dealing with alpha values the same would also makes things much simpler. Can you think of issues with this?
There was a problem hiding this comment.
The main issue is that it'd slow down conversion to nscolor (the Gecko representation of a computed rgba value which is ~everywhere). But might not be a huge issue.
There was a problem hiding this comment.
We'll have to convert to rgba (nscolor) anyway from lab/lch/etc. If this becomes a problem we can optimize.
src/color.rs
Outdated
| #[derive(Clone, Copy, PartialEq, Debug)] | ||
| pub enum AbsoluteColor { | ||
| /// Specify sRGB colors directly by their red/green/blue/alpha chanels. | ||
| RGBA(Rgba), |
There was a problem hiding this comment.
Actually I meant the variant names should be Rgba() etc (so I was expecting Rgba(RGBA), sorry if that wasn't clear). But Rgba(Rgba) is also alright.
There was a problem hiding this comment.
And now those are renamed too 😄
|
Note that I began a refactor on this same code in #308 that is still awaiting review. |
|
@bors-servo r+ |
|
📌 Commit 250e11d has been approved by |
|
(CI fix is in #310. Assuming it's green on top this looks good to me) |
|
☀️ Test successful - checks-github |
https://w3c.github.io/csswg-drafts/css-color-4/#lab-colors
The size of the new color structs are 32*4 bits which is much bigger than the original 32bit for the RGBA value. This might change in the future.