godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.93k stars 21.15k forks source link

Code editor scrolls to show the caret when it's window loses focus #89285

Open mieldepoche opened 8 months ago

mieldepoche commented 8 months ago

Tested versions

System information

linux

Issue description

https://github.com/godotengine/godot/assets/71937306/bdde6001-e372-437e-8293-93a36c0a293c

Steps to reproduce

  1. get a long gdscript file
  2. put the caret in a line and scroll it outside of the screen
  3. give the focus to another os window
  4. the code editor should scroll on its own

Minimal reproduction project (MRP)

any

kitbdev commented 8 months ago

Cannot reproduce in Godot v4.3.dev4 on Windows 10.0.22621

smnast commented 8 months ago

I can consistently reproduce this error on my Linux system (void linux, i3wm) in Godot v4.3.dev4.

The relevant function here is TextEdit::adjust_viewport_to_caret, which scrolls the editor to show the caret. Using a debugger, I found this function was always called from TextEdit::_notification: (I'll keep the code snippets to just the relevant code)

case MainLoop::NOTIFICATION_OS_IME_UPDATE: {
    if (has_focus()) {
        adjust_viewport_to_caret(0); // <--- our problem
    }
} break;

The source of this IME update notification seems to be from DisplayServerX11::process_events

case FocusOut: {
    if (wd.ime_active) {
        OS_Unix::get_singleton()->get_main_loop()->notification(MainLoop::NOTIFICATION_OS_IME_UPDATE);
    }
} break;

Now this notification would imply that IME is active, which seems to always be the case because of TextEdit::_update_ime_window_position:

void TextEdit::_update_ime_window_position() {
    DisplayServer::get_singleton()->window_set_ime_active(true, get_viewport()->get_window_id());
}

And this update function is called in TextEdit::_notification, in the case of a draw notification:

case NOTIFICATION_DRAW: {
    // at the bottom of this absurdly long case...
    if (has_focus()) {
        _update_ime_window_position();
    }
} break;

So it seems that for the viewport to scroll to show the cursor, the flag for if IME is active must be set. But this flag is always set as long as the window has focus and can draw.

Perhaps someone with more experience in Godot's IME interface can understand why this is the case.

ConteZero commented 1 month ago

I can confirm the issue, Godot 4.3 with Opensuse Tumbleweed (KDE), it's really annoying.

kitbdev commented 1 month ago