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 the initial candidate window position #2793

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhysd
Copy link
Contributor

@rhysd rhysd commented Feb 6, 2025

Fixes #2792

The issue is that the candidate window is already open when InputMethod::Open event is handled in the window manager. Since set_ime_cursor_area is not called yet when the window is opened, the position of the window is at the top left corner.

To fix this, we need to call set_ime_cursor_area earlier before the window is opened. winit's example calls the function just after calling set_ime_allowed.

This PR calls the function when allowing IME so InputMethod::Allowed event now has the position of the window.

Here is the video:

fix.mp4

@rhysd rhysd changed the title Fix candidate window position for the first time Fix the initial candidate window position Feb 7, 2025
@kenz-gelsoft
Copy link
Contributor

kenz-gelsoft commented Feb 8, 2025

I think this is sane fix for the problem I catched earlier on original PR:

#2777 (comment)

I investigated winit's macOS platform code and I find it correctly updates the internal ime_position queried from the OS and invalidates it when we call winit::Window::set_ime_cursor_area().

And as far as I tested macOS iuput methods doesn't reposition immediately while it is currently displayed. This is not only on winit apps but also on native macOS apps.

So this is not a bug of winit but is a cross platform limitation. We need to set_ime_cursor_area() before the subsequent Preedit event I guess.

Sorry about the misinformation on the decision of the initial API design.

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

Successfully merging this pull request may close these issues.

The initial IME candidate window position is wrong
2 participants