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 Support #2777

Merged
merged 15 commits into from
Feb 3, 2025
Merged

Conversation

kenz-gelsoft
Copy link
Contributor

@kenz-gelsoft kenz-gelsoft commented Feb 2, 2025

This is a PoC draft of my API Design proposal for CJK Input method.

https://discourse.iced.rs/t/api-design-proposal-cjk-input-method-support/862

This is not finished yet, but I believe this helps discussion on the above post.

I'll update this or create new PR if my proposal is welcomed.

(Or feel free other experienced iced developer finish or supercede my work!)


This PR implements CJK Input Method support on iced. Integrates it into TextInput and TextEditor widgets in iced_widget crate, so theoretically all examples support input method.

I tested combo_box, todos, markdown examples in my development.

Inputting complex charactes Commit them and insert to the app
スクリーンショット 2025-02-02 10 44 48 スクリーンショット 2025-02-02 10 45 23
スクリーンショット 2025-02-02 10 47 38

@kenz-gelsoft kenz-gelsoft marked this pull request as draft February 2, 2025 02:17
@rhysd
Copy link
Contributor

rhysd commented Feb 2, 2025

Seems related to #1858.

@kenz-gelsoft
Copy link
Contributor Author

Seems related to #1858.

Yes. Trying to resolve same problem with
smaller API impact.

@hecrj
Copy link
Member

hecrj commented Feb 2, 2025

Hey, thanks!

I think we can work with this. Let me make some changes.

@hecrj hecrj force-pushed the explore-input-method2 branch from 3322aee to 043efc0 Compare February 2, 2025 16:39
@hecrj hecrj marked this pull request as ready for review February 2, 2025 16:39
@hecrj hecrj added this to the 0.14 milestone Feb 2, 2025
@hecrj hecrj force-pushed the explore-input-method2 branch from 043efc0 to 0c6d4eb Compare February 2, 2025 16:50
@hecrj hecrj changed the title PoC: Implement CJK Input Method support Input Method Support Feb 2, 2025
@kenz-gelsoft
Copy link
Contributor Author

Hey, thanks!

I think we can work with this. Let me make some changes.

Thank you for reviewing. If you happy with my proposal and code, I'll update this branch.

Current PR is not ready for merge for me, I have some TODO items on this branch like:

  • It doesn't reflect input method's selection state
    • I'll split the preedit into some Paragraphs and give some styling to express selection
    • This can be implemented in another PR, but I prefer including in this.
  • It doesn't implement opt-out mechanism I described in Discourse post yet
    • shows_preedit_myself field on Preedit struct or so.
    • this is trivial but just not yet done
    • This also can be in another PR, but it changes API of the Shell twice, so I want to finish in this.

One or two minor improvements I want to do, but it may be in another PRs.

@hecrj
Copy link
Member

hecrj commented Feb 2, 2025

I'm not happy with the code. I am changing it.

We can discuss further fixes later.

@kenz-gelsoft
Copy link
Contributor Author

I'm not happy with the code. I am changing it.

We can discuss further fixes later.

OK, thank you taking time to brush up my code!

Should I wait your changes will be force-pushed on this branch? Or can I work on my another branch and show what needs to be added? I assume the latter.

@hecrj
Copy link
Member

hecrj commented Feb 2, 2025

I don't recommend you to keep working on this code, since I'm rewriting the whole API.

The changes you mention should be easy to add later on.

Wait a bit, I'm almost done.

@hecrj
Copy link
Member

hecrj commented Feb 2, 2025

Almost done, just a couple more things.

@hecrj
Copy link
Member

hecrj commented Feb 3, 2025

Alright! So I have refactored things a bit and also have implemented both of the features you mentioned, @kenz-gelsoft:

  • Widgets can opt-out of over-the-spot pre-edits by simply providing None to InputMethod::Open.
  • If they opt-in, they can provide a selection range which will be rendered either as a cursor or a selection (currently, just inverting foreground and background colors):

image
image

Far from perfect, but it's a start. Test it out and let me know!

@kenz-gelsoft
Copy link
Contributor Author

Alright! So I have refactored things a bit and also have implemented both of the features you mentioned, @kenz-gelsoft:

  • Widgets can opt-out of over-the-spot pre-edits by simply providing None to InputMethod::Open.
  • If they opt-in, they can provide a selection range which will be rendered either as a cursor or a selection (currently, just inverting foreground and background colors):

