-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redesign the pixel encoding procedure #58
Comments
A few things Is it necessary to separate RgbEncoding and Rgb Primaries? They both are properties of the specific Rgb Color space. Example
In this case Srgb can override the default gamma conversion with a custom conversion and for all other colors we just need to provide the default gamma value.
What do primaries have to do with gamma encoding?
If we have the encoding and decoding on the specifc rgb color space, then there will be no need to have the GammaRgb type. The Rgb type above will hold the gamma encoded values and LinearRgb will hold the linear values. Am I missing some use case that you have in mind? I need to think a bit more about the pixel type. |
I'm trying to keep things open for customization, and dynamically chosen gamma exponent can't be represented as a type. That was the whole point with the
Very good question. Answer: Nothing, really. They are just paired together in an RGB standard, together with a white point, and may be anything. The |
One issue with having Gamma and Linear types separate is you cannot prevent mistakes like this
This can be avoided if the gamma or linear is part of the trait. |
|
I was confusing the trait implementation with the struct implementation So we can just have 2 traits. Having fn gamma() on the Gamma trait will make GammaRgb redundant
Having an impl like this will take care of the standard converison linear gamma conversions.
|
Why not implement Blend etc on the gamma encoded value by converting to Linear, atleast for the known types. What about from and into color? Do we add from_rgb_linear and assume existing from_rgb converts to the gamma value? |
That's basically what this proposal is all about, but without assuming exponential encoding, or tight coupling between encoding and primaries. It's common, but we don't have to assume it.
Does that convert from linear to linear?
Well, hmm... I suppose they could. They were mainly meant to be temporary states, but that doesn't stop them from jumping to and from being linear, just like with premultiplied alpha.
I'm not sure if they should be concerned by the encoded variants at all, but then I don't know what happens if we add things like non-linear HSV... |
Lets take an example of converting D50 to D65
That would be a lot easier than
Also for IntoColor and AdaptInto to work for both Rgb and LinearRgb, you would need a trait bound of So having a custom Gamma If the user has just a separate gamma, wouldn't it be easier to let users convert it into a known color directly or implement the entire RgbVariant trait? That way all other operations will work easily and will be a more explicit error. Do we need Hsl, LinearHsl, Hwb, LinearHwb, Hsv, LinearHsv as well? |
|
Would it have to? The white point doesn't change. |
Yeah, I guess these will need to be taken care of in the from_rgb and into_rgb by conversion via xyz . |
We need to rethink the conversions. Right now From and Into only handle different colors (same white point and rgb variant). Chromatic adaptation handles different whitepoints and uses From and Into for conversions. We need to handle |
Sorry, I somehow missed the other post. I'm not sure I see what the problem is. Separating primaries and encoding will simply separate two independent, but often coupled, concepts. One is part of the definition of the color space, and the other tells us how to store the values. The only thing they really have in common is that standardized encodings comes with an RGB space definition, including primaries, but any primaries can really be used with any encoding. I want to model this independence in a way that is as usable as possible, which is why I think that the concepts should be separated. The encoding doesn't have to have a preferred set of primaries. It could just as easily be replaced with yet another type parameter:
Separating encoding and primaries makes this even easier. Just implement the encoding trait and don't care about primaries.
Yes, eventually. They are tightly connected to RGB. Didn't the colormine tests assume that they converted to encoded RGB? |
I'm not sure this is entirely related to this particular topic, but yes, we need to decide the limits of those conversions. |
|
Again, there are more encoding methods than a gamma exponent. That's why I don't call it gamma, and that's also why I don't think adding a
Here are some more: http://www.color.org/chardata/rgb/rgb_registry.xalter Some of them are linear and some of them are a bit more exotic.
I'm not 100% sure I follow. I would gladly remove the pixel types, but I have seen them used as input types to functions, so I think they may still be useful in those situations. They are otherwise not particularly useful and their conversion functionality can be replaced by functions, as demonstrated by the existence of
Yes, but they will just have to follow the same pattern as for RGB. We can ignore them for now. The idea is to convert like this: Hsl -> EncodedRgb -> LinearRgb -> whatever. Luma is gray, which is why it's also affected.
Working with encoded colors will give the wrong result in most situations, except where the opposite is defined. The very description of Palette is "A Rust library for linear color calculations and conversion", and I would like to keep that focus. It's, by the way, very easy to ignore the the linearity restriction, by interpreting pixel values as if they were linear.
Conversion to and from linear should only be necessary during input/output. A color doesn't have to be listed in the trait to implement it, meaning that
I think only the linear versions are necessary, given that we only restrict the implementations of various operations to them. |
I am not disagreeing with this. I expect the Rgb encoding to have 3 methods
So this is just a basic implementation which avoids repetition for common use cases like AdobeRgb and AppleRgb. Just set the gamma and you're done. For Srgb etc. gamma() is not used and encode and decode method need to be overridden. This is just to simplify the implementation for a common use case.
Pixel type needs to be there. I was trying to understand the point you were making in the first comment regarding the pixel type. I wanted to know if you expect any major changes to the pixel type or will having the RgbEncoding trait cover most of the use case?
Yes we can implement from and into color for gamma rgb but it will be different from the rest of the colors. This works
This won't work
Adding into and from for the encoded colors makes sense from a usage point of view. |
But pub trait ExponentialEncoding {
pub fn gamma<T: Float>() -> T;
}
impl<E: ExponentialEncoding> RgbEnchoding for E {
//...
}
pub struct Gamma18;
impl ExponentialEnchoding for Gamma18 {
fn gamma<T: Float>() -> T {
flt(1.8)
}
}
type AppleRgb = Gamma18; ...modified for however the primaries are to be connected to the encoding.
I'm concerned about them becoming too monolithic, but I don't know... Maybe an |
I was playing around with the idea of having separate traits for This allows RGB standards that functions as both primary sets and encodings, loose encodings and primary sets, and on-the-fly custom standards using a Oh, and standards can also be derived from each other. That's useful for easy customization. |
I was just thinking something similar. Is it possible to take it one step further and include white point in it as well
We can then use this for all colors including the cie colors. For cie colors when the primaries/encodings are not needed, you can just use dummy values ( like SrgbEncidng, SrgbPrimaries etc) which will not be used any way. We can even define a WhitePoint<Dummy, Dummy, Wp> if it makes it any simpler for the cie colors. I think it will make the library a lot simpler. What's not clear to me is how it will play with the encoded colors. Do we include the float trait in it as well? One downside I see is that right now it is easier to see which trait is being used where. For example From and Into color use the Primaries Trait for Xyz <-> Rgb, but with the single trait it will be a bit harder to track the effects of a particular trait. |
Absolutely!
I'm not so sure about that, on the other hand... Any RGB specific info is totally irrelevant for the CIE types, unless they are converted to RGB, in which case the RGB type can provide them. I fear that they may become type checking noise, and behave badly because of
Yes, but not in the
Downside with my proof of concept or with the ubiquitous |
I was talking about the ColorProfile trait. The more I think about, the more I like the ColorProfile trait. I think for most users of the library this will be a lot simpler. You just choose a color profile and it keeps track through all conversions etc and will need fewer type annotations. For advanced as you have said it is very customizable. Let's think on it a bit more. For now I will change the Rgb colors to use the ColorProfile trait and lets see if it makes sense. |
I'm still not convinced. Forcing the user to pick a particular set of primaries and encoding, even if it's irrelevant, doesn't feel right. It's also like trading one problem for another. What if the user uses more than one "profile"? This extra information will only be used when converting to and from RGB, so it's otherwise just baggage that will interfere with type checking.
Can we at least call it something else, and save |
So here is the plan for the Rgb colors
I was planning to make the the RgbColorSpace whitepoint same as the primaries white point, but then I realized that you might need something like Also, in the color profile, encoding means something else and the gamma correction function is called color component transfer function. So I changed it. Let me know if you prefer TransferFunction or Gamma or GammaCorrection. I think Gamma is the more common term. Any other suggestion including naming are welcome :) |
First, as I have said multiple times now, "gamma" is the exponent in the f(x, gamma) = xgamma function. It's very specific, while a transfer function can be non-exponential. "Transfer function" is an ok term, but I would prefer something else than The traits can, in general, be simplified by moving the type parameters and associated types to their functions. That will avoid some conflict risks. Something like this (I'm also playing with other ideas): trait RgbStandard {
type TransferFunction: TransferFunction;
type Primaries: Primaries;
type WhitePoint: WhitePoint; //The WhitePoint trait shouldn't really need its Float parameter
}
pub trait TransferFunction {
fn from_linear<T: Float>(T) -> T;
fn to_linear<T: Float>(T) -> T;
}
///Represents the tristimulus values for the Rgb primaries
pub trait Primaries {
//The primaries are the same, no matter the white point.
fn red<W: WhitePoint, T: Float>() -> Yxy<W, T>;
fn green<W: WhitePoint, T: Float>() -> Yxy<W, T>;
fn blue<W: WhitePoint, T: Float>() -> Yxy<W, T>;
}
//We can make use of the fact that one type can be many things here,
//like in my proof of concept. Just put it in a module and there won't be
//any significant naming collisions.
impl RgbStandard for Srgb {
type WhitePoint = D65;
type Primaries = Srgb;
type TransferFunciton = Srgb;
}
//I'm still not convinced that we should take the
//transfer function into account in linear RGB.
//I'm therefore checking how it looks without them,
//but a third alternative could be a trait for only primaries
//and white point.
pub struct RgbLinear<P = Srgb, W = D65, T = f32>
where T: Float,
P: Primaries,
W: WhitePoint
{
pub red: T,
pub green: T,
pub blue: T,
pub rgb_space: PhantomData<(P, W)>,
} I didn't think about it before, but the |
True. But gamma in color is typically associated with any non linear transformation, not necessarily just an exponent. Srgb is called gamma corrected even though it is strictly not gamma (has a linear transform for low values). While not semantically correct, gamma is a more familiar term and it will be easier for understand. I am ok with using TransferFunction. I'll change RgbColorSpace to RgbStandard. I don't see how this will work, impl RgbStandard for Srgb {
type WhitePoint = D65;
type Primaries = Srgb;
type TransferFunciton = Srgb;
} Won't I have to do import primaries::Srgb as SrgbPrimaries;
import transfer_fn::Srgb as SrgbFn;
impl RgbStandard for Srgb {
type WhitePoint = D65;
type Primaries = SrgbPrimaries;
type TransferFunciton = SrgbFn;
} If we have to rename them, I'd rather name them separately. I think it will avoid confusion. I don't fully understand why pub trait TransferFunction {
fn from_linear<T: Float>(T) -> T;
fn to_linear<T: Float>(T) -> T;
} is simpler than having a bound on the trait (for whitepoit or primaries or any other trait). There will never be a case when from_linear will return f32 and into linear will return f64. Is that something that needs to be enforced (that they return the same type of value?)? Do we do this for FromColor and IntoColor as well.
Look at this use case. I have some color in Rgb, I convert it into hsl and do some operations on it. convert it back to rgb and return it. I am not using the whitepoint here even though I am forced to select a white point here. I think we should optimize for the most common case case here rather than choosing which characteristics are the important and which are not. That is part of the reason I was suggesting we make cie colors also use color space trait. Thinking about this, can we make it so that only the traits being used need to be provided. Ex
|
Note that I'm not angry, annoyed or anything like that, even if this turned into a bit of a rant. It's just that we seem to have slightly different views of what these type parameters are meant to do, and these views are currently colliding.
I thought they could be the same type. I don't see a reason why the type
The float types are not part of the transfer function type, so I would say that making the trait parametric over the float types would be restricting in the wrong way. A transfer function is what it is, no matter if it's used for
We don't know that. It may be improbable, but not impossible. The user is free to do whatever they want with them, including using them for multiple float types.
I don't think so. That's also a different case, since it's more connected to the types that implements them.
Let's take a look at what's the most common case. These are the projects here on GitHub where "palette" occurs in a .toml file. They are only 13, some of them are false positives and some of them are no longer active. Not much for statistics, but still.
That's six useful samples, one without clear intent, and six false positives. I guess we should really optimize for HSL/HSV <-> RGB (66.66% of the cases) and maybe gradients (33.33% of the cases), based on those. None of that needs any information about white point and primaries. Only the transfer function, for "proper" (linear) gradients and shading. 33.33% of all of them makes use of the built in sRGB encoding. It could be 50% if one of them didn't use its own encoding system. I don't think that's a helpful source for optimization decisions, at the moment.
I'm trying to model the definitions of the colors and the only reason why the white point is everywhere is that it's a part of every definition, except two. Filling them in seemed to be more good than bad.
This is the minimal knowledge you need to express their physical properties. Extending XYZ and xyY, the only non-preceptive spaces, with the white point makes sure they are also preceptive at the same at the type level, saying that they are meant to be observed under the same conditions. We could express a limited set of the same properties by removing all of the type parameters and using names such as So, the question isn't exactly what's important or not, but how much intent we want to reflect with the types. We must also remember that this isn't optional. There is no way to opt out of expressing some intent if there is a type parameter for it, so it's always or never. The primaries are only really relevant when converting to and from the RGB group, and the transfer function is only really relevant when converting between the linear and non-linear RGB groups. Converting to and from linear RGB should normally only happen on two occasions in a program; during input and output, and it will probably not be much worse than Converting to and from something outside the RGB group will only happen during more advanced color computations and that's what I would expect happens most, in a more advanced application. It will hopefully still not be too bad, since such an application is hopefully divided into functions, where the input and output types should define the RGB space well enough for type inference. The way I see it is that there isn't much impact we can have on these two situations without making other situations way worse. Extending the bundled intent of every color with RGB primaries and transfer functions will reduce Sure, most users would probably stick to only one RGB model, but I think they can survive having to specify it once or twice, if that would mean that almost every other scenario wouldn't get worse. The combination of carefully designed traits, a good selection of type aliases, and the usual type inference will probably be helpful enough. That's the way I'm reasoning. |
I am not sure I understand. Are you saying that in my example there is no need to rename the Srgb imports? It's a compile error. I my experience name space collisions are never fun to deal with. The other way to look at this is to use it like the error type and prefix the module name import primaries;
import transfer_fn;
impl RgbStandard for Srgb {
type WhitePoint = D65;
type Primaries = primaries::Srgb
type TransferFunciton = transfer_fn::Srgb;
} I guess that should work too.
The two conversions might happen in different parts of the program and we are losing that information. Think of a pipeline that does multiple operations on a color and at the end with the linear color I have no way of encoding it into the original form without manually keeping track of it. Is this a common enough use case to need an explicit type? I leave it up to you to decide. Finally, any reason to choose the trait over a type alias. I remember discussing this but I can't find it. type Srgb = Rgb<SrgbTf, SrgbPrimaries, D65>
// vs
pub struct Srgb;
impl<T: Float> RgbColorSpace<T> for Srgb {
type TransferFunction = SrgbTransferFunction;
type Primaries = SrgbPrimaries;
type WhitePoint = D65;
} |
I think it's ok to lose some of that information, as long as the risk of any mix-ups and accidents is minimized. The transfer function is pretty insignificant during computations, since it doesn't affect how the color is perceived (except if it's misinterpreted, but that's always a risk), and it will be known again when the color is encoded. No big deal, IMO. The primaries are more critical, since they do have an effect on the colors, but I don't know if they are critical enough to be as ubiquitous as the white point.
I'm very tired and may have misunderstood everything, but I'll try to answer anyway. The user will know what they start out with and what they want to end up with. That should be enough for the type inference to fill in most of the blanks. Let's say you start out with //`pix` is some pixel representation, like `[u8; 3]`.
//`Srgb<T = f32>` is an alias for `Rgb<rgb_standards::Srgb, T>`.
let color = Srgb::from_pixel(&pix).into_linear(); //We just lost the transfer function
let lab = color.into_lab(); //We just lost the primaries
let lab_shifted = lab.shift_hue(180.0.into());
//The compiler knows that this will become an `Srgb`,
//so it know the primaries from the future.
let new_color = lab_shifted.into_rgb();
//The transfer function will also become known here
pix = Srgb::from_linear(new_color).into_pixel(); Expanding this to a larger application should introduce enough input and return types to aid the type inference and keep it simple enough. Still, it's hard to be sure about anything without trying it for real.
I'm not sure what you are asking, but I'll sleep on it. |
I have an alternate proposal. Currently this is an issue // this works
let xyz: Xyz<D65> = Xyz::with_wp(...)
let rgb: LinearRgb<SrgbPrimaries, D65> = Xyz::with_wp(...)
// this does not
let xyz: Xyz<SrgbPrimaries, D65> = Xyz::with_wp(...)
let rgb: LinearRgb<TransferFunction, SrgbPrimaries, D65> = Xyz::with_wp(...) So remove the trait bounds from all colors. Ex Struct Xyz<C, T>{
x: T,
...
color: C // whatever you want to call this
} Add the 3 basic traits // the basic traits
trait WhitePoint
trait Primaries
trait TransferFunction Add the composing traits trait ColorSpace{
type TransferFucntion,
type Primaries,
type WhitePoint
}
impl WhitePoint for ColorSpace
impl Primaries for ColorSpace
impl TransferFunction for ColorSpace
trait LinearColor{
type Primaries,
type WhitePoint
}
impl WhitePoint for LinearColor
impl Primaries for LinearColor All the conversions and adaptation need to be modified to use the base traits FromColor for Hsl{
from_xyz<C: Primaries + WhitePoint>(xyz: Xyz<C>) -> Hsl<C>{
....
}
// no trait bounds here. Should work for trait
from_rgb<C>(rgb: Rgb<C,T>) -> Hsl<C>{
....
}
} So this will work now let xyz_d50: Xyz<D65> = Xyz::with_color(...)
let xyz_linear: Xyz<LinearSrgb> = Xyz::with_color(...)
let xyz_color: Xyz<SrgbColorSpace> = Xyz::with_color(...)
// should work
xyz_d50.to_lab
// will not work. needs primaries trait and will be a compile error
xyz_d50.to_hsl()
// both should work
xyz_linear.to_lab()
xyz_linear.to_rgb()
//should work for gamma encoding
xyz_color.into_encoded()
xyz_color.to_lab()
xyz_color.to_rgb() So you only opt in to the traits required and higher order traits can be passed through conversions Issues:
Further optimizations: Can the trait made optional i.e ``Xyz and Xyz<D50,T> or Xyz<Srgb,T>` should work. I can't think of a way this will work. Maybe using a wrapper type? Might get too complicated |
That's a very interesting idea, and it's actually a quite good compromise. The only problems I can see is that |
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
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:
or, using a shortcut:
There's also the case where a simple custom gamma exponent can be used:
This is fine and all, but it's not really easy to use in a general way:
GammaRgb
has a variable gamma exponent, which makes it very different from other encodings, and impossible to fully express as a simple type.This 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 additionalgamma
component, besides the usualred
,green
andblue
. 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.The 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, ...>
andRgb<Gamma, Srgb, ...>
, but recent problems with such marker parameters, as well as the additional complexity of extra type parameters, makes that method less attractive.The 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:
Encodings 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 aboveRgbEncoding
that makes it possible for an encoding to map to any set of primaries:The 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 oldGammaRgb
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.The 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:
These 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.
Something 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 theAlpha
type, but I haven't figured out exactly how, so that question remains open.An other open question is type aliases for common RGB types, such as AdobeRgb. One idea is to not alias sRGB, and only implement
new
forRgb<Srgb, D65, T>
, similar to how it's done with onlyD65
today. An other is to alias all of them and not pick a default.The text was updated successfully, but these errors were encountered: