Improve usability of zooming in the animation editor #85142

CookieBadger · 2023-11-20T17:17:19Z

This PR seeks to improve the zoom usability in the AnimationPlayer editor, I would be happy to hear your feedback! Especially the changes I had to make to the header file is something that a more experienced pair of eyes should be laid on.

I added some code in the AnimationTrackEditor, such that zooming with the mouse zooms in on the current mouse position, whereas the zoom panner always zooms in on the center of the currently visible timeline, instead of zooming on the beginning in both cases.
This implementation comes as a result of the proposal #6204

I have never contributed before, so please bear with me :)

CookieBadger · 2023-11-22T14:33:23Z

Instead of always zooming in on the left side of the timeline, this is the way zooming behaves now:

Also works in the Bezier editor:

editor/animation_track_editor.h

fire · 2023-11-22T14:52:27Z

I like it! @TokageItLab What do you think?

Calinou

Tested locally, there are some issues:

The zoom is slightly offset from the mouse cursor position (or the center when using the zoom bar). This seems to vary depending on the key column's size and the editor window width.
Editor scales above 100% further offset the zoom (see the second video). Perhaps some multiplying/dividing by EDSCALE needs to be done somewhere.

This has the potential to be a big productivity boost, so it's encouraging to see this being worked on nonetheless 🙂

100% editor scale

simplescreenrecorder-2023-11-22_22.36.55.mp4

200% editor scale

simplescreenrecorder-2023-11-22_22.39.45.mp4

TokageItLab · 2023-11-22T23:22:39Z

I think it is a good way to go for improvement. I will test it as soon as some of the points already pointed out are fixed.

CookieBadger · 2023-11-27T21:11:51Z

Thank you for pointing the issues out, the zoomed timeline length was not calculated correctly. Should be fixed now!

CookieBadger · 2023-12-01T13:17:35Z

@TokageItLab do you want to give it a try?

Calinou

Tested locally (rebased on top of master d76c1d0), it works as expected (tested at 100% and 150% scale factors).

TokageItLab

When using the mouse wheel, it makes sense and works correctly based on the current cursor position. Nice work.

However, when using the slider in the lower right corner, the zoom should be based on the current position (e.g. Affter Effects bahave that). Especially when changing the zoom factor from small to large, I believe it is an odd behavior to have the end of the animation as the reference.

Current this PR:

sl1.mp4

FYI Affter Effects:

sl2.mp4

CookieBadger · 2023-12-15T15:32:26Z

Thanks for the suggestion @TokageItLab , makes sense. Something that I don't like about AfterEffects' approach though is that the timeline view switches position such that the cursor is centered upon zooming (0:06 in the video). I think this is quite distracting/disturbing, and it would be more appropriate if the zoom focused on the cursor, but didn't make the timeline jump around, like this:

However then, an edge case that makes that approach problematic, is what to do if the cursor position is outside the currently displayed range. It might again be distracting or disturbing if it automatically switches to the middle, but with the current approach it is unclear what the zoom is actually focusing on.

Alternatively, I could test for this case, and then just zoom on the middle of the timeline as before, but I'd like to know what you think.

YuriSizov · 2023-12-15T16:31:51Z

I think if the cursor is not visible at the moment zooming into the middle makes sense.

CookieBadger · 2023-12-27T02:14:44Z

@TokageItLab would you like to see any other changes?

TokageItLab

Now it seems to be working fine overall.

One concern is that the center appears to shake when zooming with the slider, so it would be nice if this could be removed, but it can be followed up later as another PR.

wiggled_current.mp4

editor/animation_track_editor.cpp

CookieBadger · 2023-12-27T13:58:23Z

Fixed comment style, added #define constant, and added a tiny offset to prevent the wiggling that @TokageItLab mentioned. I think the PR is ready for merge now.

zoom_slider3.mp4

editor/animation_track_editor.cpp

CookieBadger · 2023-12-28T23:52:35Z

