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

Improvements to Gradient2D Editor #70940

Merged
merged 1 commit into from
May 29, 2023

Conversation

MewPurPur
Copy link
Contributor

@MewPurPur MewPurPur commented Jan 5, 2023

A few small tweaks to the GradientTexture2D editor.

  • Modifies snap_size into snap_count, which everyone I asked seemed to prefer for Gradient in Overhaul the Gradient Editor #70760. I think it does make more sense here too, as we're representing fractions and not pixels. But feel free to discuss
    image
  • If two handles are overlapping and the user clicks the area where they overlap, picks the handle that's closer, instead of always the FROM handle.
  • Remembers snapping options between sessions on a per-GradientTexture2D basis.

Improvements to Undo/Redo

  • Only leaves a message in the output when you release a handle to prevent spam
  • Fixes occasions in which it used to break
  • Doesn't leave a message if you leave the handle where you grabbed it

All of these Undo/Redo behaviors match how it is done in the CanvasItem editor.

@MewPurPur MewPurPur requested a review from a team as a code owner January 5, 2023 08:27
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from 248ef0e to f55cb4a Compare January 5, 2023 08:33
@KoBeWi KoBeWi added this to the 4.x milestone Jan 5, 2023
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from f55cb4a to 555f1ff Compare January 7, 2023 11:47
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch 2 times, most recently from d1072b1 to 42e2d9a Compare January 16, 2023 21:37
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from 42e2d9a to 61b8103 Compare March 27, 2023 22:21
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from 61b8103 to e92572d Compare April 16, 2023 02:40
@MewPurPur
Copy link
Contributor Author

MewPurPur commented Apr 16, 2023

This hasn't been receiving much attention, I thought I'd do a follow-up but I might as well just finish the job of polishing it now.

  • Added hover indication for the handles
  • Added axis snapping with Shift
  • Added temporary grid snapping with Ctrl

These are pretty expected functionalities from a 2D editor, I don't think I need to justify them

@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch 5 times, most recently from 09d2cf0 to 31db4b9 Compare April 16, 2023 22:09
@KoBeWi
Copy link
Member

KoBeWi commented Apr 24, 2023

Added temporary grid snapping with Ctrl

I think the editor should update as soon as you press Ctrl. Right now it has no effect until you drag.
EDIT:
Ah, I see it's implemented, but doesn't work 🙃

@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch 2 times, most recently from acbcb61 to e8fbb52 Compare April 24, 2023 20:09
@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from e8fbb52 to 60c6e8e Compare April 24, 2023 20:18
@MewPurPur
Copy link
Contributor Author

A bit short on time, sorry, but I think I addressed most of the remarks. Just not the static variable one, I'm not familiar with this feature, sorry-

@KoBeWi
Copy link
Member

KoBeWi commented Apr 24, 2023

Example is here:

static const String META_POINTER_PROPERTY_BASE;

It's static const, so that it's a constant that's allocated only once for the whole class (without static it's once per instance).
You need to initialize it in cpp file.

@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch 3 times, most recently from 146f983 to 14f244d Compare May 7, 2023 15:45
@MewPurPur
Copy link
Contributor Author

I think the editor should update as soon as you press Ctrl. Right now it has no effect until you drag.

I removed the implementation, I actually found it very odd when the grid appeared when you do stuff like Ctrl+Z, so I think it is reasonable to only show when you are dragging.

Implement the static const thing too. Also moved drawing to a _draw() function.

@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from 14f244d to d5428fd Compare May 7, 2023 15:55
@KoBeWi

This comment was marked as resolved.

@MewPurPur
Copy link
Contributor Author

MewPurPur commented May 7, 2023

Very weird, I have no idea what could possibly be causing this, but I'll try to debug it now. The only thing relating to those functions I remember doing was using the static var and meta suggestions...

@MewPurPur
Copy link
Contributor Author

MewPurPur commented May 7, 2023

The GradientTexture2D now gets rebuilt every time I press the snap button. I'm 90% sure this has to do with the added editor metadata causing the property tree to be updated, rebuilding this editor plugin, which shouldn't happen.

Busy now but I'll fix this soon. (Unless someone fixes the underlying cause - the pointless property tree rebuilding. pls...)

@KoBeWi
Copy link
Member

KoBeWi commented May 7, 2023

#76814
Although there is a faulty logic there, because you are storing the default value (true). Otherwise the button would be correctly unpressed after update...

@MewPurPur
Copy link
Contributor Author

Ah that's true. Thanks for pointing it out. I just saw that the code for the "READY" notif was running and rebuilding the editor, so I thought this might be the problem.

@MewPurPur MewPurPur force-pushed the better-gradient2d-editor branch from d5428fd to 39b79bb Compare May 7, 2023 20:45
@KoBeWi
Copy link
Member

KoBeWi commented May 7, 2023

I removed the implementation, I actually found it very odd when the grid appeared when you do stuff like Ctrl+Z, so I think it is reasonable to only show when you are dragging.

But it could at least update when you press Ctrl while dragging (without any movement). That's what I meant.

@Calinou
Copy link
Member

Calinou commented May 28, 2023

But it could at least update when you press Ctrl while dragging (without any movement). That's what I meant.

Note that this also needs to be done in the Gradient editor PR. That said, I don't consider it a blocker for either PR.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased against latest master 06ccbfe), it works.

Code looks good to me at a glance.

@akien-mga akien-mga modified the milestones: 4.x, 4.1 May 29, 2023
@akien-mga akien-mga merged commit 9f05e16 into godotengine:master May 29, 2023
@akien-mga
Copy link
Member

Thanks!

@MewPurPur MewPurPur deleted the better-gradient2d-editor branch May 29, 2023 09:10
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.

None yet

4 participants