Generalize the RGB types over RGB standards #60

Ogeon · 2016-03-10T21:54:32Z

This changes the Rgb type to have a type parameter that implements the RgbStandard trait. This trait defines a set of primaries, white point and transfer function. It does also make the RGB types more uniform and type safe.

Closes #66, closes #31, closes #58

Ogeon · 2016-03-10T22:00:02Z

This is a beginning. It doesn't compile, yet, because of problems with the definition of IntoColor and FromColor. These have to be redefined and I'm too tired right now to come up with a solution. The problem is that the RgbSpace needs to be known to convert to Rgb, but the whole trait can't depend on it (like in the current implementation), because then it won't be known when converting between non-RGB types, like XYZ -> Lab.

I'll write a better description later, but I decided to bundle the primaries and white point in the RgbSpace trait, and bundle it, together with a transfer function, as RgbStandard.

Ogeon · 2016-03-10T22:50:34Z

...thinking about it, something like this may work: fn into_rgb<S: RgbSpace<WhitePoint=Wp>>(self) -> Rgb<S, T>. The macros will be... fun.

homu · 2016-03-11T11:53:42Z

☔ The latest upstream changes (presumably #61) made this pull request unmergeable. Please resolve the merge conflicts.

sidred · 2016-03-11T12:39:26Z

src/pixel/mod.rs

@@ -10,6 +10,15 @@ pub use self::gamma_rgb::GammaRgb;
 mod srgb;
 mod gamma_rgb;

+///A transfer function to and from linear space.
+pub trait TransferFn {


It will be good to add rgb_to_linear and rgb_from linear with default implementation, once the linear and gamma rgb are defined.

To TransferFn or just to the pixel module?

To the TransferFn. Hsl to LinearHsl, should convert though rgb. So that is the only conversion TransferFn should care about.

It can also apply to Luma/Grey.

We need a trait to convert gamma encoded colors to linear colors (and the inverse).

let rgb = linear_rgb.from_linear() currently does not work. If we do not want to add another trait, this is where these conversions will go for every type that can convert to and from linear.

Or we will need something similar to the From and Into traits that will leverage the transfer function to do the linear <-> non-linear conversions.

It's a bit hard to generalize that in the TransferFn trait, since there are other parameters involved. There's currently the Srgb::from_linear(color) method, which should contain all the necessary information to convert anything to Srgb.

let rgb = linear_rgb.from_linear(), which I guess is the same as let rgb = linear_rgb.into_nonlinear(), will always need extra information about the transfer function. That can't be changed by changing the TransferFn trait.

Ogeon · 2016-03-11T13:54:16Z

Looks like the workaround works for now. The only problem is the occasional detour through XYZ when going from Rgb<A, T> to Rgb<A, T>. I don't think we can specialize for that.

Ogeon · 2016-03-11T19:10:53Z

pixel::Srgb is now almost completely replaced. There are still some missing conversion implementations, but they are on their way. I just have to figure how to do it in a good way. Possibly another From vomiting macro.

Anyway, it looks like it fits right into the previous model, with a few small adjustments.

Ogeon · 2016-03-12T14:30:03Z

I ended up implementing a few conversions manually and modify the macros for the rest. It's now possible to freely convert Rgb and linear types using From and Into.

The next phase will probably be the integration of the other RGB related types. I'm just not sure how they should be presented to the user. Just as plain Hsl<S, T> or with accompanying type aliases, like HslSrgb? Adding aliases doesn't hurt, except for the clutter factor, but it's relatively low if they are limited to the Srgb variants.

Ogeon · 2016-03-13T15:36:59Z

The previously mentioned round trip when converting from LinRgb<A, T> to LinRgb<A, T> caused infinite recursion when Hsv joined the club. The problem was that Hsv<A, T> -> LinRgb<A, T> would be treated as Hsv<B, T> -> LinRgb<A, T>, which expands to (Hsv<B, T> -> LinRgb<B, T> -> LinRgb<A, T> -> Hsv<A, T>) -> LinRgb<A, T>, which keeps exploding.

The solution I'm using to break the recursion is to check if A::Primaries == B::Primaries, by comparing their type IDs, and not do any conversion if they are the same. We already know that their white points are the same, since that's a requirement, so they should represent the same RGB space, but possibly with different actual type (e.x. Srgb == (Srgb, D65)). I expect this check to be optimized away after monomorphisation, and would otherwise most likely be cheaper than always going through XYZ, so I would say it's a win in any case.

sidred · 2016-03-15T03:50:24Z

src/rgb/mod.rs

+pub type LinSrgba<T = f32> = LinRgba<standards::Srgb, T>;
+
+///An RGB space and a transfer function.
+pub trait RgbStandard {


Should the RgbStandard have the WhitePoint type as well? The use case I see for it is supporting non standard white points like ICC Srgb D50 <SrgbTranfFn, SrgbSpace, D50>.

That's still possible, since RgbStandard is implemented for (Srgb, D50, Srgb).

Ogeon · 2017-08-08T11:56:18Z

Hmm... this looks like it's close to being complete, judging from the diff. Just need to

fix the conflict,
~~see if the primaries/whitepoint relation is something that requires changes~~,
~~find a better name than with_wp~~,
check how the docs looks,
unify Rgb and LinRgb,
unify Rgb and GammaRgb.

Ogeon · 2017-09-01T21:29:40Z

I have been making an attempt to unify Rgb and LinRgb, trying to get rid of both LinRgb and GammaRgb in the end, but I'm afraid the Color type stands in my way. Either that or I'm too tired and confused to realize why things don't work as they should. I'll see if I can make some progress later. Otherwise I'm saving it for another time.

Ogeon · 2017-09-02T15:55:37Z

Seems like it's going to work, after all, so I'm adding some points to the checklist.

Ogeon · 2018-02-17T14:48:25Z

Ok, I think I'll consider this done, at last. I skipped a couple of points from the list and decided to push them into the future.

Ogeon · 2018-02-17T14:48:40Z

bors r+

60: [WIP] Generalize the RGB types over RGB standards r=Ogeon a=Ogeon This changes the `Rgb` type to have a type parameter that implements the `RgbStandard` trait. This trait defines a set of primaries, white point and transfer function. It does also make the RGB types more uniform and type safe. Closes #66, closes #31, closes #58

bors · 2018-02-17T15:04:40Z

Build succeeded

continuous-integration/travis-ci/push

sidred reviewed Mar 11, 2016
View reviewed changes

Ogeon force-pushed the rgb_reform branch from 01bbd73 to ffde5ec Compare March 11, 2016 13:44

Ogeon force-pushed the rgb_reform branch from c1240be to 50b813a Compare March 13, 2016 17:25

sidred reviewed Mar 15, 2016
View reviewed changes

Ogeon mentioned this pull request Nov 15, 2016

HSL in sRGB space #66

Closed

Ogeon mentioned this pull request Aug 8, 2017

Project Status? #71

Closed

Ogeon added this to the 0.3.0 milestone Aug 8, 2017

Ogeon force-pushed the rgb_reform branch from 50b813a to 732ea60 Compare September 1, 2017 19:07

Ogeon added 6 commits February 17, 2018 13:40

Make rgb mod public and add RGB definition traits

1398ef8

Convert Rgb to use RgbSpace instead of WhitePoint

71ed349

Rename Rgb and Rgba to LinRgb and LinRgba

a17bf4f

Replace pixel::Srgb with rgb::Rgb<Srgb, _>

ca23a58

Implement conversion to and from nonlinear RGB

8873740

Use RGB space instead of white point in Hsv

2f74f6c

Ogeon added 4 commits February 17, 2018 13:55

Use RGB space instead of white point in Hsl

4376027

Use RGB space instead of white point in Hwb

c1bb094

Unify linear and nonlinear RGB

afe5d2c

Unify GammaRgb and Rgb

ce9eb1d

Ogeon force-pushed the rgb_reform branch from 7679341 to ce9eb1d Compare February 17, 2018 13:01

Ogeon added 2 commits February 17, 2018 14:52

Break up RGB standards into smaller modules

e843419

Some fixes

127fc1a

Ogeon changed the title ~~[WIP] Generalize the RGB types over RGB standards~~ Generalize the RGB types over RGB standards Feb 17, 2018

bors bot merged commit 127fc1a into master Feb 17, 2018

bors bot deleted the rgb_reform branch February 17, 2018 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize the RGB types over RGB standards #60

Generalize the RGB types over RGB standards #60

Ogeon commented Mar 10, 2016 •

edited

Loading

Ogeon commented Mar 10, 2016

Ogeon commented Mar 10, 2016

homu commented Mar 11, 2016

sidred Mar 11, 2016

Ogeon Mar 11, 2016

sidred Mar 11, 2016

Ogeon Mar 11, 2016

sidred Mar 11, 2016

Ogeon Mar 11, 2016

Ogeon commented Mar 11, 2016

Ogeon commented Mar 11, 2016

Ogeon commented Mar 12, 2016

Ogeon commented Mar 13, 2016

sidred Mar 15, 2016

Ogeon Mar 15, 2016

Ogeon commented Aug 8, 2017 •

edited

Loading

Ogeon commented Sep 1, 2017

Ogeon commented Sep 2, 2017 •

edited

Loading

Ogeon commented Feb 17, 2018

Ogeon commented Feb 17, 2018

bors bot commented Feb 17, 2018

Generalize the RGB types over RGB standards #60

Generalize the RGB types over RGB standards #60

Conversation

Ogeon commented Mar 10, 2016 • edited Loading

Ogeon commented Mar 10, 2016

Ogeon commented Mar 10, 2016

homu commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ogeon commented Mar 11, 2016

Ogeon commented Mar 11, 2016

Ogeon commented Mar 12, 2016

Ogeon commented Mar 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ogeon commented Aug 8, 2017 • edited Loading

Ogeon commented Sep 1, 2017

Ogeon commented Sep 2, 2017 • edited Loading

Ogeon commented Feb 17, 2018

Ogeon commented Feb 17, 2018

bors bot commented Feb 17, 2018

Build succeeded

Ogeon commented Mar 10, 2016 •

edited

Loading

Ogeon commented Aug 8, 2017 •

edited

Loading

Ogeon commented Sep 2, 2017 •

edited

Loading