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

Input method select list position is not correct #2056

Closed
chrisniael opened this issue Feb 1, 2019 · 7 comments
Closed

Input method select list position is not correct #2056

chrisniael opened this issue Feb 1, 2019 · 7 comments

Comments

@chrisniael
Copy link

MacOS Mojave 10.14.2
Alacritty v0.2.7

2019-02-01 1 34 26

@chrisduerr chrisduerr changed the title Mac input method select list position is not correct. Input method select list position is not correct Feb 2, 2019
@chrisduerr
Copy link
Member

Note this is not macOS-specific, but has been reproduced on Linux in #2061.

@chrisduerr
Copy link
Member

Also, the following patch puts the IME update after potential terminal resize events, could you let me know if this fixes the problem?

diff --git a/src/main.rs b/src/main.rs
index a5564cd..f362319 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -233,16 +233,16 @@ fn run(
 
         // Maybe draw the terminal
         if terminal_lock.needs_draw() {
-            // Try to update the position of the input method editor
-            #[cfg(not(windows))]
-            display.update_ime_position(&terminal_lock);
-
             // Handle pending resize events
             //
             // The second argument is a list of types that want to be notified
             // of display size changes.
             display.handle_resize(&mut terminal_lock, &config, &mut [&mut resize_handle, &mut processor]);
 
+            // Try to update the position of the input method editor
+            #[cfg(not(windows))]
+            display.update_ime_position(&terminal_lock);
+
             drop(terminal_lock);
 
             // Draw the current state of the terminal

@pickfire
Copy link

pickfire commented Feb 2, 2019

@chrisduerr Why not update ime position after redraw?

@chrisduerr
Copy link
Member

Redrawing shouldn't do anything special to mutate the terminal state.

If my patch doesn't work though, feel free to try that too.

@awused
Copy link
Contributor

awused commented Feb 2, 2019

@chrisduerr That patch does not fix the issue for me, the IME position is unchanged.

I quickly fixed it for myself by dividing nspot_x and nspot_y in update_ime_position by hidpi_factor.

@chrisduerr
Copy link
Member

Not taking DPI into account, sounds like a very likely cause for this problem! Thanks for looking into this @awused, unfortunately I'm completely unfamiliar with IME.

Would you mind sending a PR? I'll gladly help out with style or other issues if there still are some.

@chrisniael
Copy link
Author

chrisniael commented Feb 3, 2019

@chrisduerr , @awused 's PR code works fine for me.

chrisduerr pushed a commit that referenced this issue Feb 5, 2019
Since the IME was positioned using physical coordinates,
the location would be incorrect with monitors using a DPR
other than 1.0.

This has been resolved by converting the physical position
to a logical position using the methods built into winit.

Fixes #2056.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants