Skip to content
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

Support for ACEScg #284

Open
LoganDark opened this issue Jun 23, 2022 · 34 comments
Open

Support for ACEScg #284

LoganDark opened this issue Jun 23, 2022 · 34 comments
Labels
color type A new color space, model or meta type

Comments

@LoganDark
Copy link

LoganDark commented Jun 23, 2022

Description

I recently discovered ACEScg via a ShaderToy, but it seems like there's a complete lack of Rust crates that implement conversion functions for it.

Since palette is focused on being correct (performance notwithstanding) I think it would be nice to support it here, since I can't read matrices.

Motivation

As far as I understand it, ACEScg is an alternative to Linear sRGB that can be used internally by renderers and compositors. It's a bit more complex than that (and I would like if it were easier to find documentation on WTF to do about all that) but ACEScg on its own should be easy enough to support.

Also see: https://acescolorspace.com/

@Ogeon
Copy link
Owner

Ogeon commented Jun 23, 2022

That would be cool! The fact that it's an RGB based space should make it relatively easy to add, unless I missed anything while doing a quick search for details.

@LoganDark
Copy link
Author

Do you plan on implementing this yourself any time soon, and if so, do you have an ETA? If not, I might just have to roll my own instead of waiting for palette to get it.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

We could look into patching it into 0.6 if it's urgent. I can prepare a patch branch. Otherwise, I can't promise any particular ETA. 0.7 has more on its to-do list.

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

if it's urgent.

It's not urgent; it's just for a personal project of mine. Besides, even if it was urgent, it wouldn't feel right to make demands of you. My personal priorities/urgencies shouldn't obligate you to work faster.

0.7 has more on its to-do list.

Would that include stuff like #282 that seem to refactor quite a lot?

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

Sorry about the phrasing, I didn't mean to imply that you are making demands. I should have asked if it's something you would like to use pretty soon or if it's just a general suggestion.

I may be wrong but this looks like it could just be another RGB standard to add, and shouldn't need anything that's new in 0.7. So it shouldn't be a problem to fast track it as a 0.6.1 patch. 🙂 Within the weekend if everything goes well.

It would be nice if #282 makes it in (it probably will) but there's a bit more work to do there. My personal to-do list consist mostly of refactored traits. There's a handful left, still.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

If you would like to use it already today, you can create a custom RGB standard in your project with the primaries, white point, and the linear transfer function. It will make the matrix for you and should work out of the box.

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

Sorry about the phrasing, I didn't mean to imply that you are making demands. I should have asked if it's something you would like to use pretty soon or if it's just a general suggestion.

In that case, yes, I'm looking to use it myself.

I may be wrong but this looks like it could just be another RGB standard to add, and shouldn't need anything that's new in 0.7. So it shouldn't be a problem to fast track it as a 0.6.1 patch. 🙂 Within the weekend if everything goes well.

Yeah, I'm pretty sure it's a linear RGB color space. I'm not sure if acescolorspace.com's images are tonemapped or not, it's kinda implied that they're not (just going straight from ACEScg to sRGB??) which would be incredibly impressive on ACEScg's part.

If you would like to use it already today, you can create a custom RGB standard in your project with the primaries, white point, and the linear transfer function. It will make the matrix for you and should work out of the box.

I think I recall reading some documentation on this somewhere, but palette's crate-level docs are missing an overview of where I can find specific tasks, so I can't find it now. Maybe it was a different crate.

In any case, could you clarify how I would do that?

edit: Looking at the Srgb struct it looks fairly straightforward. Gonna see if I can implement it myself. Thanks!

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