I looked through your refactoring changes. Looks great.

Opt-out by InputMethod::Open with preedit=None is clean and reasonable. Clamping preedit inside window's viewport is what I want to do!

I can't test this in my office hours, I will test this and feedback later. Please wait.

@kenz-gelsoft
Copy link
Contributor Author

kenz-gelsoft commented Feb 3, 2025

Slow redraw on typing with input method. And stop updating if we select text before starting input with input method.

image

@kenz-gelsoft
Copy link
Contributor Author

kenz-gelsoft commented Feb 3, 2025

Start typing when there is text selection before showing candidate window at the first time, then it fails to place candidate window at correct position. Because you don't pass the first character position of the first line of the selection as caret position. No, it was not the case.

This occured only on macOS. I guess this is macOS winit's bug that worked around here https://github.com/iced-rs/iced/pull/1858/files#diff-a595007c8791daf8513eded932e3a5a9cb14bd3bae92637b7b2fcd086125f56dR45 in previous PR #1858

macOS specific problem may be worked on another PR I think.

スクリーンショット 2025-02-03 23 37 35

@kenz-gelsoft
Copy link
Contributor Author

kenz-gelsoft commented Feb 3, 2025

@hecrj We need to call request_redraw() just after drawing preedit. This fixes slow redraw problem I described in #2777 (comment).

Slow redraw and stop redrawing when we have text selection is the same problem. It is redrawed only when caret blinks.

diff --git a/winit/src/program/window_manager.rs b/winit/src/program/window_manager.rs
index ae214e7c..ddd39587 100644
--- a/winit/src/program/window_manager.rs
+++ b/winit/src/program/window_manager.rs
@@ -266,6 +266,7 @@ where
                     self.state.viewport().logical_size(),
                 ),
             );
+            self.raw.request_redraw();
         }
     }
 }

@kenz-gelsoft
Copy link
Contributor Author

kenz-gelsoft commented Feb 3, 2025

Second scrollable affects to preedit position in the first scrollable. I avoided this on my branch by checking if the CaretInfo already reported before updating scrollable's child.

スクリーンショット 2025-02-03 23 56 04

My proposed fix

diff --git a/widget/src/scrollable.rs b/widget/src/scrollable.rs
index 0a93584e..00c646db 100644
--- a/widget/src/scrollable.rs
+++ b/widget/src/scrollable.rs
@@ -730,6 +730,11 @@ where
                 let translation =
                     state.translation(self.direction, bounds, content_bounds);

+                let can_request_input_method = match shell.input_method() {
+                    InputMethod::None => true,
+                    _ => false,
+                };
+
                 self.content.as_widget_mut().update(
                     &mut tree.children[0],
                     event.clone(),
@@ -745,10 +750,12 @@ where
                     },
                 );

-                if let InputMethod::Open { position, .. } =
-                    shell.input_method_mut()
-                {
-                    *position = *position + translation;
+                if can_request_input_method {
+                    if let InputMethod::Open { position, .. } =
+                        shell.input_method_mut()
+                    {
+                        *position = *position + translation;
+                    }
                 }
             };

@hecrj
Copy link
Member

hecrj commented Feb 3, 2025

We need to call request_redraw() just after drawing preedit.

We just need to call request_redraw after an IME event. I think e8c680c should fix it.

Second scrollable affects to preedit position in the first scrollable.

Good catch. This should be fixed by 141290c.

This occured only on macOS. I guess this is macOS winit's bug that worked around here https://github.com/iced-rs/iced/pull/1858/files#diff-a595007c8791daf8513eded932e3a5a9cb14bd3bae92637b7b2fcd086125f56dR45 in previous PR #1858

winit's bugs should be fixed upstream, ideally.

@kenz-gelsoft
Copy link
Contributor Author

@hecrj I confirmed two problems fixed. I couldn't find more bugs.

It is now usable enough to merge! Here we go!

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

@kenz-gelsoft Awesome! Thanks for all the work and testing!

Next step is implementing on-the-spot pre-edits, but I'm still quite happy iced is now somewhat usable for a lot more people.

Let's ship it! 🚢

@TonalidadeHidrica
Copy link

It's a great leap for iced toward truly useful real-world app library for us CJK people! Thanks for everyone involved.

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.

4 participants