Godot: Add new Color points to Gradient in gdscrip results in wrong ramp

[MikeSchulze]

Godot version:

Godot Engine v3.2.2.stable.mono.official - https://godotengine.org
OpenGL ES 2.0 Renderer: GeForce GTX 980/PCIe/SSE2
OpenGL ES 2.0 Batching: ON

OS/device including version:
Windows 10

Issue description:
I try to use Gradient in a godot script to build a Color ramp from [green]->[red]->[blue]
The result is [green]->[blue]->[red] but should be [green]->[red]->[blue]

Steps to reproduce:

class TileIdToColorMapper extends Object:
    var _gradient:Gradient = Gradient.new()

    func _init():
        _gradient.add_point(0.5, Color.red)
        print(_gradient.get_point_count())
        # set ramp
        _gradient.set_color(0, Color.green)
        _gradient.set_color(1, Color.red)
        _gradient.set_color(2, Color.blue)


        prints(_gradient.interpolate(_gradient.get_offset(0)), "index", 0, "offset", _gradient.get_offset(0))
        prints(_gradient.interpolate(_gradient.get_offset(1)), "index", 1,"offset", _gradient.get_offset(1))
        prints(_gradient.interpolate(_gradient.get_offset(2)), "index", 2, "offset", _gradient.get_offset(2))
        prints("all offsets:", _gradient.offsets)

the output are

3
0,1,0,1 index 0 offset 0
0,0,1,1 index 1 offset 0.5
1,0,0,1 index 2 offset 1
all offsets: [0, 0.5, 1]

The result is [green]->[blue]->[red] but should be [green]->[red]->[blue]

Minimal reproduction project:

[pouleyKetchoupp]

More details about the cause:
Points are ordered by offset only when get_color_at_offset is called, which leads to applying colors at the wrong positions when calling set_color after adding a new point.
The solution is to sort the points when set_color is called in case is_sorted is false.

[MikeSchulze]

Hi @pouleyKetchoupp thanks for confirming this bug, the documentation is very rare and no examples exists.

[somnathsarkar]

Hi @pouleyKetchoupp I was looking into fixing this issue. The proposed solution of sorting the points when set_color is called and is_sorted is false works, but shouldn't this also be done in get_color, get_offset and set_offset as well?

[maxxtepper]

Hello! This is my first time contributing to open source code! This issue appears to be straight forward; after looking through gradient.cpp and gradient.h, just a few lines of code need to be added to the methods where point sorting is needed (at the least set_color, and possibly the ones mentioned by @somnathsarkar). If points are not sorted, then sort. I would be happy to take this on!

[pouleyKetchoupp]

Welcome @somnathsarkar & @maxxtepper!

shouldn't this also be done in get_color, get_offset and set_offset as well?

Yes, I think it would be a good idea to make a private method to update the sorting and call it for all these cases as well.

@maxxtepper:
Your contribution would be welcome, but I would suggest to wait for a little bit since @somnathsarkar mentioned working on this issue, in order to avoid doing the work twice and ending up with two almost identical PRs.
In the meantime you can search for other issues marked with "junior job" label.

[somnathsarkar]

@pouleyKetchoupp Thanks for the update, I've submitted a PR for this issue #41166

[pouleyKetchoupp]

@maxxtepper Don't hesitate to review the PR if you want to help with the contribution. It's an important part of the work too.