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.

Sign up for GitHub

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

Add FPS compatible option in second mode of the animation editor snapping #99013

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Add FPS compatible option in second mode of the animation editor snapping #99013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 55 additions & 9 deletions editor/animation_track_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@

constexpr double FPS_DECIMAL = 1.0;
constexpr double SECOND_DECIMAL = 0.0001;
constexpr double FPS_STEP_FRACTION = 0.0625;

void AnimationTrackKeyEdit::_bind_methods() {
ClassDB::bind_method(D_METHOD("_update_obj"), &AnimationTrackKeyEdit::_update_obj);
Expand Down Expand Up @@ -3776,6 +3775,7 @@ void AnimationTrackEditor::set_animation(const Ref<Animation> &p_anim, bool p_re
step->set_read_only(false);
snap_keys->set_disabled(false);
snap_timeline->set_disabled(false);
fps_compat->set_disabled(false);
snap_mode->set_disabled(false);
auto_fit->set_disabled(false);
auto_fit_bezier->set_disabled(false);
Expand All @@ -3798,6 +3798,7 @@ void AnimationTrackEditor::set_animation(const Ref<Animation> &p_anim, bool p_re
step->set_read_only(true);
snap_keys->set_disabled(true);
snap_timeline->set_disabled(true);
fps_compat->set_disabled(true);
snap_mode->set_disabled(true);
bezier_edit_icon->set_disabled(true);
auto_fit->set_disabled(true);
Expand Down Expand Up @@ -5029,7 +5030,12 @@ void AnimationTrackEditor::_snap_mode_changed(int p_mode) {
}
marker_edit->set_use_fps(use_fps);
// To ensure that the conversion results are consistent between serialization and load, the value is snapped with 0.0625 to be a rational number when FPS mode is used.
step->set_step(use_fps ? FPS_STEP_FRACTION : SECOND_DECIMAL);
step->set_step(use_fps ? FPS_DECIMAL : SECOND_DECIMAL);
if (use_fps) {
fps_compat->hide();
} else {
fps_compat->show();
}
_update_step_spinbox();
}

Expand All @@ -5045,7 +5051,6 @@ void AnimationTrackEditor::_update_step_spinbox() {
} else {
step->set_value(1.0 / animation->get_step());
}

} else {
step->set_value(animation->get_step());
}
Expand All @@ -5054,6 +5059,20 @@ void AnimationTrackEditor::_update_step_spinbox() {
_update_snap_unit();
}

void AnimationTrackEditor::_update_fps_compat_mode(bool p_enabled) {
_update_snap_unit();
}

void AnimationTrackEditor::_update_nearest_fps_label() {
bool is_fps_invalid = nearest_fps == 0;
if (is_fps_invalid) {
nearest_fps_label->hide();
} else {
nearest_fps_label->show();
nearest_fps_label->set_text("Nearest FPS: " + itos(nearest_fps));
}
}

void AnimationTrackEditor::_animation_update() {
timeline->queue_redraw();
timeline->update_values();
Expand Down Expand Up @@ -5115,6 +5134,7 @@ void AnimationTrackEditor::_notification(int p_what) {
bezier_edit_icon->set_button_icon(get_editor_theme_icon(SNAME("EditBezier")));
snap_timeline->set_button_icon(get_editor_theme_icon(SNAME("SnapTimeline")));
snap_keys->set_button_icon(get_editor_theme_icon(SNAME("SnapKeys")));
fps_compat->set_button_icon(get_editor_theme_icon(SNAME("FPS")));
view_group->set_button_icon(get_editor_theme_icon(view_group->is_pressed() ? SNAME("AnimationTrackList") : SNAME("AnimationTrackGroup")));
selected_filter->set_button_icon(get_editor_theme_icon(SNAME("AnimationFilter")));
imported_anim_warning->set_button_icon(get_editor_theme_icon(SNAME("NodeWarning")));
Expand Down Expand Up @@ -5160,9 +5180,8 @@ void AnimationTrackEditor::_update_step(double p_new_step) {
double step_value = p_new_step;
if (timeline->is_using_fps()) {
if (step_value != 0.0) {
// step_value must also be less than or equal to 1000 to ensure that no error accumulates due to interactions with retrieving values from inner range.
// A step_value should be less than or equal to 1000 to ensure that no error accumulates due to interactions with retrieving values from inner range.
step_value = 1.0 / MIN(1000.0, p_new_step);
;
}
timeline->queue_redraw();
}
Expand Down Expand Up @@ -7314,19 +7333,30 @@ void AnimationTrackEditor::_selection_changed() {
}

void AnimationTrackEditor::_update_snap_unit() {
nearest_fps = 0;

if (step->get_value() <= 0) {
snap_unit = 0;
_update_nearest_fps_label();
return; // Avoid zero div.
}

if (timeline->is_using_fps()) {
_clear_selection(true); // Needs to recreate a spinbox of the KeyEdit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, can you explain/document this line better? It's causing the animation player to unselect keyframes when I click them in my fork. What is this line trying to solve? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

The FPS in the key edit inspector is displayed dependent on the FPS of the snap value. For example, a key placed at 1s will appear as follows in FPS mode:

image image

I thought that changing the FPS with the keyedit inspector displayed would implicitly change the FPS in the inspector and have some negative effect, but sorry I did not realize that _update_snap_unit() is fired each key time is changed in the inspector (although I think it's a redundant).

There are some instability issues, such as undo after switching from second mode and keys not working due to spin box arrows, but this seems to be a problem that has been occurring for some time and probably needs to be revert in this area and stabilized from other parts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, thank you for these details! This is starting to make sense. Hopefully these redundant method calls can be untangled easily. For now I will remove this line and deal with any eccentricities on my end. Cheers!

snap_unit = 1.0 / step->get_value();
} else {
double integer;
double fraction = Math::modf(step->get_value(), &integer);
fraction = 1.0 / Math::round(1.0 / fraction);
snap_unit = integer + fraction;
if (fps_compat->is_pressed()) {
snap_unit = CLAMP(step->get_value(), 0.0, 1.0);
if (!Math::is_zero_approx(snap_unit)) {
real_t fps = Math::round(1.0 / snap_unit);
nearest_fps = int(fps);
snap_unit = 1.0 / fps;
}
} else {
snap_unit = step->get_value();
}
}
_update_nearest_fps_label();
}

float AnimationTrackEditor::snap_time(float p_value, bool p_relative) {
Expand All @@ -7347,6 +7377,10 @@ float AnimationTrackEditor::snap_time(float p_value, bool p_relative) {
return p_value;
}

float AnimationTrackEditor::get_snap_unit() {
return snap_unit;
}

void AnimationTrackEditor::_show_imported_anim_warning() {
// It looks terrible on a single line but the TTR extractor doesn't support line breaks yet.
EditorNode::get_singleton()->show_warning(
Expand Down Expand Up @@ -7600,6 +7634,18 @@ AnimationTrackEditor::AnimationTrackEditor() {
snap_keys->set_pressed(true);
snap_keys->set_tooltip_text(TTR("Apply snapping to selected key(s)."));

fps_compat = memnew(Button);
fps_compat->set_flat(true);
bottom_hb->add_child(fps_compat);
fps_compat->set_disabled(true);
fps_compat->set_toggle_mode(true);
fps_compat->set_pressed(true);
fps_compat->set_tooltip_text(TTR("Apply snapping to the nearest integer FPS."));
fps_compat->connect(SceneStringName(toggled), callable_mp(this, &AnimationTrackEditor::_update_fps_compat_mode));

nearest_fps_label = memnew(Label);
bottom_hb->add_child(nearest_fps_label);

step = memnew(EditorSpinSlider);
step->set_min(0);
step->set_max(1000000);
Expand Down
7 changes: 7 additions & 0 deletions editor/animation_track_editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,8 @@ class AnimationTrackEditor : public VBoxContainer {
AnimationMarkerEdit *marker_edit = nullptr;
HSlider *zoom = nullptr;
EditorSpinSlider *step = nullptr;
Button *fps_compat = nullptr;
Label *nearest_fps_label = nullptr;
TextureRect *zoom_icon = nullptr;
Button *snap_keys = nullptr;
Button *snap_timeline = nullptr;
Expand Down Expand Up @@ -637,6 +639,8 @@ class AnimationTrackEditor : public VBoxContainer {
void _track_grab_focus(int p_track);

void _update_scroll(double);
void _update_nearest_fps_label();
void _update_fps_compat_mode(bool p_enabled);
void _update_step(double p_new_step);
void _update_length(double p_new_len);
void _dropped_track(int p_from_track, int p_to_track);
Expand Down Expand Up @@ -853,6 +857,8 @@ class AnimationTrackEditor : public VBoxContainer {
void _pick_track_select_recursive(TreeItem *p_item, const String &p_filter, Vector<Node *> &p_select_candidates);

double snap_unit;
bool fps_compatible = true;
int nearest_fps = 0;
void _update_snap_unit();

protected:
Expand Down Expand Up @@ -935,6 +941,7 @@ class AnimationTrackEditor : public VBoxContainer {
bool can_add_reset_key() const;
float get_moving_selection_offset() const;
float snap_time(float p_value, bool p_relative = false);
float get_snap_unit();
bool is_grouping_tracks();
PackedStringArray get_selected_section() const;
bool is_marker_selected(const StringName &p_marker) const;
Expand Down
1 change: 1 addition & 0 deletions editor/icons/FPS.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion editor/plugins/animation_player_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,10 @@ float AnimationPlayerEditor::_get_editor_step() const {
const Ref<Animation> anim = player->get_animation(current);
ERR_FAIL_COND_V(anim.is_null(), 0.0);

float step = track_editor->get_snap_unit();

// Use more precise snapping when holding Shift
return Input::get_singleton()->is_key_pressed(Key::SHIFT) ? anim->get_step() * 0.25 : anim->get_step();
return Input::get_singleton()->is_key_pressed(Key::SHIFT) ? step * 0.25 : step;
}

void AnimationPlayerEditor::_animation_name_edited() {
Expand Down