iced-rs / iced

A cross-platform GUI library for Rust, inspired by Elm
https://iced.rs
MIT License
23.06k stars 1.06k forks source link

basic IME supporting #1474

Closed KentaTheBugMaker closed 1 year ago

KentaTheBugMaker commented 1 year ago

General

This PR fix #979,#1544. iced ime support was broken due to winit 0.27 update . This PR add event to fit new winit IME event model and introduce new event support for text_input.

how to test

download app from my repo https://github.com/t18b219k/iced_text_input_sample/

Phase 1 : Can we input CJK characters?

tested on

Phase 2 : Candidate position is near the TextInput?

You maybe failed to reproduce due to IME version. candidate positon on X11 require LibX11 1.8.2 or above.

Wayland requirement.

tested on

Phase 3 draw underline and cursor

behavior

https://user-images.githubusercontent.com/48007646/205664366-ea71ddaf-a778-4f1a-9d0e-1b5cb557e70c.mp4

test

not required. We render underline and cursor correctly if winit correctly report it.

KentaTheBugMaker commented 1 year ago

I don't have apple devices please test someone .

KentaTheBugMaker commented 1 year ago

we should move IME window to correct position to provide native look and feel where we should implement this logic.

Cupnfish commented 1 year ago

Tested and does not work correctly on m1 macOS 13.0 Beta.

KentaTheBugMaker commented 1 year ago

if alacritty works well this should work as well. please test alacritty works well.

someoneinjd commented 1 year ago

On my mac the candidate box is showing but it's in the wrong place (macOS Monterey 12.6, MacBook Pro (16-inch, 2019))

截屏2022-10-18 14 20 09

In alacritty it works well. (alacritty 0.11.0)

截屏2022-10-18 14 26 31
KentaTheBugMaker commented 1 year ago

now candidate window placed near TextInput . this positioning is not best but much better than nothing,

KentaTheBugMaker commented 1 year ago

@Cupnfish @someoneinjd please test candidate window position is correct.

someoneinjd commented 1 year ago

Thanks for your hard work. It works in most cases. But when I use the input method for the first time since the application started, the position is wrong, and after that the position is correct.

截屏2022-10-20 20 34 49 截屏2022-10-20 20 35 02

Alacritty also has this problem, so it might be a winit problem?

截屏2022-10-20 20 35 45 截屏2022-10-20 20 35 56
someoneinjd commented 1 year ago

I also tested on my surface pro 6 (Arch Linux + Linux 6.0.1 + wayland 1.21.0 + fcitx5 5.0.19 + sway 1.7). And it doesn't have this problem. 20221020_20h59m10s_grim 20221020_21h00m42s_grim

Cupnfish commented 1 year ago

Thanks for your hard work. It works in most cases. But when I use the input method for the first time since the application started, the position is wrong, and after that the position is correct. 截屏2022-10-20 20 34 49 截屏2022-10-20 20 35 02 Alacritty also has this problem, so it might be a winit problem? 截屏2022-10-20 20 35 45 截屏2022-10-20 20 35 56

This is also the case on my laptop. Not only that, switching to another app and then switching back triggers the bug again.

KentaTheBugMaker commented 1 year ago

Maybe we need to notify IMEEnabled Event from IME to TextInput . I will try to fix this problem today .

KentaTheBugMaker commented 1 year ago

@Cupnfish @someoneinjd please test condidate window is correct position even first time.

someoneinjd commented 1 year ago

I tried but the same error again.

截屏2022-10-21 09 22 14

I added some log code to see whether the position was set.

diff --git a/native/src/widget/text_input.rs b/native/src/widget/text_input.rs
index f372b2ba..2ba8d06d 100644
--- a/native/src/widget/text_input.rs
+++ b/native/src/widget/text_input.rs
@@ -758,6 +758,7 @@ where
                 (text_bounds.x + position.0) as i32,
                 (text_bounds.y) as i32 + size as i32,
             );
+            println!("[IME Enabled] Set IME {:?}", position);
             ime.set_ime_position(position.0, position.1);
         }
         Event::Keyboard(keyboard::Event::IMEPreedit(text)) => {
@@ -802,6 +803,7 @@ where
                 (text_bounds.x + position.0) as i32,
                 (text_bounds.y) as i32 + size as i32,
             );
