Overhaul the Gradient Editor

[MewPurPur]

Closes godotengine/godot-proposals#6555

(I murdered this PR twice with bad rebasing, sorry about that :/ replaces #71746)

I just wanted to implement snapping in the Gradient Editor. But when I pulled on that thread, the whole ball of yarn unraveled.

Refactoring

Previously, we had two sets of files (gradient_editor and gradient_editor_plugin).

gradient_editor had the GradientEditor class. This was for the colorful rectangle that lets you edit a gradient, having all the logic around GUI input on it and also the color selector.

gradient_editor_plugin had the EditorInspectorPluginGradient class, which included the GradientEditor and a small button for reversing the gradient. It also had the GradientReverseButton class, because... yeah, needed a hack just to add that button. This wasn't scalable.

I renamed GradientEditor to GradientEdit, whereas the whole thing with the buttons is now a GradientEditor. These were all moved to one file like other editor plugins.

Before	After

Snap Button

Looks like this:

The slider and the vertical lines are only shown when snap is toggled.

I removed Ctrl+Shift+Drag (which snaps with 40 partitions) as the snap button makes this quite obsolete. Ctrl+Drag still works though, but it applies the snap count of the gradient.

Shift+Drag still works and is quite useful to get two points near each other when snapping. I gave it a smaller threshold by default. This can also be combined with Ctrl.

Improved Undo/Redo system

The undo/redo system wasn't functional. The single manager in _ramp_changed() left thousands of vague "Gradient Changed" messages in the output and would usually only lead you one step back before breaking, and that step back would usually not be where you wanted to get.

I didn't want to leave this system even more broken than before, so I modernized it. Now it has more descriptive messages and is more predictable. Code was simplified a lot too, removing some unused or unnecessary things.

Testing/Review on this would be appreciated because it might corrupt gradients if not done right!

Misc enhancements

• Improved the condition for when the handle is drawn in black or white, to maximize its contrast.

Before	After

• Added an indicator for overbright colors. (as shown in the previous section)
• Improved the condition for whether the color picker is shown above or below. If the color picker can fit neither below nor above, then it picks the direction with more space.
• Added hover indicators:
  
• Added the ability to deselect all handles by right-clicking anywhere without handles.
• Fixed drawing of translucent colors by using a separate checkerboard pattern for each handle.
• Added solid color indicators to translucent color handles:
  
• Improved the logic for keeping handles selected during operations (i.e. flipping when the gradient is reversed).
• Visually glitched handles with offsets outside of the [0, 1] range are no longer shown.
• Fixed typo in gradient.h (ponit -> point 🎠)

[KoBeWi]

Tested the editor and functionally it's good, although it would be nice if snapping was remembered. Right now it resets even if you just fold the gradient. You could use project metadata for that, or a static member.

[MewPurPur]

Not sure right now how I could save snapping, especially since it seems mostly useful on a per-gradient basis... GradientTexture2D's snapping could also use the solution to this.

[MewPurPur]

Reversing broke again if you spam-click the button. Help

[YuriSizov]

@MewPurPur How does it break? Have you considered adding a guard to the method, so it doesn't run again until it finishes the previous call?

Also, you haven't addressed some of the comments that you have marked as resolved. Is that intentional?

[MewPurPur]

No, not intentional, sorry, I reset my fix-up thinking that was what introduced the glitch.

• Spam-clicking reverse causes a tiny lag spike and points of the gradient occasionally disappear
• Doesn't happen when I just spam reverse() on a gradient in my project
• Undo also causes it (but the Undo of reverse() is also reverse() so that's not surprising)

Idek how to put guards in Undo Redo, but I'll try more things in a little.

[YuriSizov]

For UndoRedo you can use the merge events feature, then multiple subsequent actions should be treated as one operation. But I meant adding guards to the method that handles the click itself, before the UndoRedo gets called.

[MewPurPur]

Okay, now all the resolved things are fixed, sorry for the blunder. The reverse bug isn't fixed though, and it's easier to trigger than I thought. You don't have to spam-click, I had it happen after spacing out my clicks for about a second. (though it never happens if you hold off for longer than a second)

[YuriSizov]

The reverse bug isn't fixed though, and it's easier to trigger than I thought. You don't have to spam-click, I had it happen after spacing out my clicks for about a second. (though it never happens if you hold off for longer than a second)

Then you definitely need some guards that would make the reverse_gradient call return early if the gradient is currently being updated, and if the editor itself is currently being updated. Basically, at the start of reverse_gradient you would set a flag, and only when GradientEdit handles the change do you reset it.

[MewPurPur]

I found more things about the reverse_gradient() bug.

• It's actually not caused by spam-clicking at all. Seems to be a random chance it happens, something like 1 in 3 times. But I never had it happen the first time.
• Seems like points are not disappearing, but instead getting caught up in each other and getting the exact same color.

[MewPurPur]

W. T. F. now I'm occasionally getting the gradient corrupted with handles becoming colors that didn't even exist on the gradient.

EDIT: I removed the crash dump, it was unrelated and I've fixed it. But the thing with the colors is real.

[YuriSizov]

@MewPurPur If the issue is within the gradient itself, and not within this editor plugin, then we can go ahead with this PR (and make sure the issue is reported, so it can be looked into). Can it be reproduced without your PR? With the editor UI, or, say, by calling the reverse function from a script in a loop?

[MewPurPur]

I don't think it's within Gradient. I'm perplexed.

[MewPurPur]

Still no idea what it is, but I can only reproduce with one inconspicuous gradient. Reversing it in any context corrupts it, not just the gradient editor.

[gd_resource type="Gradient" format=3 uid="uid://dd3hdrq23ffbj"]

[resource]
interpolation_mode = 2
offsets = PackedFloat32Array(0.1, 0.4, 0.6, 0.9)
colors = PackedColorArray(0.109667, 0, 0.14, 1, 0.63, 0.1701, 0.177765, 1, 1, 0.3825, 0.05, 1, 1, 0.978, 0.88, 1)

So no, doesn't seem like a bug with my PR after all. Very strange though.

[MewPurPur]

In spite of this, I actually can't reproduce the bug with this gradient outside of this PR.

Send help

[YuriSizov]

I can't reproduce your bug even when holding down Undo/Redo shortcut (so things change very rapidly back and forth). There is a tiny bit of lag when the editor refreshes if you have packed arrays unfolded. But it doesn't corrupt the gradient in any way.

I say we merge it and maybe somebody will be able to reproduce your heisenbug.

[MewPurPur]

Me neither. Hopefully it's just some sort of artifact from this PR being up since pre-4.0 times, that won't happen in real projects. By now this is easily the weirdest bug I've ever had.

[Calinou]

Tested locally (rebased on top of master 2048fe5), it works as expected.

[akien-mga]

Thanks MewPurPur for this big improvement, and thanks to all reviewers too :)