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

Overhaul the Gradient Editor #70760

Closed
wants to merge 0 commits into from
Closed

Conversation

MewPurPur
Copy link
Contributor

@MewPurPur MewPurPur commented Dec 31, 2022

temp note: There's a few bugs I found while testing this again, will fix soon

  • Triple click on a handle allows you to move it while color popup is visible
  • Could probably check if I can fix the handle drawing after draw_rect() was fixed up

See also #70548, where I isolated the bugfixes in this PR so it can more easily get merged into 4.0

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 GradientEditorRect. The stuff inside EditorInspectorPluginGradient are essentially moved to the new GradientEditor, which includes a GradientEditorRect and regular buttons. These were all moved to one file.

Before After
image image

Snap Button

It looks like this:

image

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

I removed Ctrl+Drag (which snaps by 0.1) and Ctrl+Shift+Drag (which snaps by 0.025) as the snap button makes these tricks completely obsolete.

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 for this reason.

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.

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.
  • Added an indicator for overbright colors.
    image
  • 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.
  • Improved the logic for keeping handles selected during operations (i.e. flipping when the gradient is reversed).
  • Fixed typo in gradient.h (ponit -> point 🎠)

Bugfixes

The bugfixes are in the aforementioned PR, so I'll remove this section if it gets merged.

  • Fixes click detection bugs
  • Fixes handles deselecting when you double-click the color selector.
  • Fixes the color of new points not always being the hovered one.

@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jan 1, 2023

I would prefer a more laid back milestone because this isn't a bugfix, I made a separate PR with just the bugfixes. I could use some time to improve this PR.

@MewPurPur MewPurPur marked this pull request as draft January 1, 2023 01:09
@Calinou Calinou modified the milestones: 4.0, 4.x Jan 1, 2023
@MewPurPur MewPurPur marked this pull request as ready for review January 1, 2023 05:50
@MewPurPur MewPurPur force-pushed the grid-ient branch 6 times, most recently from 9cdfe5a to e0fcbd3 Compare January 1, 2023 16:19
@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jan 2, 2023

Little enhancement I just pushed: Gave visual feedback to hovering the handles:
image

Also I fixed the bugs I found in undo/redo yesterday.

@MewPurPur MewPurPur force-pushed the grid-ient branch 10 times, most recently from 504c78e to 03dcc8a Compare January 3, 2023 07:06
@Ansraer
Copy link
Contributor

Ansraer commented Jan 3, 2023

Maybe the Snap field should let you set the number of intervals instead of the distance between them?
So instead of 0.05 users would set it to 20.

@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jan 4, 2023

Mhm, I asked a few people. A statistically significant number prefer what you suggested :P (me included) So there we go.

image

Edit: Messed up a rebase, sorry :/
Edit: Two rebases * what's wrong with me

@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jan 20, 2023

Oops.

Edit: I can't fix this, sorry. I'll start a new PR. And I believe I've worked out how to rebase for real this time, so this won't happen again >_<

Here's the new PR: #71746

@aaronfranke aaronfranke removed this from the 4.x milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants