godot icon indicating copy to clipboard operation
godot copied to clipboard

Add `Gradient::interpolate` static method

Open adamscott opened this issue 1 year ago • 1 comments
trafficstars

Follow up to #97375

This PR adds the Gradient::interpolate() static method in order to offer to users a better way to quickly interpolate physically or perceptually color values. It adds a note to the documentation for Color::lerp() too.

Color Gradient::interpolate(const Color &p_from, const Color &p_to, float p_weight, Gradient::ColorSpace p_interpolation_color_space = Gradient::ColorSpace::GRADIENT_COLOR_SPACE_LINEAR_SRGB, Gradient::InterpolationMode p_interpolation_mode = Gradient::InterpolationMode::GRADIENT_INTERPOLATE_LINEAR)

I chose to create a static method in Gradient (instead of Color) because:

  1. Gradient already has color space management.
  2. Makes it easier to point people to an alternative to Color::lerp.
  3. Gradient already takes as input sRGB values.
  4. This implementation makes extensive use of the Gradient resource, see the equivalent GDScript code below.

I struggled to figure out how to explain properly in the documentation the use of Color::lerp without having to resort to a big block of code. This PR makes it simple: you want accurate sRGB color interpolation? Use Gradient::interpolate. This static function is just a useful shortcut to something that people can already do, but in a WAY LESS verbose fashion:

var gradient:  = Gradient.new()
gradient.set_color(0, color_one)
gradient.set_offset(0, 0.0)
gradient.set_color(1, color_two)
gradient.set_offset(1, 1.0)
gradient.interpolation_color_space = color_space
gradient.interpolation_mode = interpolation_mode
var result: = gradient.sample(weight)

Example project: gradient-interpolate.zip

Capture d’écran, le 2024-09-23 à 16 37 48

adamscott avatar Sep 23 '24 20:09 adamscott

~~What is the algorithm for perceptually color interpolate? I'll check~~

Seems to be linear/gamma and oklab (lights)?

I have an implementation of spectral paint mixing (for physical paints) https://github.com/V-Sekai/godot-paint-mixer, but it's not fully related to this current pull request.

The theory of spectral paints is from https://en.wikipedia.org/wiki/Kubelka%E2%80%93Munk_theory.

Edited:

The design proposal makes sense. lerp usually means a mathematical operation like between 0-1 and you cannot do that with colour unless you define a mathematical space where 0-1 smoothly changes color. I've at least mentioned three spaces: 1. linear color space 2. oklab color space 3. Kubelka–Munk color space. This code adds a way that fits adding more color spaces.

fire avatar Sep 23 '24 21:09 fire

I'm not clear on the implication of storing a thread_local static Ref<T>, and this feels pretty heavy handed for the purpose of running an equation to convert colors.

I'm not saying we shouldn't do that but I'd like more opinions from e.g. @RandomShaper and @clayjohn.

I tend to agree. We are starting to build a lot of infrastructure here to avoid writing code that could be expressed more simply. Look at the code actually run to handle this (it involves sorting a 2-element array and then running a binary search on that same array).

Basically what this PR is allowing users to do is interpolate two gamma space sRGB colors in either Linear space or OKLab space with linear or cubic interpolation.

The original problem comes from how challenging it is to understand blending and color spaces. Having a helper function can assist users as instead of saying "convert the color to a different color space then lerp, then convert back", you can just say: "that's because the default lerp looks bad, instead use this way of lerping and it will look good". I think there is merit in that, however I think:

  1. The demand needs to come from users and not engine devs. There is a real risk here of implementing a solution in search of a problem
  2. The helper function should be super simple, we could easily do the following to solve the original problem without adding as much bloat:
Color Color::lerp_linear(const Color &p_to, float p_weight) const {
	Color a = srgb_to_linear();
	Color b = p_to.srgb_to_linear();
	a.r = Math::lerp(a.r, b.r, p_weight);
	a.g = Math::lerp(a.g, b.g, p_weight);
	a.b = Math::lerp(a.b, b.b, p_weight);
	a.a = Math::lerp(a.a, b.a, p_weight);
	return a.linear_to_srgb();
}

clayjohn avatar Sep 25 '24 18:09 clayjohn

@clayjohn That makes sense.

Can you tell us the story of how you intend to use this in your game project @adamscott?

fire avatar Sep 25 '24 18:09 fire

Closing per Clay's comment.

adamscott avatar Sep 26 '24 12:09 adamscott

@clayjohn Can we please have the smaller helper as a new PR?

Zireael07 avatar Sep 26 '24 14:09 Zireael07

@clayjohn Can we please have the smaller helper as a new PR?

I don't think asking for PRs is a great thing to do. I like the idea behind it, but:

  1. Clay is really busy, and the amount of time he did give to this issue (and the fact that he suggested code) is already big.
  2. It's a really simple PR to do, I wouldn't ask the lead maintainer of rendering to do this.

adamscott avatar Sep 26 '24 14:09 adamscott