Skip to content

Fix: Ensures correct IME behavior when the text input area gains or loses focus.#4896

Merged
emilk merged 9 commits intoemilk:masterfrom
rustbasic:patch92
Aug 28, 2024
Merged

Fix: Ensures correct IME behavior when the text input area gains or loses focus.#4896
emilk merged 9 commits intoemilk:masterfrom
rustbasic:patch92

Conversation

@rustbasic
Copy link
Copy Markdown
Contributor

@rustbasic rustbasic commented Aug 1, 2024

Fix: Ensures correct IME behavior when the text input area gains or loses focus.

Fix: Handling state.ime_enabled in multiple TextEdit.
Fix: A symptom of characters being copied when there are multiple TextEdits.

Fix Issues: When focus is moved elsewhere, you must set state.ime_enabled = false, otherwise the IME will have problems when focus returns.

Fix Issues: A symptom of characters being copied when there are multiple TextEdits.
Deletes all current IME events, preventing them from being copied to other TextEdits, without saving the TextEdit ID,

( Related Issues: Some LINUX seem to trigger an IME enable event on startup. So, when we gained focus, we do state.ime_enabled = false. )

}
}

if state.ime_enabled && response.lost_focus() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couldnt you just leave the state.ime_enabled` ? Because if its true its getting set to false and else it is already false, so to me it seems like an unnecessary condition

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Couldnt you just leave the state.ime_enabled` ? Because if its true its getting set to false and else it is already false, so to me it seems like an unnecessary condition

When focus is moved elsewhere, you must set ime_enabled to 'false`, otherwise the IME will have problems when focus returns.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sure the idea was the following

if response.lost_focus() {
state.ime_enabled = false;
}

Copy link
Copy Markdown
Contributor Author

@rustbasic rustbasic Aug 3, 2024

Choose a reason for hiding this comment

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

sure the idea was the following

if response.lost_focus() {
state.ime_enabled = false;
}

Yes, I thought about this too. If ime_enabled is not true, there is no need to check lost_focus() , so there is a difference in speed and readability. Let's wait for emilk's opinion on which one is better.

thank you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok but if so it would be good to add a comment there anyway

@rustbasic rustbasic changed the title Fix: Handling state.ime_enabled in multiple TextEdit. Fix: IME-related processing when focus is lost in IME enabled state. Aug 4, 2024
emilk pushed a commit that referenced this pull request Aug 5, 2024
Fix: Changed the handling method of `Ime::Preedit(_, None)`

Fix: backspace fail after ime input

* Related #4358
* Related #4430 
* Related #4436
* Related #4794 
* Related #4896
* Closes #4908 

Issues: backspace fail after ime input
* #4908 (Chinese)

Changed the handling method of `Ime::Preedit(_, None)`
@rustbasic rustbasic changed the title Fix: IME-related processing when focus is lost in IME enabled state. Fix: IME-related processing when focus is gained or lost in IME enabled state. Aug 27, 2024
@rustbasic rustbasic changed the title Fix: IME-related processing when focus is gained or lost in IME enabled state. Fix: Ensures correct IME behavior when the text input area gains or loses focus. Aug 27, 2024
@emilk emilk added the IME label Aug 27, 2024
@emilk emilk added the egui label Aug 28, 2024
@emilk emilk merged commit 8e5492b into emilk:master Aug 28, 2024
rib pushed a commit to EmbarkStudios/egui that referenced this pull request Sep 30, 2024
Fix: Changed the handling method of `Ime::Preedit(_, None)`

Fix: backspace fail after ime input

* Related emilk#4358
* Related emilk#4430 
* Related emilk#4436
* Related emilk#4794 
* Related emilk#4896
* Closes emilk#4908 

Issues: backspace fail after ime input
* emilk#4908 (Chinese)

Changed the handling method of `Ime::Preedit(_, None)`
486c pushed a commit to 486c/egui that referenced this pull request Oct 9, 2024
Fix: Changed the handling method of `Ime::Preedit(_, None)`

Fix: backspace fail after ime input

* Related emilk#4358
* Related emilk#4430 
* Related emilk#4436
* Related emilk#4794 
* Related emilk#4896
* Closes emilk#4908 

Issues: backspace fail after ime input
* emilk#4908 (Chinese)

