Add functions to get min/max component values for all color types, alpha #173
Conversation
|
I'm working on a project where I want to generate random Lab colors but possibly other spaces in the future. Hwb
fn is_valid(&self) -> bool {
self.blackness >= T::zero() && self.blackness <= T::one() &&
self.whiteness >= T::zero() && self.whiteness <= T::one() &&
self.whiteness + self.blackness <= T::one()
}Technically LchFor
I chose 128 as the maximum value to be returned even though it can actually be up to ~181. However, the values in the fully saturated range are well out of space for Srgb. |
|
Yeah, it's hard to define what's really the lower and upper bounds in some cases. It's a questionable concept for hues, as they are periodic, and even stranger for HWB, as you noticed. Most of the spaces have tricky detail, really, but it's usually fine to ignore them and/or clamp the final output. Considering that this is based on your project's use case, would you mind posting the code for generating the colors, for the discussion? I'm wondering if having associated constants like Also, for Lch, I'm not sure what to do for the chroma. In case you haven't looked into the details, Lab is basically a cube, while Lch is a cylinder. With chroma=128, the cylinder will reach the sides of the Lab cube, but not its corners. Issues like with Lab and Hwb is why I'm leaning towards looking into options. If random generation was built in, we could just suggest using Lab when Lch doesn't generate the desired results, without defining a definite chroma limit. Besides, generating cylindrical colors without taking the clumping in the center into consideration may generate too many grayish samples. And Hwb could be generated in a correct and uniform way from the start. I don't know if I'm over thinking it, but I think random generation is something that could be provided as an optional feature. |
|
Yes, the main point is that there are a lot of details to take into account across the various models and I wanted to reduce some of the cognitive load for users (and myself). I think there are some places for improvement in reducing possible confusion or at least providing direction. The documentation is there but it's not obvious what the implication of each unit or scaling is for color type values. Here's a simple example of a color I'm generating from a range using let lab = Lab::new(
rng.gen_range(0.0, 100.0),
rng.gen_range(-128.0, 127.0),
rng.gen_range(-128.0, 127.0),
);
// With the PR, I imagined making random colors like this but still feels clunky
let min = RGB.get_lower_bounds();
let max = RGB.get_upper_bounds();
let rgb = Rgb::new(
rng.gen_range(min.red, max.red),
rng.gen_range(min.green, max.green),
rng.gen_range(min.blue, max.blue),
);Something like a Another case recently of using the bounds was adjusting the luma of two colors until they had an adequate contrast ratio. I was able to keep the colors close enough to their original values while getting something that had good contrast for text and a background color by adjusting each proportionally 1/4 of the way toward black/white. I'm open to any options and also the idea of |
|
Sounds like the associated constants would fulfill your immediate needs. It's effectively the same thing, but without the additional implications of being valid in combination. But I'm not sure how well they work with type parameters, now that I think about it. They may need to be static methods instead (i.e. The I'll create an issue, but feel free to add any use case you stumble upon. It's always useful. |
|
This is a more conservative approach. I didn't provide any min or max for Hues or Hwb at all. I couldn't originally figure out Xyz but then I did. I tried to mark the functions as The build failed on the no_std_test initially. For some reason the Float trait is dropped or ambiguous? If I add a error[E0599]: no method named `sqrt` found for type `f64` in the current scope
--> palette/src/:53
|
133 | from_f64((127.0f64 * 127.0 + 128.0 * 128.0).sqrt())
| ^^^^ method not found in `f64`
|
= help: items from traits can only be used if the trait is in scope
= note: the following traits are implemented but not in scope; perhaps add a `use` for one of them:
candidate #1: `use num_traits::float::Float;`
candidate #2: `use num_traits::real::Real;`
error: aborting due to previous errorI fixed it with // fails
from_f64((127.0f64 * 127.0 + 128.0 * 128.0).sqrt())
// passes
from_f64(crate::float::Float::sqrt(127.0f64 * 127.0 + 128.0 * 128.0)) |
|
I readded HWB and Alpha. I removed HWB because I wasn't sure about it in general. You need whiteness to get the max blackness and you need blackness to get the max whiteness. The straight answer is that they both max out at 1 but it's the only model with dependent components in the library. I wasn't sure whether to implement for Alpha now that it wasn't part of a trait. I also wasn't sure whether the max should be |
|
5 builds passed but the Mac stable errored out as unresponsive. They were having issues recently with Mac builds https://www.traviscistatus.com/incidents/9861dc7dtsb0?u=nfsz8t7c7m1q I noticed many errors in the past few commits with 5b11613 a few hours ago being the last one where all Mac branches built. |
okaneco
changed the title
get_lower_bound and get_upper_bound to Limited
|
I restarted the Mac build. Looks like it's working this time.
Understandable. I think it's less weird now when they are not part of a complete color value. The value on its own doesn't have any other meaning than "this is the maximum value of this particular channel".
Yes, it should be |
palette/src/lch.rs
Outdated
| /// Return the `chroma` value maximum. This value will suffice for most | ||
| /// use cases. | ||
| pub fn max_chroma() -> T { | ||
| from_f64(128.0) | ||
| } | ||
|
|
||
| /// Return the `chroma` extended maximum value. | ||
| pub fn max_extended_chroma() -> T { | ||
| from_f64(crate::float::Float::sqrt(127.0f64 * 127.0 + 128.0 * 128.0)) | ||
| } |
There was a problem hiding this comment.
The documentation should probably describe the geometric reason for picking one or the other. I don't have any good suggestion off the top of my head right now, but something about how the first one doesn't cover the full space, while still covering enough to be useful, and the second one covering the full space, but also overshoothing.
Oh, and I forgot before, but the square root thingy should maybe use 128 all the way. That way it should reach the negative corner. even if it extends outside all other corners. Once again, this format is weird, but it's the expected one.
|
Never mind, the Mac build gave up. We can try again later. |
Add functions for Alpha, Hsl, Hsv, Hwb, Lab, Lch, Luma, Rgb, Yxy, Xyz Add max_extended_chroma for Lch in addition to max_chroma
|
I've updated the documentation for max_chroma/extended, hopefully it will suffice but I can change with any feedback. Travis is in maintenance but should be back up soon. https://www.traviscistatus.com/incidents/blv0v9hd1flk?u=f362jrtv8qb6 |
|
Looks like they may have fixed the issues now, so time to merge. Thanks for this addition! bors r+ |
Build succeeded |
Add functions to retrieve the maximum and minimum component values for all color types and alpha.
There are no functions for min/max values of Hue components since they are periodic and range from 0 to 360 degrees, 360 degrees not inclusive.
Lch and HWB are special cases:
chromais fully saturated starting at 128 but continues to 181, so two max chroma functions are implemented.