any other requests? I'd be happy to finish this by the end of the year 😄

editor/animation_track_editor.h

KoBeWi · 2024-01-06T12:27:31Z

editor/animation_track_editor.cpp

@@ -1239,6 +1239,59 @@ void AnimationMultiTrackKeyEdit::set_use_fps(bool p_enable) {
 }

 void AnimationTimelineEdit::_zoom_changed(double) {


Shouldn't the variables here be float? Most methods called here use float.

I am not sure. get_value() returns a double, and hscroll (derives Range) expects a double, so I thought I'd just do all calculations with doubles. Should I change everything to be a float instead?

editor/animation_track_editor.cpp

akien-mga · 2024-01-08T11:17:27Z

Thanks! And congrats for your first merged Godot contribution 🎉

CookieBadger · 2024-01-08T13:17:35Z

Thank you!! 😊

AThousandShips added topic:editor usability topic:animation labels Nov 20, 2023

AThousandShips requested a review from groud November 20, 2023 17:18

AThousandShips added this to the 4.x milestone Nov 20, 2023

AThousandShips removed the request for review from groud November 20, 2023 17:18

Calinou added the enhancement label Nov 20, 2023

CookieBadger force-pushed the animation-player-improvements branch from ceab9b5 to 3ad79d4 Compare November 21, 2023 17:46

CookieBadger changed the title ~~[WIP] Usablity improvements of Zooming in AnimationPlayer~~ [WIP] Usability improvements of Zooming in AnimationPlayer Nov 21, 2023

CookieBadger force-pushed the animation-player-improvements branch from 3ad79d4 to df8cc28 Compare November 22, 2023 13:52

CookieBadger changed the title ~~[WIP] Usability improvements of Zooming in AnimationPlayer~~ Usability improvements of Zooming in AnimationPlayer Nov 22, 2023

CookieBadger marked this pull request as ready for review November 22, 2023 13:53

AThousandShips requested a review from a team November 22, 2023 14:11

CookieBadger commented Nov 22, 2023

View reviewed changes

editor/animation_track_editor.h Outdated Show resolved Hide resolved

CookieBadger force-pushed the animation-player-improvements branch from df8cc28 to 8ebe33a Compare November 22, 2023 17:30

Calinou reviewed Nov 22, 2023

View reviewed changes

CookieBadger force-pushed the animation-player-improvements branch from 8ebe33a to 2b85213 Compare November 27, 2023 21:09

CookieBadger force-pushed the animation-player-improvements branch from 2b85213 to 00a5415 Compare November 27, 2023 21:18

Calinou approved these changes Dec 4, 2023

View reviewed changes

YuriSizov requested a review from TokageItLab December 13, 2023 12:54

YuriSizov modified the milestones: 4.x, 4.3 Dec 13, 2023

TokageItLab requested changes Dec 13, 2023

View reviewed changes

YuriSizov changed the title ~~Usability improvements of Zooming in AnimationPlayer~~ Usability improvements of zooming in the animation editor Dec 16, 2023

YuriSizov changed the title ~~Usability improvements of zooming in the animation editor~~ Improve usability of zooming in the animation editor Dec 16, 2023

TokageItLab approved these changes Dec 27, 2023

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

AThousandShips reviewed Dec 27, 2023

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

CookieBadger force-pushed the animation-player-improvements branch from ea591f3 to 5426769 Compare December 27, 2023 13:44

TokageItLab reviewed Dec 27, 2023

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

CookieBadger force-pushed the animation-player-improvements branch from 5426769 to 26688c1 Compare December 27, 2023 14:58

akien-mga requested a review from KoBeWi January 2, 2024 16:54

KoBeWi reviewed Jan 6, 2024

View reviewed changes

editor/animation_track_editor.h Outdated Show resolved Hide resolved

KoBeWi reviewed Jan 6, 2024

View reviewed changes

editor/animation_track_editor.h Outdated Show resolved Hide resolved

KoBeWi reviewed Jan 6, 2024

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

KoBeWi reviewed Jan 6, 2024

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

KoBeWi reviewed Jan 6, 2024

View reviewed changes

editor/animation_track_editor.cpp Outdated Show resolved Hide resolved

CookieBadger force-pushed the animation-player-improvements branch 2 times, most recently from ac81bbe to a720758 Compare January 6, 2024 23:54

Improved usability of zoom features in animation player

5b3d5e0

CookieBadger force-pushed the animation-player-improvements branch from a720758 to 5b3d5e0 Compare January 7, 2024 00:34

KoBeWi approved these changes Jan 7, 2024

View reviewed changes

akien-mga merged commit a2bd7c3 into godotengine:master Jan 8, 2024
15 checks passed

akien-mga mentioned this pull request Jan 8, 2024

Make AnimationPlayer editor zoom on the current cursor position instead of the beginning of the track godotengine/godot-proposals#6204

Closed

CookieBadger mentioned this pull request Feb 2, 2024

Fix vertical zoom factor in Animation Bezier Editor #87857

Merged

CookieBadger deleted the animation-player-improvements branch February 18, 2024 20:01

CookieBadger mentioned this pull request Feb 18, 2024

Improve Animation Player Usability #72088

Closed

Calinou mentioned this pull request Jul 7, 2024

Backport AnimationPlayer editor zoom towards current timeline position to 3.x #94001

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve usability of zooming in the animation editor #85142

Improve usability of zooming in the animation editor #85142

CookieBadger commented Nov 20, 2023 •

edited

Loading

CookieBadger commented Nov 22, 2023

fire commented Nov 22, 2023

Calinou left a comment •

edited

Loading

TokageItLab commented Nov 22, 2023 •

edited

Loading

CookieBadger commented Nov 27, 2023

CookieBadger commented Dec 1, 2023

Calinou left a comment

TokageItLab left a comment •

edited

Loading

CookieBadger commented Dec 15, 2023

YuriSizov commented Dec 15, 2023

CookieBadger commented Dec 27, 2023

TokageItLab left a comment

CookieBadger commented Dec 27, 2023

CookieBadger commented Dec 28, 2023

KoBeWi Jan 6, 2024

CookieBadger Jan 6, 2024

akien-mga commented Jan 8, 2024

CookieBadger commented Jan 8, 2024

		@@ -1239,6 +1239,59 @@ void AnimationMultiTrackKeyEdit::set_use_fps(bool p_enable) {
		}

		void AnimationTimelineEdit::_zoom_changed(double) {

Improve usability of zooming in the animation editor #85142

Improve usability of zooming in the animation editor #85142

Conversation

CookieBadger commented Nov 20, 2023 • edited Loading

CookieBadger commented Nov 22, 2023

fire commented Nov 22, 2023

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

100% editor scale

200% editor scale

TokageItLab commented Nov 22, 2023 • edited Loading

CookieBadger commented Nov 27, 2023

CookieBadger commented Dec 1, 2023

Calinou left a comment

Choose a reason for hiding this comment

TokageItLab left a comment • edited Loading

Choose a reason for hiding this comment

CookieBadger commented Dec 15, 2023

YuriSizov commented Dec 15, 2023

CookieBadger commented Dec 27, 2023

TokageItLab left a comment

Choose a reason for hiding this comment

CookieBadger commented Dec 27, 2023

CookieBadger commented Dec 28, 2023

KoBeWi Jan 6, 2024

Choose a reason for hiding this comment

CookieBadger Jan 6, 2024

Choose a reason for hiding this comment

akien-mga commented Jan 8, 2024

CookieBadger commented Jan 8, 2024

CookieBadger commented Nov 20, 2023 •

edited

Loading

Calinou left a comment •

edited

Loading

TokageItLab commented Nov 22, 2023 •

edited

Loading

TokageItLab left a comment •

edited

Loading