Changed the handling method of `Ime::Preedit(_, None)`
486c pushed a commit to 486c/egui that referenced this pull request Oct 9, 2024
…oses focus. (emilk#4896)

Fix: Ensures correct IME behavior when the text input area gains or
loses focus.

Fix: Handling `state.ime_enabled` in multiple `TextEdit`.
Fix: A symptom of characters being copied when there are multiple
TextEdits.

* Related emilk#4137
* Related emilk#4358 
* Closes emilk#4374
* Related emilk#4436
* Related emilk#4794 
* Related emilk#4908 

* Related emilk#5008

Fix Issues: When focus is moved elsewhere, you must set
`state.ime_enabled = false`, otherwise the IME will have problems when
focus returns.

Fix Issues: A symptom of characters being copied when there are multiple
TextEdits.
Deletes all current `IME events`, preventing them from being copied to
`other TextEdits`, without saving the `TextEdit ID`,

( Related Issues: Some `LINUX` seem to trigger an IME enable event on
startup. So, when we gained focus, we do `state.ime_enabled = false`. )
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
Fix: Changed the handling method of `Ime::Preedit(_, None)`

Fix: backspace fail after ime input

* Related emilk#4358
* Related emilk#4430 
* Related emilk#4436
* Related emilk#4794 
* Related emilk#4896
* Closes emilk#4908 

Issues: backspace fail after ime input
* emilk#4908 (Chinese)

Changed the handling method of `Ime::Preedit(_, None)`
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…oses focus. (emilk#4896)

Fix: Ensures correct IME behavior when the text input area gains or
loses focus.

Fix: Handling `state.ime_enabled` in multiple `TextEdit`.
Fix: A symptom of characters being copied when there are multiple
TextEdits.

* Related emilk#4137
* Related emilk#4358 
* Closes emilk#4374
* Related emilk#4436
* Related emilk#4794 
* Related emilk#4908 

* Related emilk#5008

Fix Issues: When focus is moved elsewhere, you must set
`state.ime_enabled = false`, otherwise the IME will have problems when
focus returns.

Fix Issues: A symptom of characters being copied when there are multiple
TextEdits.
Deletes all current `IME events`, preventing them from being copied to
`other TextEdits`, without saving the `TextEdit ID`,

( Related Issues: Some `LINUX` seem to trigger an IME enable event on
startup. So, when we gained focus, we do `state.ime_enabled = false`. )
emilk pushed a commit that referenced this pull request Apr 6, 2026
…#7983)

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/main/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* Depends on #7967
* Closes #7485
* Should fix #7906 (This issue doesn't seem to have been resolved, but
the author closed it; I personally don't have the environment to verify
whether it is fixed.)
* Replaces #4137, #4896, and partially #7810
* [x] I have followed the instructions in the PR template

This PR started as a fix for #7485, but has since evolved into a broader
rewrite of IME-related logic.

## Overview

This PR primarily introduces a new public method, `owns_ime_events`, on
[`Memory`], and refactors parts of [`TextEdit`] to integrate with it.

Previously, each [`TextEdit`] widget independently determined whether to
handle IME events and stored its own IME-related state. This approach
made ownership-handling fragmented and was therefore error-prone.

With this PR:
- IME event ownership is centralized, ensuring that at most a single
widget owns IME events per frame.
- [`PlatformOutput`]'s `ime` field can be set to `None` for at least one
frame when IME composition is interrupted, allowing the IME to be
properly dismissed.

## Details

Two new public methods are introduced on [`Memory`]:

- `fn owns_ime_events(&self, id: Id) -> bool`: check IME event ownership
for the current frame for the widget with the given `id`.
- `fn interrupt_ime(&mut self)`: interrupt the current IME composition,
if any.

Since the newly added methods on [`Memory`] are public, other widgets
can also participate in IME handling without risking ownership conflicts
of IME events.

I also added an internal (`pub(crate)`) field on [`TextEditState`],
called `cursor_purpose`, to distinguish the role of the [`TextEdit`]
cursor.

Additionally, `egui::ImeEvent::Enabled` and `egui::ImeEvent::Disabled`
have been removed, as they are no longer used anywhere.

## Demonstrations

### Windows: The Korean IME text duplication bug fixed in #4137 does not
reappear.

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Correct (no regression)</td>
<td>Correct</td>
</tr>
<tr>
<th>Screencast</th>
<td>


![win-kor-after](https://github.com/user-attachments/assets/1b080c8f-2031-406f-8781-aacafd5c879a)
</td>
<td>


![win-kor-before](https://github.com/user-attachments/assets/20258841-72fe-4652-b9a9-9b40e338ccf2)
</td>
</tr>
</tbody>
</table>

### Windows: Chinese and Japanese IMEs now behave more consistently with
the Korean IME in similar scenarios.

This change does not matter much, as composition is rarely interrupted
mid-process with these IMEs in typical usage.

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Composition can be interrupted by clicking (like Korean IMEs)</td>
<td>Composition can not interrupted by clicking</td>
</tr>
<tr>
<th>Screencast (Builtin Chinese IME)</th>
<td>


![win-cmn-after](https://github.com/user-attachments/assets/2c76b0a9-da6d-48e1-84e0-47d9631f1196)
</td>
<td>


![win-cmn-before](https://github.com/user-attachments/assets/ea125fb8-c325-48d5-abaf-17d495b8f075)
</td>
</tr>
<tr>
<th>Screencast (Builtin Japanese IME)</th>
<td>


![win-jpn-after](https://github.com/user-attachments/assets/c69e5f48-65b1-4c0f-af4a-522d2f47b75d)
</td>
<td>


![win-jpn-before](https://github.com/user-attachments/assets/a0f1fdad-4f6c-40c2-af57-029f42acf6d5)
</td>
</tr>
</tbody>
</table>

### macOS: was buggy, still buggy

Likely due to this upstream bug in `winit`:
rust-windowing/winit#4432
Once `winit` is updated to a version that includes the fix, the behavior
should become correct with this PR.

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Buggy as before</td>
<td>Buggy: Characters are duplicated</td>
</tr>
<tr>
<th>Screencast</th>
<td>


![mac-kor-after](https://github.com/user-attachments/assets/c2bd90e8-e473-49c8-9537-c970c92889bf)
</td>
<td>


![mac-kor-before](https://github.com/user-attachments/assets/63b6cd8a-8903-4743-98bf-ee15296354ba)
</td>
</tr>
</tbody>
</table>

### Wayland + iBus: Korean IME duplication bug fixed

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Correct</td>
<td>Buggy: Characters are duplicated</td>
</tr>
<tr>
<th>Screencast</th>
<td>


![wayland-kor-after-2](https://github.com/user-attachments/assets/b154add5-a1ce-4e3a-b243-e72480820c1b)
</td>
<td>


![wayland-kor-before-2](https://github.com/user-attachments/assets/43b28374-f273-4b6f-9845-3efd96ec9a37)
</td>
</tr>
</tbody>
</table>

### Wayland + iBus: #7485 is fixed

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Correct</td>
<td>Buggy: Only a single ASCII character can be typed after
<code>TextEdit</code> is focused</td>
</tr>
<tr>
<th>Screencast</th>
<td>


![wayland-7485-after](https://github.com/user-attachments/assets/ec33a54d-1d4e-40f9-8c82-202104bd2d85)
</td>
<td>


![wayland-7485-before](https://github.com/user-attachments/assets/20d2d395-03fd-4966-a376-87249a41aab3)
</td>
</tr>
</tbody>
</table>

### Wayland + iBus: selection is also not broken

This PR does not reintroduce the selection bug fixed in #7973.

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Behavior</th>
<td>Correct</td>
</tr>
<tr>
<th>Screencast</th>
<td>


![wayland-focus-after](https://github.com/user-attachments/assets/daa29197-f7f7-4a7b-b454-c28ee9afa9c1)
</td>
</tr>
</tbody>
</table>

### X11 + Fcitx5: IME composition can be interrupted

But due to #7975, the experience is still subpar. (Uncommitted text is
lost after interruption.)

<table>
<thead>
<tr>
<th></th>
<th>With this PR</th>
<th>Without this PR</th>
</tr>
</thead>
<tbody>
<tr>
<th>Screencast</th>
<td>


![x11-after](https://github.com/user-attachments/assets/e626d9ed-89a2-4825-9cde-3a67723bcb82)
</td>
<td>


![x11-before](https://github.com/user-attachments/assets/da93b351-9488-4da9-aa56-b64190e84ec3)
</td>
</tr>
</tbody>
</table>


[`Memory`]: https://docs.rs/egui/latest/egui/struct.Memory.html
[`TextEdit`]:
https://docs.rs/egui/latest/egui/widgets/text_edit/struct.TextEdit.html
[`PlatformOutput`]:
https://docs.rs/egui/latest/egui/struct.PlatformOutput.html
[`TextEditState`]:
https://docs.rs/egui/latest/egui/widgets/text_edit/struct.TextEditState.html
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.

3 participants