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

Fix warp_mouse for transient windows, fix mouse jumping around when spinbox released #64574

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

Begah
Copy link
Contributor

@Begah Begah commented Aug 18, 2022

Problem

When using the spinbox on a transient window, releasing the mouse after a drag (to increase/decrease value) put the cursor in the wrong place.

bugshowcase-2022-08-18_16.09.52.mp4

At the same time, i also fixed a small annoyance: when releasing after drag, the cursor jumped around instead of appearing directly at the correct position

bugshowcase2-2022-08-18_16.10.47.mp4

(Those 2 problems where notices on Linux(Ubuntu) + X11, i don't know about the other os)

Reproduce the problems

To reproduce the problems, you need to create a simple editor plugin :

@tool
extends EditorPlugin

func _enter_tree():
	add_tool_menu_item("Test", test_panel.bind())
	
func _exit_tree():
	remove_tool_menu_item("Test")

func test_panel() -> void:
	var res_test = load("res://test.tscn")
	
	var window := Window.new()
	window.title = "Testing"
	window.wrap_controls = true
	window.transient = true
	window.always_on_top = false
	window.exclusive = true
	window.popup_window = true
	
	window.disable_3d = true
	
	var instance = res_test.instantiate()
	
	window.add_child(instance)
	window.min_size = instance.custom_minimum_size

	var base_control := get_editor_interface().get_base_control()
	base_control.add_child(window)
	window.popup_centered(window.min_size)

Then just create a simple test scene with a spinbox inside it, then try to drag to change the spinbox's value

Fix

The proposed fix in this commit fixes both of those problems, but changing warp_mouse to a virtual function will decrease performance a bit, but i seemed like the cleanest approach.

@Begah Begah requested review from a team as code owners August 18, 2022 14:22
@Calinou Calinou added this to the 4.0 milestone Aug 18, 2022
@Begah Begah force-pushed the fix_transient_window_wrap_mouse branch 2 times, most recently from 5f45bee to c01a588 Compare August 18, 2022 14:43
@Begah Begah requested a review from a team as a code owner August 18, 2022 14:43
@Begah Begah force-pushed the fix_transient_window_wrap_mouse branch from c01a588 to c35e50b Compare August 18, 2022 14:48
@Begah
Copy link
Contributor Author

Begah commented Aug 20, 2022

Anyone knows what the class_db.hpp:160:37: error: too many initializers for 'GDNativeExtensionClassCreationInfo'
error is during the linux build? Didn't get the error on my computer.

@bruvzg
Copy link
Member

bruvzg commented Aug 22, 2022

Anyone knows what the class_db.hpp:160:37: error: too many initializers for 'GDNativeExtensionClassCreationInfo'
error is during the linux build? Didn't get the error on my computer.

Rebase your PR to fix it (see https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html#the-interactive-rebase). There was a change to GDExtension API, it's unrelated to this PR, but CI is pulling the latest version of godot-cpp regardless of the version of GDExtension headers in your branch.

@bruvzg bruvzg self-requested a review August 22, 2022 11:55
@Begah Begah force-pushed the fix_transient_window_wrap_mouse branch from c35e50b to 33e4353 Compare August 22, 2022 12:27
@Begah
Copy link
Contributor Author

Begah commented Aug 22, 2022

Anyone knows what the class_db.hpp:160:37: error: too many initializers for 'GDNativeExtensionClassCreationInfo'
error is during the linux build? Didn't get the error on my computer.

Rebase your PR to fix it (see https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html#the-interactive-rebase). There was a change to GDExtension API, it's unrelated to this PR, but CI is pulling the latest version of godot-cpp regardless of the version of GDExtension headers in your branch.

Of course! It should have been obvious, thanks alot!

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Those 2 problems where notices on Linux(Ubuntu) + X11, i don't know about the other os)

Both issues are reproducible on macOS as well, so it's probably relevant for all platforms, and both issues are fixed by this PR.

Code seems fine, but need to account for single-window mode.

scene/main/window.cpp Outdated Show resolved Hide resolved
scene/main/window.cpp Outdated Show resolved Hide resolved
@Begah Begah force-pushed the fix_transient_window_wrap_mouse branch from 33e4353 to 5ac5c8b Compare August 25, 2022 09:31
@Begah Begah requested a review from bruvzg August 26, 2022 11:36
@akien-mga akien-mga merged commit ff612b6 into godotengine:master Aug 26, 2022
@akien-mga
Copy link
Member

Thanks!

@Begah Begah deleted the fix_transient_window_wrap_mouse branch August 26, 2022 12:20
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