{"action":"created","issue":{"url":"https://api.github.com/repos/Ogeon/palette/issues/58","repository_url":"https://api.github.com/repos/Ogeon/palette","labels_url":"https://api.github.com/repos/Ogeon/palette/issues/58/labels{/name}","comments_url":"https://api.github.com/repos/Ogeon/palette/issues/58/comments","events_url":"https://api.github.com/repos/Ogeon/palette/issues/58/events","html_url":"https://github.com/Ogeon/palette/issues/58","id":138692457,"number":58,"title":"Redesign the pixel encoding procedure","user":{"login":"Ogeon","id":225229,"avatar_url":"https://avatars.githubusercontent.com/u/225229?v=3","gravatar_id":"","url":"https://api.github.com/users/Ogeon","html_url":"https://github.com/Ogeon","followers_url":"https://api.github.com/users/Ogeon/followers","following_url":"https://api.github.com/users/Ogeon/following{/other_user}","gists_url":"https://api.github.com/users/Ogeon/gists{/gist_id}","starred_url":"https://api.github.com/users/Ogeon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Ogeon/subscriptions","organizations_url":"https://api.github.com/users/Ogeon/orgs","repos_url":"https://api.github.com/users/Ogeon/repos","events_url":"https://api.github.com/users/Ogeon/events{/privacy}","received_events_url":"https://api.github.com/users/Ogeon/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/Ogeon/palette/milestones/3","html_url":"https://github.com/Ogeon/palette/milestones/0.3.0","labels_url":"https://api.github.com/repos/Ogeon/palette/milestones/3/labels","id":1554098,"number":3,"title":"0.3.0","description":"","creator":{"login":"Ogeon","id":225229,"avatar_url":"https://avatars.githubusercontent.com/u/225229?v=3","gravatar_id":"","url":"https://api.github.com/users/Ogeon","html_url":"https://github.com/Ogeon","followers_url":"https://api.github.com/users/Ogeon/followers","following_url":"https://api.github.com/users/Ogeon/following{/other_user}","gists_url":"https://api.github.com/users/Ogeon/gists{/gist_id}","starred_url":"https://api.github.com/users/Ogeon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Ogeon/subscriptions","organizations_url":"https://api.github.com/users/Ogeon/orgs","repos_url":"https://api.github.com/users/Ogeon/repos","events_url":"https://api.github.com/users/Ogeon/events{/privacy}","received_events_url":"https://api.github.com/users/Ogeon/received_events","type":"User","site_admin":false},"open_issues":5,"closed_issues":1,"state":"open","created_at":"2016-01-31T19:03:59Z","updated_at":"2016-03-05T15:59:46Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2016-03-05T15:46:36Z","updated_at":"2016-03-05T17:59:48Z","closed_at":null,"body":"Something that has been growing in my mind for a few days is the idea of changing the way colors are encoded into pixel friendly formats. The current procedure may look like this:\r\n\r\n```rust\r\nlet linear = Rgb::new(1.0, 0.3, 0.6);\r\nlet srgb = Srgb::from_linear(linear);\r\nlet pixel: [u8; 3] = srgb.to_pixel();\r\n```\r\n\r\nor, using a shortcut:\r\n\r\n```rust\r\nlet linear = Rgb::new(1.0, 0.3, 0.6);\r\nlet pixel: [u8; 3] = Srgb::linear_to_pixel(linear);\r\n```\r\n\r\nThere's also the case where a simple custom gamma exponent can be used:\r\n\r\n\r\n```rust\r\nlet linear = Rgb::new(1.0, 0.3, 0.6);\r\nlet pixel: [u8; 3] = GammaRgb::linear_to_pixel(linear, 2.2);\r\n```\r\n\r\nThis is fine and all, but it's not really easy to use in a general way:\r\n\r\n 1. There's no common trait or type for encoded colors.\r\n 2. The alpha component is baked into the color type.\r\n 3. `GammaRgb` has a variable gamma exponent, which makes it very different from other encodings, and impossible to fully express as a simple type.\r\n\r\nThis problem is tightly connected to #31, where a general RGB type is proposed. The way `GammaRgb` is structures interferes with that proposal, since it has an additional `gamma` component, besides the usual `red`, `green` and `blue`. I'm therefore proposing that we redesign the way pixel encoding works to make it more friendly towards the idea of a more generalized RGB type. I would also like to extend this to other RGB related color spaces, such as HSV, HSL, Luma and HWB. These can all be seen as input types and they are, often enough, defined in terms of already encoded RGB.\r\n\r\nThe initial proposal is to keep linear and encoded types separate. The purpose of this library is to strongly encourage a linear work flow and this can easily be achieved by only implementing operations on the linear variant. There was a proposal, by @sidred, to have marker type parameters, such as `Rgb<Linear, Srgb, ...>` and `Rgb<Gamma, Srgb, ...>`, but [recent problems](https://github.com/Ogeon/palette/pull/56#issuecomment-189837611) with such marker parameters, as well as the additional complexity of extra type parameters, makes that method less attractive.\r\n\r\nThe separation of linear and encoded colors, using two different structs, will come with a bit of duplication, but the duplication will mainly be restricted to construction and fields. The linear variants will implement the operation traits, while the encoded variants will be focused on packing and converting data. Here is an example of how it may look when combined with #31:\r\n\r\n```rust\r\npub trait RgbEncoding {\r\n    ///Encode a color component.\r\n    fn encode<T: Float>(value: T) -> T;\r\n\r\n    ///Decode a color component.\r\n    fn decode<T: Float>(value: T) -> T;\r\n}\r\n\r\npub trait RgbPrimaries {\r\n    //get red, green, blue primaries, etc.\r\n}\r\n\r\n///Marker type, which both defines a set of primaries and an encoding\r\npub struct Srgb;\r\n\r\nimpl  RgbEncoding for Srgb {\r\n    //...\r\n}\r\n\r\nimpl  RgbPrimaries for Srgb {\r\n    //...\r\n}\r\n\r\nstruct LinearRgb<Prim: RgbPrimaries = Srgb, ...> {\r\n    //..\r\n    primaries: PhantomType<Prim>,\r\n}\r\n\r\nstruct Rgb<Enc: RgbEncoding = Srgb, ...> {\r\n    //..\r\n    primaries: PhantomType<Enc>,\r\n}\r\n```\r\n\r\nEncodings and primaries are often strongly connected to each other, which is why the same type can be used for both. The problem with this is that it's hard to decouple them, but that's hopefully rare enough to make this more good than bad. The only problem left is the `GammaRgb`. Its variable gamma component makes it incompatible with the above pattern, and having floating point values as type parameters is nothing but a (bad?) dream at this stage. We can, however, make more assumptions and add more escape hatches. I'm therefore proposing the addition of the probably most common gamma exponent, 2.2, as an encoding type, and the extension of the above `RgbEncoding` that makes it possible for an encoding to map to any set of primaries:\r\n\r\n```rust\r\npub trait RgbEncoding {\r\n    type Primaries: RgbPrimaries;\r\n\r\n    ///Encode a color component.\r\n    fn encode<T: Float>(value: T) -> T;\r\n\r\n    ///Decode a color component.\r\n    fn decode<T: Float>(value: T) -> T;\r\n}\r\n\r\n///Gamma exponent 2.2, with any primaries.\r\nstruct Gamma22<P: RgbPrimaries>;\r\n\r\nimpl<P: RgbPrimaries> RgbEncoding for Primaries<P>\r\n    type Primaries = P;\r\n\r\n    //...\r\n}\r\n\r\nimpl  RgbEncoding for Srgb {\r\n    type Primaries = Srgb;\r\n    //...\r\n}\r\n\r\nfn main() {\r\n    let encoded = Rgb::<Gamma22<Srgb>>::new(...);\r\n    let linear = encoded.to_linear(); //Gives LinearRgb<Srgb>\r\n}\r\n```\r\n\r\nThe preferred primaries can then be fetched from the associated `Primaries` type when converting from encoded to linear. Now, what about those other gamma values? That's where we need escape hatches. One such escape hatch could be to keep the old `GammaRgb` type around. It may be a bit unwieldy, but it works. An other is to just have a straight-to-pixel conversion, like the `*::linear_to_pixel` function in the current design. More suggestions are welcome here.\r\n\r\nThe last question is what encoded types are allowed to do. My proposal is \"basically nothing\", since I'm still strongly in favor of keeping them strictly separated from linear types. This is to minimize any risk of mistaking non-linear values for linear and forcing the user to make an explicit conversion between them. The minimal set of functions I can think of at the moment is:\r\n\r\n```rust\r\nimpl<E: RgbEncoding, ...> Rgb<E, ...> {\r\n    fn new(red: T, green: T, blue: T) -> Rgb<E, ...> {...}\r\n    fn new_u8(red: u8, green: u8, blue: u8) -> Rgb<E, ...> {...}\r\n    fn from_pixel<P: RgbPixel<T>>(pixel: P) -> Rgb<E, ...> {...}\r\n    fn to_pixel<P: RgbPixel<T>>(&self) -> P {...}\r\n    fn from_linear<C: Into<LinearRgb<E::Primaries, ...>>(color: C) -> Rgb<E, ...> {...}\r\n    fn to_linear<C: From<LinearRgb<E::Primaries, ...>>(&self) -> C {...}\r\n\r\n    //Useful addition\r\n    fn linear_to_pixel<C: Into<LinearRgb<E::Primaries, ...>, P: RgbPixel<T>>(color: C) -> P {\r\n        Rgb<E, ...>::from_linear(color).to_pixel()\r\n    }\r\n}\r\n```\r\n\r\nThese can, of course, be tweaked and changed. Some of these functions would be nice as a general encoding trait, but that's quite complex and may be better discussed at a later stage. \r\n\r\nSomething this proposal hasn't touched, so far, is what to do with the `alpha` component. I think it would be best to make use of the `Alpha` type, but I haven't figured out exactly how, so that question remains open.\r\n\r\nAn other open question is type aliases for common RGB types, such as AdobeRgb. One idea is to not alias sRGB, and only implement `new` for `Rgb<Srgb, D65, T>`, similar to how it's done with only `D65` today. An other is to alias all of them and not pick a default."},"comment":{"url":"https://api.github.com/repos/Ogeon/palette/issues/comments/192699098","html_url":"https://github.com/Ogeon/palette/issues/58#issuecomment-192699098","issue_url":"https://api.github.com/repos/Ogeon/palette/issues/58","id":192699098,"user":{"login":"sidred","id":6623285,"avatar_url":"https://avatars.githubusercontent.com/u/6623285?v=3","gravatar_id":"","url":"https://api.github.com/users/sidred","html_url":"https://github.com/sidred","followers_url":"https://api.github.com/users/sidred/followers","following_url":"https://api.github.com/users/sidred/following{/other_user}","gists_url":"https://api.github.com/users/sidred/gists{/gist_id}","starred_url":"https://api.github.com/users/sidred/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sidred/subscriptions","organizations_url":"https://api.github.com/users/sidred/orgs","repos_url":"https://api.github.com/users/sidred/repos","events_url":"https://api.github.com/users/sidred/events{/privacy}","received_events_url":"https://api.github.com/users/sidred/received_events","type":"User","site_admin":false},"created_at":"2016-03-05T17:59:48Z","updated_at":"2016-03-05T17:59:48Z","body":">  Into_linear shouldn't even be implemented for LinearRgb.\r\n\r\nI was confusing the trait implementation with the struct implementation\r\n\r\nSo we can just have 2 traits. Having fn gamma() on the Gamma trait will make GammaRgb redundant\r\n```\r\npub trait Gamma {\r\n    fn gamma() -> T;\r\n\r\n    ///Encode a color component.\r\n    fn to_gamma<T: Float>(value: T) -> T {\r\n      let gamma = Self::gamma();\r\n      let inv_gamma = if gamma.is_normal() { T::one() / gamma } else { T::one()}\r\n      value.powf(inv_gamma)\r\n   }\r\n\r\n    ///Decode a color component.\r\n    fn to_linear<T: Float>(value: T) -> T {\r\n      let gamma = Self::gamma();\r\n      value.powf(inv_gamma)\r\n   }\r\n}\r\n\r\npub trait RgbPrimaries {\r\n    //get red, green, blue primaries, etc.\r\n}\r\n\r\n```\r\nHaving an impl like this will take care of the standard converison linear gamma conversions.\r\n```\r\nimpl<R: Gamma + RgbPrimaries> LinearRgb<R> {\r\n    to_linear() {\r\n     R::to_linear() // will take care of the default Srgb, Adbode Rgb conversions from the traits\r\n  }\r\n}\r\n```\r\n"}}