Sure! There's definitely room for improvements when it comes to the documentation. What you want to look for is these traits:

  • Primaries, where you just need to return the xyY primaries for the space (Y doesn't seem to be mentioned, so may implicitly be 1.0). You can grab them from here, for example: https://docs.acescentral.com/specifications/acescg/
  • WhitePoint, same as above, but for the reference white. This one uses XYZ, though, and I honestly don't remember why. But you can just call into_color or into_color_unclamped on the xyY value if you don't want to convert on your own.
  • RgbSpace to combine the primaries and white point.
  • RgbStandard to attach the LinearFn transfer function.

The space and standard traits may be skipped by using a (PrimariesType, WhitePointType, LinearFn) tuple.

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

  • where you just need to return the xyY primaries for the space (Y doesn't seem to be mentioned, so may implicitly be 1.0).

I honestly have no idea what xyY is. Though I'm still definitely a color rookie....

I'll see if I can get a working implementation together using the current version of palette. :)

WhitePoint, same as above, but for the reference white. This one uses XYZ, though, and I honestly don't remember why. But you can just call into_color or into_color_unclamped on the xyY value if you don't want to convert on your own.

The white point is just D65 which is already a constant in palette.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

I honestly have no idea what xyY is

It's essentially the coordinate system for the gamut/chromaticity diagrams. Wikipedia has an explanation of how to derive it from XYZ. But it should be enough to get the x and y numbers from the specification and input them into the Yxy type.

But let me know if anything is unclear!

The white point is just D65 which is already a constant in palette.

It's apparently closer to D60, which seem to be missing.

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

seem to be missing.

Patch release time? :P

You could add ACEScg support in it as well or I could just release a palette-acescg crate. I don't need a D50 constant since the white point is also provided on the page you linked.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

It's probably not the exact same anyway, but it doesn't hurt to add any missing white points that are useful. At least feel free to open a PR if you get ACEScg working! I have prepared a 0.6 branch for patching but will be a bit busy for the rest of the day.

@LoganDark
Copy link
Author

It's probably not the exact same anyway

Yep, I'm going to use the ACEScg white point, but having D60 constants in palette would make it more complete. Though I'm not going to use those most likely :D

@LoganDark
Copy link
Author

let red = Srgb::new(1.0f64, 0.0, 0.0);:

  • let red: Rgb<AcesCg, f64> = red.into();: nope
  • let red: Rgb<AcesCg, f64> = red.into_format();: nope
  • let red: Rgb<AcesCg, f64> = red.into_encoding();: nope
  • let red: Rgb<AcesCg, f64> = red.into_linear().into_encoding();: nope
  • let red = Rgb::<AcesCg, f64>::from_encoding(red);: nope
  • let red = Rgb::<AcesCg, f64>::from_color(red);: nope
  • let red = Rgb::<AcesCg, f64>::from_color_unclamped(red);: nope
  • let red = Rgb::<AcesCg, f64>::adapt_from(red);: FINALLY?

THIS IS NOT DOCUMENTED!

-_-

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

Anyway, my implementation of ACEScg already works mostly correctly (this crate is really convenient actually), except for the fact that it starts to disagree substantially with the calculator after about 4 decimal places. Now, I have no idea why this is, but it may be something worth looking into.

use palette::{FloatComponent, Xyz, Yxy};
use palette::encoding::linear::LinearFn;
use palette::rgb::{Primaries, RgbSpace, RgbStandard};
use palette::white_point::WhitePoint;

pub enum AP1 {}

impl Primaries for AP1 {
	fn red<Wp: WhitePoint, T: FloatComponent>() -> Yxy<Wp, T> {
		Yxy::with_wp(T::from_f64(0.713), T::from_f64(0.293), T::from_f64(1.0))
	}

	fn green<Wp: WhitePoint, T: FloatComponent>() -> Yxy<Wp, T> {
		Yxy::with_wp(T::from_f64(0.165), T::from_f64(0.830), T::from_f64(1.0))
	}

	fn blue<Wp: WhitePoint, T: FloatComponent>() -> Yxy<Wp, T> {
		Yxy::with_wp(T::from_f64(0.128), T::from_f64(0.044), T::from_f64(1.0))
	}
}

#[derive(Copy, Clone, Eq, PartialEq, Debug)]
pub enum AcesCg {}

impl WhitePoint for AcesCg {
	fn get_xyz<Wp: WhitePoint, T: FloatComponent>() -> Xyz<Wp, T> {
		Xyz::with_wp(T::from_f64(0.9526460745698463), T::from_f64(1.0), T::from_f64(1.0088251843515859))
	}
}

impl RgbSpace for AcesCg {
	type Primaries = AP1;
	type WhitePoint = AcesCg;
}

impl RgbStandard for AcesCg {
	type Space = AcesCg;
	type TransferFn = LinearFn;
}

#[test]
fn print_stuff() {
	use palette::chromatic_adaptation::AdaptFrom;
	use palette::rgb::Rgb;
	use palette::Srgb;

	let red = Srgb::new(1.0f64, 0.0, 0.0);
	let green = Srgb::new(0.0f64, 1.0, 0.0);
	let blue = Srgb::new(0.0f64, 0.0, 1.0);
	let white = Srgb::new(1.0f64, 1.0, 1.0);

	eprintln!("red:   {:?}", Rgb::<AcesCg, f64>::adapt_from(red));
	eprintln!("green: {:?}", Rgb::<AcesCg, f64>::adapt_from(green));
	eprintln!("blue:  {:?}", Rgb::<AcesCg, f64>::adapt_from(blue));
	eprintln!("white: {:?}", Rgb::<AcesCg, f64>::adapt_from(white));
}

(P.S. your PhantomData<S> inside of Rgb means that even uninhabited enums need to implement Copy/Clone/Debug in order for Rgb to implement it, which seems like an oversight. You should probably be using PhantomData<&'static S> or something instead, so you can reference the generic parameter S in methods, but without "owning" an instance of it)

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

Direct RGB to RGB conversion is currently limited to colors with the same white point. It may have been a case where the type inference breaks down, or just legacy. It may be possible to drop that restriction. But for now, I think you probably want to go via XYZ, so some form of let red = Rgb::<AcesCg, f64>::from_color(Xyz::from_color(red));. adapt_from is for white balancing (one of the traits I want to refactor) and will make them look similar to the D65 colors when observed under an ACES illuminant.

THIS IS NOT DOCUMENTED!

So much to do, so little time. It's usually only me on this project and it's not my daytime job, so progress is always a bit slow. That's why I usually don't give ETAs too. 😉

P.S. your PhantomData<S> inside of Rgb means that even uninhabited enums need to implement Copy/Clone/Debug in order for Rgb to implement it, which seems like an oversight.

Yes, this is old legacy and I'm considering changing it, now when I know better. Probably to something involving fn, like PhantomData<fn(S)>.

@LoganDark
Copy link
Author

LoganDark commented Jun 24, 2022

for now, I think you probably want to go via XYZ, so some form of let red = Rgb::<AcesCg, f64>::from_color(Xyz::from_color(red));. ``

lol Rgb::<AcesCg, f64>::from_color(Xyz::from_color(red)) = "type mismatch resolving <AcesCg as RgbSpace>::WhitePoint == D65"

this sucks

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

It's set to prefer sticking to the same white point throughout the process. Changing white point can have weird visual consequences if it's done the wrong way. You are always free to use other tools if this doesn't fit your preferences, but you will still have to go through the same mathematics.

Now, if this was the upcoming 0.7 version, I would have just handed you a super convenient method to tell it to change the white point, but it's not. XYZ is actually completely independent of the white point so you can just construct a new Xyz with the same numbers, but with the ACES white point, and then convert back to ACEScg RGB. Rgb::<AcesCg, f64>::from_color(Xyz::new(xyz_d65.x, xyz_d65.y, xyz_d65.z)) may work unless the default type parameters for Xyz kick in.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

Ah, you know what? I had misunderstood and you did it correctly from the start with adapt_from. Sorry about that. See here, for example, the matrix in Shder Toy has it built in: https://computergraphics.stackexchange.com/questions/9834/how-to-convert-from-xyz-or-srgb-to-acescg-ap1

So any inaccuracies are probably from either floating point rounding or differences in the input/constants/matrices.

@LoganDark
Copy link
Author

It's set to prefer sticking to the same white point throughout the process. Changing white point can have weird visual consequences if it's done the wrong way. You are always free to use other tools if this doesn't fit your preferences, but you will still have to go through the same mathematics.

Sorry, I'm not trying to insult palette specifically, it's just this situation is extremely frustrating. The amount of trial and error I had to go through just to land on a solution that works but is apparently wrong is just really annoying. Basically I'm frustrated right now, so do bear that in mind.

Using this function:

fn transmute_xyz<Fwp: WhitePoint, Twp: WhitePoint, T: FloatComponent>(xyz: Xyz<Fwp, T>) -> Xyz<Twp, T> {
	Xyz { x: xyz.x, y: xyz.y, z: xyz.z, white_point: PhantomData }
}

and this conversion: Rgb::<AcesCg, f64>::from_color(transmute_xyz(Xyz::from_color(red)))

... the values are even more off. Absolute error of around 0.1 :(

I'm so confused.


I had typed the above before you wrote your latest reply.

Ah, you know what? I had misunderstood and you did it correctly from the start with adapt_from. Sorry about that. See here, for example, the matrix in Shder Toy has it built in: https://computergraphics.stackexchange.com/questions/9834/how-to-convert-from-xyz-or-srgb-to-acescg-ap1

:p

So any inaccuracies are probably from either floating point rounding or differences in the input/constants/matrices.

Would love to find a more precise definition of the primaries & white point, because that is almost definitely where the precision loss is coming from. There is no way they are only defined to 3 decimal places. But it costs hundreds of dollars to get access to the original specification.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

I know it can be frustrating when it doesn't work as one expects, but the best we can do is keep a good tone and keep on figuring it out. It's not about insulting Palette or not, I'm not expecting it to work for everyone. I just want to make it clear that I haven't been able to do much research into the specifics and I'm also learning as I go. I just can't give you the answer without looking deeper into the details, myself. So I'm also not deliberately trying to waste your time or anything, I'm just doing my best to help.

Like you, I'm not happy to drop hundreds of dollars on getting the specification so we need to use what we can get. But if you want to get the exact same result as in the calculator, you need to find out exactly what it does and what kind of data types it uses (likely f64 as you do, because javascript). I was trying to get my hands on a reference implementation when I bumped into that stackexchange thread. I'm sadly out of time for today, so I'll have to continue with the research later.

@LoganDark
Copy link
Author

I just can't give you the answer without looking deeper into the details, myself. So I'm also not deliberately trying to waste your time or anything, I'm just doing my best to help.

Thank you, you've been quite helpful throughout this exchange (I had no idea I could just implement my own color spaces in a separate crate).

likely f64 as you do, because javascript

I'm pretty sure it sends a web request to the acescolorspace.com server which uses OpenColorIO (C++) on the server side. So maybe I can look at the OCIO source, if it's open-source.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

I'm pretty sure it sends a web request to the acescolorspace.com server which uses OpenColorIO (C++) on the server side. So maybe I can look at the OCIO source, if it's open-source.

That may be the case! I saw this link in their references but haven't dug into it yet: https://github.com/AcademySoftwareFoundation/OpenColorIO

@LoganDark
Copy link
Author

Looks like that matches what I'm using here... and I double-checked that my xyY to XYZ calculations are correct (assuming Y=1):

>> let white_x = 0.32168f64;
>> let white_y = 0.33767f64;
>> white_x / white_y
0.9526460745698463
>> (1.0 - white_x - white_y) / white_y
1.0088251843515859

I wouldn't chalk this up to floating-point imprecision just yet, because it would take an extreme amount of rounding errors to be off by even 0.00001 (that is an insanely large epsilon value), so it's probably just a difference in the algorithms that palette and OCIO are using.

I would lean towards OCIO being the correct one, so maybe as a part of 0.7 you can review what is going on there.

@Ogeon
Copy link
Owner

Ogeon commented Jun 24, 2022

Thanks a lot for both submitting the request and taking the time to try to figure it out! It helps a lot!

Looks like that matches what I'm using here... and I double-checked that my xyY to XYZ calculations are correct (assuming Y=1)

Yes, looks like that's all correct. Then there's likely some difference in the process, yes, unless there's some bizarre platform difference. It's possible to get different results on different machines, but I wouldn't reach for that explanation in the first place. It's at least great to have their implementation as a reference!

@LoganDark
Copy link
Author

LoganDark commented Jun 25, 2022

So ACEScg is not as simple as just converting to and from sRGB. Converting albedo colors (absorption/reflectance) to ACEScg is done as I've implemented it, but once you have a fully rendered scene, you have to apply a specific Reference Rendering Transform (which is ACES' official name for a tonemapping function) before actually converting that for display. Namely, ACEScg has a higher dynamic range than just 0-1, as acescolorspace.com says, pure white is around a value of 16.

So converting an ACEScg output into sRGB for display basically requires a specific tonemapper that I haven't been able to find yet.

See https://chrisbrejon.com/cg-cinematography/chapter-1-5-academy-color-encoding-system-aces

@Ogeon
Copy link
Owner

Ogeon commented Jun 25, 2022

Right, that makes it more complex... I found a reference implementation of various transforms, including the/a RRT, here: https://github.com/ampas/aces-dev/tree/master. It's not exactly simple (that's par for the course) but quite readable from a glance. They have test images too, that could be used for verification.

It should be possible to port it over to Rust but it's clearly no longer a matter of just putting in a few chromaticity coordinates and calling it a day. 🤔

@LoganDark
Copy link
Author

LoganDark commented Jun 25, 2022

It should be possible to port it over to Rust but it's clearly no longer a matter of just putting in a few chromaticity coordinates and calling it a day. 🤔

Well, someone's gotta do the work eventually. This is a real professional color space, perhaps one of the first that palette could support, and the value in supporting it is potentially immense.

I recently encountered someone who uses Rust for media ingestion and conversion tasks. Do you want palette to be of utility to people like that? They described it as a "VFX pipeline". This gives me the impression that people are looking into using Rust for more serious production work.

Plus everyone is making hobby RGB raytracers left and right.

Of course, this is not going to be any OpenColorIO, but we can still open up the door. Maybe palette could support more of the ACES spaces as well; they share some common aspects, like the AP1 primaries (except for ACES 2065).

@LoganDark
Copy link
Author

It's most likely that I'll have to be the one to implement this; who knows if you'll want it directly in palette, I could end up with a general palette-aces crate that implements a lot more than just ACEScg.

@Ogeon
Copy link
Owner

Ogeon commented Jun 25, 2022

I'm just saying that it's larger than I first expected, but I'm not surprised. It's usually not that simple. Staging the implementation, and starting with some kind of MVP as you said, sounds like the way to go.

I recently encountered someone who uses Rust for media ingestion and conversion tasks. Do you want palette to be of utility to people like that?

That's part of the goal, or at least to offer tools and building blocks for a VFX pipeline, even if it isn't the most advanced parts to begin with. I'm trying to move it in that direction and away from simplifications and opinionated assumptions.

It's most likely that I'll have to be the one to implement this; who knows if you'll want it directly in palette, I could end up with a general palette-aces crate that implements a lot more than just ACEScg.

If you want it to be done as soon as possible, then that's sadly the case. As you know, I can't promise when I will get around to do it myself. The time I spend is very uneven and my focus is on fixing up the remaining traits. I'm definitely interested in having at lest the core parts in Palette, so PRs are welcome, but I will also help paving the way for an external crate if you prefer that. It would be a first, so there are probably some rough edges.

@LoganDark
Copy link
Author

If you want it to be done as soon as possible, then that's sadly the case. As you know, I can't promise when I will get around to do it myself. The time I spend is very uneven and my focus is on fixing up the remaining traits. I'm definitely interested in having at lest the core parts in Palette, so PRs are welcome, but I will also help paving the way for an external crate if you prefer that. It would be a first, so there are probably some rough edges.

I think this would be a good case study on whether Palette's API is working or not. So far 0.6 is feeling pretty OK, I haven't tried 0.7 but I probably could if I finish implementing this.

Maybe just going for spectral would be easier xD (By the way, does palette have wavelength-to-XYZ stuff?)

@Ogeon
Copy link
Owner

Ogeon commented Jun 25, 2022

Maybe just going for spectral would be easier xD

If we are talking 3D rendering, I did that in another project of my own. It's pretty cool, especially since you can get things like dispersion "for free", but requires more memory to store a spectrum per pixel. I used a method that's similar to what's described here: https://jo.dreggn.org/path-tracing-in-production/2017/talk-jo/

By the way, does palette have wavelength-to-XYZ stuff?

Not really. There's some kind of curse over all attempt to implement it. 👻 Jokes aside, it's a mix of low prio/demand and no obvious way of doing it nicely so far. The difficult part is to represent and sample the spectrum in a generic way, but the trick may be to just provide an interface for plugging in external representations. 🤔 But that's a library problem and just implementing it for a specific purpose is really quite easy!

@LoganDark
Copy link
Author

requires more memory to store a spectrum per pixel

Why do you need to store a spectrum per pixel? Can't you just store XYZ per pixel?

@Ogeon
Copy link
Owner

Ogeon commented Jun 25, 2022

You can do that too, it's fine! It depends on what kind of pipeline you want.

@Ogeon Ogeon added the color type A new color space, model or meta type label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
color type A new color space, model or meta type
Projects
None yet
Development

No branches or pull requests

2 participants