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

Color Grading with Lookup Table #124

Closed
milcktoast opened this issue Apr 25, 2019 · 12 comments
Closed

Color Grading with Lookup Table #124

milcktoast opened this issue Apr 25, 2019 · 12 comments
Labels
discussion An open discussion or announcement

Comments

@milcktoast
Copy link

It'd be great to have a color grading effect that uses a lookup table to apply the color transform. I might have time to work on adding it if you like the idea.

@vanruesc
Copy link
Member

Hi,

that sounds cool! I'd be happy to include it in the library.

What do you think about the name ColorTransformationEffect?

For testing you can either create your own setup or you can add the effect to the ColorGradingDemo. If you choose to add it to the existing demo, remember to disable the other effects by setting their blend functions to SKIP.

Before you start, make sure you've read the contributing guidelines 😄
If you have any questions or need assistance, just ask here.

@vanruesc vanruesc added the discussion An open discussion or announcement label Apr 25, 2019
@Yzrsah
Copy link

Yzrsah commented May 6, 2019

ColorTransformation is too similar to ColorSpaceTransform. A color space transform is a different concept from a LUT, though a non-finite color space transform can be implemented using a LUT.

@vanruesc
Copy link
Member

vanruesc commented May 6, 2019

Thanks for the feedback @Yzrsah!

ColorTransformation is too similar to ColorSpaceTransform.

I guess that's true. However, both of the references that @jpweeks linked to use the term "color transformation" for this kind of image processing so I wouldn't rule it out yet.

I looked around for more references:

We could go with ColorCorrectionEffect or ColorGradingEffect. I lean more towards color correction.

Any suggestions?

@Yzrsah
Copy link

Yzrsah commented May 7, 2019

There is thousands of other things that can be included inside ColorCorrection or ColorGrading besides LUT. Having the LUT effect actually be called "ColorCorrection" implies that the library is limited in scope (i.e. Babylon is very limited). If you use something like Davinci Resolve for color correction and color grading, a LUT is a LUT. They do not try to give you newbie terms because the newbie terms would be too confusing and lacking meaning. i.e. "color grading" is ambiguous as it's a description of a category so it could not be used to have a meaning in a color grading app. It could only have a meaning in something that is limited in scope and therefore uses vague terms.

from the titles ColorGrading and ColorCorrection I could not guess that it's a LUT effect so I would google and open the docs to find out if they're a LUTEffect or something else

@milcktoast
Copy link
Author

@Yzrsah Most color grading techniques can be cached by a LUT; this is why they're so useful to realtime rendering. It is unlikely anyone is going to want to apply many independent, potentially computationally expensive, color grading transformations to a final render in a realtime rendering pipeline. Naming it ColorGradingEffect or ColorCorrectionEffect seems appropriate to me considering this is a realtime effects library.

@Yzrsah
Copy link

Yzrsah commented May 7, 2019

@jpweeks I.e. It's not possible to provide other color grading features in the future because it is too expensive and hardware performance in the future is static, and also we are not capable of writing fast shaders or making decisions about performance in our apps, therefore this library should be limited because this is going to be the only color grading effect and therefore the LUT should be named after the category of color grading.

@vanruesc
Copy link
Member

vanruesc commented May 7, 2019

Having the LUT effect actually be called "ColorCorrection" implies that the library is limited in scope

I don't see how this could limit the scope of the library. I'd argue that a term like "correction", "grading" or "transformation" implies flexibility.

If you use something like Davinci Resolve for color correction and color grading, a LUT is a LUT

Well, if you use the ColorTransformationEffect in postprocessing, a LUT is also a LUT. The effect doesn't have to be limited to the LUT functionality.

"color grading" is ambiguous as it's a description of a category

I agree. The terms "color grading" as well as "color correction" both describe a diverse group of color transformations. That's also why there are several similar effects in the ColorGradingDemo. However, the LUT approach is a specific color correction implementation. And for that reason a name like ColorCorrectionEffect would be appropriate.

from the titles ColorGrading and ColorCorrection I could not guess that it's a LUT effect so I would google and open the docs to find out if they're a LUTEffect or something else

The first thing that the user would look for is something related to color correction. The implementation details are secondary. You are actually describing a best case scenario with the documentation example.

It's not possible to provide other color grading features in the future

There's no reason to assume that.

this is going to be the only color grading effect and therefore the LUT should be named after the category of color grading

Don't underestimate the power of semantic versioning. If there's a better color grading solution, we can just replace the LUT implementation entirely or make the LUT feature optional.

In my opinion, the name of the effect should describe what it does, not what it uses to do it. The original resource calls it color transformation, so I think we should call it ColorTransformationEffect after all. ColorCorrectionEffect would be fine, too.

@milcktoast
Copy link
Author

Just an update on this:
I have this effect implemented in a project I'm working on; I'll merge it back into the library and create a PR once the project is finished and I have a bit more time.

@vanruesc
Copy link
Member

Got it, thanks for the update 👍

@vanruesc
Copy link
Member

@jpweeks Any news on this?

@vanruesc
Copy link
Member

vanruesc commented Sep 4, 2019

Closing this due to inactivity.

Feel free to re-open if you'd like to continue working on this!

@vanruesc vanruesc closed this as completed Sep 4, 2019
@milcktoast
Copy link
Author

I ended up not creating a PR since it's a very easy effect to setup with glslify and the glsl-lut package. Here's a gist if anyone's interested:
https://gist.github.com/jpweeks/6eef45a288e2348e850474a172cc992f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion An open discussion or announcement
Projects
None yet
Development

No branches or pull requests

3 participants