+            println!("[IME Preedit] Set IME {:?}", position);
             ime.set_ime_position(position.0, position.1);

             shell.publish(message);

And here is the output (I typed "你" first and then "好")

    Finished dev [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/todos`
2022-10-21 09:22:06.499 todos[25370:2729210] TSM AdjustCapsLockLEDForKeyTransitionHandling - _ISSetPhysicalKeyboardCapsLockLED Inhibit
[IME Enabled] Set IME (55, 205)
[IME Preedit] Set IME (55, 205)
[IME Preedit] Set IME (55, 205)
[IME Preedit] Set IME (77, 205)
[IME Preedit] Set IME (77, 205)
[IME Preedit] Set IME (77, 205)

It seems that the position was calculated correctly but set_ime_position didn't work.

KentaTheBugMaker commented 1 year ago

if we set IME position when we receive click event maybe we can achieve correct positioning.

KentaTheBugMaker commented 1 year ago

@someoneinjd @Cupnfish please test if it works. set_ime_position when TextInput clicked. If this doesn't work we need to talk with alacritty team.

Cupnfish commented 1 year ago

@someoneinjd @Cupnfish please test if it works. set_ime_position when TextInput clicked. If this doesn't work we need to talk with alacritty team.

It still comes up with that problem.

someoneinjd commented 1 year ago

Unfortunately the problem still exists

截屏2022-10-22 07 44 51
KentaTheBugMaker commented 1 year ago

shall we release this PR or not? if you fixed this problem please open PR to my repo.

someoneinjd commented 1 year ago

I find that set_ime_position in IMEEnabled doesn't work, maybe we need to set the candidate box position before receiving this event. But this may not be easy to achieve, because when I use the Chinese input method to start todos and type, IMEEnabled is the first event received by the update function. Maybe we can force the user to click the input box before they can type.

zmtq05 commented 1 year ago

image

Unfocus text_input then type korean, preedit character(expect) inserted in text_input. It can delete manually but isn't deleted automatically.

Used font: NanumGothic.ttf OS: Windows 10

KentaTheBugMaker commented 1 year ago

IME handling is very platform dependent issue so feel free to PR to my repo .

KentaTheBugMaker commented 1 year ago

@hecrj This PR is very platform issue but no one tried to improving this PR. I think it better to basic support than nothing. So i want to left task for community to improve support.

zmtq05 commented 1 year ago

I want to help, but I don't know where to start. Can I get a hint?

KentaTheBugMaker commented 1 year ago

@zmtq05 here is my repo https://github.com/t18b219k/iced it seems to me connecting to IME is ok . so I want to you to dive to native/src/widget/text_input.rs update function. before you making PR please run CI to build test binary. if you can please install CJK font for easy test.

zmtq05 commented 1 year ago

I resolve my issue by does nothing when receive preedit event and unfocused. but japanese and chinese candidate window still appear when unfocused then input. EDIT: egui(+ eframe) is same.

sorry for bad english.

Riey commented 1 year ago

Thanks for your hard work. It works in most cases. But when I use the input method for the first time since the application started, the position is wrong, and after that the position is correct.

截屏2022-10-20 20 34 49 截屏2022-10-20 20 35 02

Alacritty also has this problem, so it might be a winit problem?

截屏2022-10-20 20 35 45 截屏2022-10-20 20 35 56

Related X11 bug

https://bugs.freedesktop.org/show_bug.cgi?id=1580

KentaTheBugMaker commented 1 year ago

underlining when using IME. Bold line means this section text is converting.

https://user-images.githubusercontent.com/48007646/198844635-dd08f4e4-47b4-4ec2-8a3f-d580096cc5f4.mp4

KentaTheBugMaker commented 1 year ago

@zmtq05 Thank you for good idea. My implementation is buggy when we use IME and open many TextInputs. Your idea implemented in my latest patch. https://github.com/t18b219k/iced/commit/fbc9c595c994831e4c18030b4d9d09a2bea3a516

zmtq05 commented 1 year ago

underline position is weird. image

zmtq05 commented 1 year ago

image Cursor must be located after the character. image

KentaTheBugMaker commented 1 year ago

There is no IME event standard even in MS. Korean always returns indicator position 0,0 means cursor should place before editing character but Japanese and Chinese IME returns correctly.

KentaTheBugMaker commented 1 year ago

ibus-hangul returns correctly. ibus-pinyin emit same event as MS Korean but correctly report. ibus-mozc report not same between GTK app and Wayland textinput protocol if we apply MS Korean fix will break ibus-pinyin. but still acceptable.

zmtq05 commented 1 year ago
diff --git a/native/src/widget/text_input/ime_range.rs b/native/src/widget/text_input/ime_range.rs
index f66b8f72..a31eaef4 100644
--- a/native/src/widget/text_input/ime_range.rs
+++ b/native/src/widget/text_input/ime_range.rs
@@ -55,6 +55,8 @@ impl IMERange {
         if let Some((start, end)) = self.bold_line_range {
             if end == text.len() {
                 Some(text)
+            } else if start == 0 && end == 0 {
+                Some(text.split_at(end).1)
             } else if end == start {
                 Some(text.split_at(end).0)
             } else {

I fixed issue by this. Could you test japanese and chinese?

KentaTheBugMaker commented 1 year ago

My developing branch is ime_support. same Korean fix implemented in ime_support branch. your impl not break JP and pinyin but not tested on chewing.

KentaTheBugMaker commented 1 year ago

@hecrj please merge winit 0.27.5 to support candidate window position on x11 platform.

candidate window position setting works on fcitx5 5.0.19 + winit 0.27.5. but not works on winit 0.27.2.

KentaTheBugMaker commented 1 year ago

@hecrj I did best but there is bug in Desktop enviroment but current iced text_input status is same as alacritty and also better than egui.

zmtq05 commented 1 year ago

image Cursor position issue again. :joy:

KentaTheBugMaker commented 1 year ago

maybe reintroduced by merge conflict resolve or IMEState refactoring.

KentaTheBugMaker commented 1 year ago

please someone test macos japanese IME to find bug in underline.

KentaTheBugMaker commented 1 year ago

@Cupnfish I made some change to this PR. please test underlining if you can input Japanese please test boldline is correct.

KentaTheBugMaker commented 1 year ago

@someoneinjd @Cupnfish IME enable and disable logic changed to support Password input . Windows and linux works correctly but macos is correctly working? if macos correctly working we can merge this PR.

someoneinjd commented 1 year ago

https://user-images.githubusercontent.com/61791143/206238705-05654eb8-8303-4ca4-b69c-d4a68c5613f9.mov

looks like it works fine on intel macOS Ventura 13.0.1

KentaTheBugMaker commented 1 year ago

ok it seems ready to merge. @hecrj All platforms works fine. please merge !

Drodofsky commented 1 year ago

I am using the latest version of Pop os with Mozc-ibus (version 2.26.4220.102) as the input method. Screenshot from 2022-12-11 00-47-18

KentaTheBugMaker commented 1 year ago

@Drodofsky IBus mozc can display preedit but can't move Candidate window near the TextInput due to libx11 limitation. very recently libx11 merge request enable placing candidate window to user specified position. but ibus ,fcitx, and winit update needed to support it. PopOS/Ubuntu 22.04's libx11 version is 1.7.5 but minimum supported version is 1.8.2.

KentaTheBugMaker commented 1 year ago

@Drodofsky if you can try Manjaro , this issue fixed but unfortunately no ibus-mozc only ibus-anthy.

Drodofsky commented 1 year ago

@Drodofsky if you can try Manjaro , this issue fixed but unfortunately no ibus-mozc only ibus-anthy.

Thank you. It works on Wayland. Unfortunately, Wayland is not yet stable on Pop os.

Drodofsky commented 1 year ago

Closes #1544

KentaTheBugMaker commented 1 year ago

fortunately close bug #1544 but why #1544 closed?

Drodofsky commented 1 year ago

Winit generates no event when you hit ^, and IME is disabled.