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

Animation track keys shift position when selected #92884

Closed
KoBeWi opened this issue Jun 7, 2024 · 5 comments · Fixed by #93257
Closed

Animation track keys shift position when selected #92884

KoBeWi opened this issue Jun 7, 2024 · 5 comments · Fixed by #93257

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Jun 7, 2024

Tested versions

v4.3.beta.custom_build [e96ad5a]
Not happening in 4.3 beta1

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 31.0.15.4633) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

Animation keys are bugged. I had this situation, where selecting a key would shift it by a constant distance:

godot.windows.editor.dev.x86_64_icr9JVTuOV.mp4

Not sure how to reproduce it, but a similar bug is easy to reproduce.

Steps to reproduce

  1. Have an animation with at least 1 key
  2. Right click the key (make sure it wasn't selected before clicking)
  3. Left click next to the key
PZNdPITLAK.mp4

Minimal reproduction project (MRP)

N/A

@matheusmdx
Copy link
Contributor

Bisecting points to #92424 as the culprit:

image

@TokageItLab
Copy link
Member

TokageItLab commented Jun 16, 2024

I have investigated and found that the problem is probably due to the fact that InputEventMouseMotion at line 3049 in animation_track_editor.cpp is valid even though it should not be fired when closing a popup. But in animation_bezier_editor.cpp, mouse events are not fired when closing a popup although it is similar implementation with animation_track_editor.cpp. The timing of adding popups between them is different, so I tried changing the process priority of the popup node, but that did not solve the problem.

animation_track_editor.cpp: line 3049

if (mm.is_valid() && mm->get_button_mask().has_flag(MouseButtonMask::LEFT) && moving_selection_attempt) {
	if (!moving_selection) {
		moving_selection = true;
		emit_signal(SNAME("move_selection_begin"));
	}

	float moving_begin_time = ((moving_selection_mouse_begin_x - timeline->get_name_limit()) / timeline->get_zoom_scale()) + timeline->get_value();
	float new_time = ((mm->get_position().x - timeline->get_name_limit()) / timeline->get_zoom_scale()) + timeline->get_value();
	float delta = new_time - moving_begin_time;
	float snapped_time = editor->snap_time(moving_selection_pivot + delta);

	float offset = 0.0;
	if (abs(editor->get_moving_selection_offset()) > CMP_EPSILON || (snapped_time > moving_selection_pivot && delta > CMP_EPSILON) || (snapped_time < moving_selection_pivot && delta < -CMP_EPSILON)) {
		offset = snapped_time - moving_selection_pivot;
		moving_selection_effective = true;
	}

	emit_signal(SNAME("move_selection"), offset);
}

It seems that #92424 is causing the unintended firing of mouse events (I have confirmed that mouse events are not fired when the popup closes in prior commit from that), but I don't have a detailed reason as to why this is happening.

@bruvzg Do you have any idea?

@bruvzg
Copy link
Member

bruvzg commented Jun 16, 2024

Check if it's fixed by #93105, might be the same issue.

@TokageItLab
Copy link
Member

@bruvzg The issue still occurs in the latest commit after merging it, so it seems to be a different problem.

@bruvzg bruvzg self-assigned this Jun 16, 2024
@bruvzg
Copy link
Member

bruvzg commented Jun 17, 2024

Seems like mouse move event can be received after closing popup wit
h a click and button can be still pressed, but I do not see anything wrong with it, since mouse is moving and button is pressed.

You can also do the same by opening a popup menu, holding down the left button (while hovering the menu separator) and closing the menu with ESC, the key will be moving until you release the button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants