Add hwb color

[sidred]

Added support for HWB color model.

Added tests, implemented Alpha, ApproxEq, FromColor and IntoColor traits and added to color enum.

closes #32

[sidred]

Did not implement Saturate for Hwb. Is it needed?

[Ogeon]

Ah, nice! I'll look at this later, when my brain has received some nourishment.

Did not implement Saturate for Hwb. Is it needed?

It shouldn't if it can't be done in a trivial way. I'm not exactly sure in this case, but maybe it's enough to scale both the B and W components.

[Ogeon]

It's probably a good idea to have the sum check here, as well.

[Ogeon]

I think that was all. We can leave out Saturate for now, since it's not completely trivial.

[sidred]

I just derived the saturate formula from hsv as

hue_new = hue
w_new = w - (factor * ( 1 -w -b))
b_new = b

This will exactly match the saturation in Hsv from space.

However it does not match the saturation from Hsl space

       // matches hwb_input.saturate(amt)
        hwb_input.into_hsv().saturate(amt).into_hwb();

       // does not match hwb_input.saturate(amt)
       hwb_input.into_hsl().saturate(amt).into_hwb();

        // These saturation in Hsl and Hsv space do not match either.
        let hsv1 = hwb_input.into_hsv().saturate(amt);
        let hsv2 = hwb_input.into_hsl().saturate(amt).into_hsv();

Not sure if saturation should be equivalent in the HSL and HSV spaces.

Should I implement this saturation?

[Ogeon]

HSL and HSV are affected by the difference between lightness and value. Saturate is currently only meant as a common interface for types where saturation/chroma is clearly and trivially defined and manipulated. HWB is a bit hazy, since both whiteness and blackness affects the saturation, depending on how you see it. That's why I'm not sure if it should even implement the trait at all, and it's probably better to convert the color to a space where the saturation has a better definition.

[sidred]

Just wanted to confirm that the clamp_self check for 0 to infinity makes sense right? For all other colors we have checked 0-1, but here we are normalizing.

+    fn clamp_self(&mut self) {
+        self.whiteness = clamp(self.whiteness, T::zero(), T::infinity());
+        self.blackness = clamp(self.blackness, T::zero(), T::infinity());
+        let sum = self.blackness + self.whiteness;
+        if sum > T::one() {
+            self.whiteness = self.whiteness / sum;
+            self.blackness = self.blackness / sum;
+        }
+    }

[Ogeon]

Yes, the normalization makes it ok. You can't fool it by making one parameter negative, since it would be clamped to 0. I just realized that clamp is unnecessary, since this is equal to x.max(0).

[sidred]

I updated Limited to use max fn instead of clamp.

Should be good to go!

[Ogeon]

Great! I'll merge as soon as the tests are green. Thank you!

[Ogeon]

@homu r+

[homu]

📌 Commit 5cee7ab has been approved by Ogeon

[homu]

⚡ Test exempted - status