mirror of
https://github.com/emilk/egui.git
synced 2026-08-29 04:40:03 -04:00
Delegate handling of IME interruptions to integrations to fix virtual keyboard flickering on web (#8078)
* Closes N/A * Partially replaces #7983 * Related: #8045 * [x] I have followed the instructions in the PR template ## Details In #7983, I modified `Memory::request_focus` to interrupt any ongoing IME composition. This fixed a bug where clicking inside an already focused `TextEdit` failed to cancel the active composition, resulting in duplicated text: https://github.com/emilk/egui/pull/8045#issuecomment-4193310616 To avoid introducing API changes in that PR, I ensured the IME state was reset by forcing `PlatformOutput::ime` to `None` for at least one frame. While this works well on desktop platforms, it causes virtual keyboard flickering on the web: https://github.com/emilk/egui/pull/8045#issuecomment-4193035008 In this PR, I delegate the responsibility for handling IME composition interruptions to integrations, allowing each integration to decide how to interrupt compositions in a flexible manner. ### The new field `should_interrupt_composition` on `IMEOutput`. Instead of introducing a new `OutputCommand` variant, this PR adds a new field `should_interrupt_composition` to `IMEOutput`. Interrupting an active composition is only meaningful when IME remains allowed. If IME should be disabled altogether, `PlatformOutput::ime` can simply be set to `None`. Given this, IMO, it is more appropriate to attach the interrupt signal to `IMEOutput` (i.e., the type of `PlatformOutput::ime`).
This commit is contained in:
@@ -56,8 +56,13 @@ impl TextAgent {
|
|||||||
let input = input.clone();
|
let input = input.clone();
|
||||||
move |event: web_sys::InputEvent, runner: &mut AppRunner| {
|
move |event: web_sys::InputEvent, runner: &mut AppRunner| {
|
||||||
let text = input.value();
|
let text = input.value();
|
||||||
// Fix android virtual keyboard Gboard
|
// Workaround for an Android Gboard issue: after typing a word,
|
||||||
// This removes the virtual keyboard's suggestion.
|
// the user has to delete invisible characters (whose count
|
||||||
|
// matches the length of the current suggestion) before actual
|
||||||
|
// characters are deleted, unless the focus has been reset.
|
||||||
|
//
|
||||||
|
// this issue appears to have been fixed in Gboard sometime
|
||||||
|
// between versions 14.7.09 and 17.0.12.
|
||||||
if !event.is_composing() {
|
if !event.is_composing() {
|
||||||
input.blur().ok();
|
input.blur().ok();
|
||||||
input.focus().ok();
|
input.focus().ok();
|
||||||
@@ -132,6 +137,12 @@ impl TextAgent {
|
|||||||
|
|
||||||
let Some(ime) = ime else { return Ok(()) };
|
let Some(ime) = ime else { return Ok(()) };
|
||||||
|
|
||||||
|
if ime.should_interrupt_composition {
|
||||||
|
// no-op for now: currently, the text agent is sizeless, so any
|
||||||
|
// click shifts focus to the canvas, which naturally interrupts the
|
||||||
|
// composition.
|
||||||
|
}
|
||||||
|
|
||||||
let mut canvas_rect = super::canvas_content_rect(canvas);
|
let mut canvas_rect = super::canvas_content_rect(canvas);
|
||||||
// Fix for safari with virtual keyboard flapping position
|
// Fix for safari with virtual keyboard flapping position
|
||||||
if is_mobile_safari() {
|
if is_mobile_safari() {
|
||||||
|
|||||||
@@ -1049,7 +1049,8 @@ impl State {
|
|||||||
self.set_cursor_icon(window, cursor_icon);
|
self.set_cursor_icon(window, cursor_icon);
|
||||||
|
|
||||||
let allow_ime = ime.is_some();
|
let allow_ime = ime.is_some();
|
||||||
if self.allow_ime != allow_ime {
|
let is_toggling_ime = self.allow_ime != allow_ime;
|
||||||
|
if is_toggling_ime {
|
||||||
self.allow_ime = allow_ime;
|
self.allow_ime = allow_ime;
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
if !self.allow_ime {
|
if !self.allow_ime {
|
||||||
@@ -1066,6 +1067,14 @@ impl State {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if let Some(ime) = ime {
|
if let Some(ime) = ime {
|
||||||
|
if !is_toggling_ime && ime.should_interrupt_composition {
|
||||||
|
// TODO(umajho): use a more proper way to interrupt composition
|
||||||
|
// if `winit` provides one in the future.
|
||||||
|
|
||||||
|
window.set_ime_allowed(false);
|
||||||
|
window.set_ime_allowed(true);
|
||||||
|
}
|
||||||
|
|
||||||
let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
|
let pixels_per_point = pixels_per_point(&self.egui_ctx, window);
|
||||||
let ime_rect_px = pixels_per_point * ime.rect;
|
let ime_rect_px = pixels_per_point * ime.rect;
|
||||||
if self.ime_rect_px != Some(ime_rect_px)
|
if self.ime_rect_px != Some(ime_rect_px)
|
||||||
|
|||||||
@@ -2612,6 +2612,12 @@ impl ContextImpl {
|
|||||||
|
|
||||||
let mut platform_output: PlatformOutput = std::mem::take(&mut viewport.output);
|
let mut platform_output: PlatformOutput = std::mem::take(&mut viewport.output);
|
||||||
|
|
||||||
|
if self.memory.should_interrupt_ime()
|
||||||
|
&& let Some(ime) = &mut platform_output.ime
|
||||||
|
{
|
||||||
|
ime.should_interrupt_composition = true;
|
||||||
|
}
|
||||||
|
|
||||||
{
|
{
|
||||||
profiling::scope!("accesskit");
|
profiling::scope!("accesskit");
|
||||||
let state = viewport.this_pass.accesskit_state.take();
|
let state = viewport.this_pass.accesskit_state.take();
|
||||||
|
|||||||
@@ -79,6 +79,9 @@ pub struct IMEOutput {
|
|||||||
///
|
///
|
||||||
/// This is a very thin rectangle.
|
/// This is a very thin rectangle.
|
||||||
pub cursor_rect: crate::Rect,
|
pub cursor_rect: crate::Rect,
|
||||||
|
|
||||||
|
/// Whether any ongoing IME composition should be interrupted.
|
||||||
|
pub should_interrupt_composition: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Commands that the egui integration should execute at the end of a frame.
|
/// Commands that the egui integration should execute at the end of a frame.
|
||||||
|
|||||||
@@ -117,21 +117,10 @@ pub struct Memory {
|
|||||||
#[cfg_attr(feature = "persistence", serde(skip))]
|
#[cfg_attr(feature = "persistence", serde(skip))]
|
||||||
popups: ViewportIdMap<OpenPopup>,
|
popups: ViewportIdMap<OpenPopup>,
|
||||||
|
|
||||||
/// When the last IME interruption was made.
|
/// Whether to inform the backend to interrupt any ongoing IME composition
|
||||||
|
/// this pass.
|
||||||
#[cfg_attr(feature = "persistence", serde(skip))]
|
#[cfg_attr(feature = "persistence", serde(skip))]
|
||||||
ime_interruption_time: ImeInterruptionTime,
|
requested_interrupt_ime: bool,
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Clone, Copy, Debug, Default)]
|
|
||||||
enum ImeInterruptionTime {
|
|
||||||
#[default]
|
|
||||||
None,
|
|
||||||
|
|
||||||
/// The IME was interrupted in the current frame.
|
|
||||||
ThisFrame,
|
|
||||||
|
|
||||||
/// The IME was interrupted in the previous frame.
|
|
||||||
LastFrame,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for Memory {
|
impl Default for Memory {
|
||||||
@@ -149,7 +138,7 @@ impl Default for Memory {
|
|||||||
popups: Default::default(),
|
popups: Default::default(),
|
||||||
everything_is_visible: Default::default(),
|
everything_is_visible: Default::default(),
|
||||||
add_fonts: Default::default(),
|
add_fonts: Default::default(),
|
||||||
ime_interruption_time: Default::default(),
|
requested_interrupt_ime: Default::default(),
|
||||||
};
|
};
|
||||||
slf.interactions.entry(slf.viewport_id).or_default();
|
slf.interactions.entry(slf.viewport_id).or_default();
|
||||||
slf.areas.entry(slf.viewport_id).or_default();
|
slf.areas.entry(slf.viewport_id).or_default();
|
||||||
@@ -778,15 +767,7 @@ impl Memory {
|
|||||||
|
|
||||||
self.areas.entry(self.viewport_id).or_default();
|
self.areas.entry(self.viewport_id).or_default();
|
||||||
|
|
||||||
match self.ime_interruption_time {
|
self.requested_interrupt_ime = false;
|
||||||
ImeInterruptionTime::ThisFrame => {
|
|
||||||
self.ime_interruption_time = ImeInterruptionTime::LastFrame;
|
|
||||||
}
|
|
||||||
ImeInterruptionTime::LastFrame => {
|
|
||||||
self.ime_interruption_time = ImeInterruptionTime::None;
|
|
||||||
}
|
|
||||||
ImeInterruptionTime::None => {}
|
|
||||||
}
|
|
||||||
|
|
||||||
// self.interactions is handled elsewhere
|
// self.interactions is handled elsewhere
|
||||||
|
|
||||||
@@ -1028,30 +1009,22 @@ impl Memory {
|
|||||||
///
|
///
|
||||||
/// A widget should only consume IME events if this returns `true`. At most
|
/// A widget should only consume IME events if this returns `true`. At most
|
||||||
/// one widget can own IME events for each frame.
|
/// one widget can own IME events for each frame.
|
||||||
|
#[inline(always)]
|
||||||
pub fn owns_ime_events(&self, id: Id) -> bool {
|
pub fn owns_ime_events(&self, id: Id) -> bool {
|
||||||
let Some(focus) = self.focus() else {
|
// Note: Even if the IME is being interrupted in the current frame, we
|
||||||
return false;
|
// should not return `false` here, since we still need
|
||||||
};
|
// `PlatformOutput::ime` to be set in such cases.
|
||||||
// We check across two frames because the widget that called
|
|
||||||
// `interrupt_ime` may run after other widgets that call this method
|
self.has_focus(id)
|
||||||
// within the same frame.
|
|
||||||
if matches!(
|
|
||||||
self.ime_interruption_time,
|
|
||||||
ImeInterruptionTime::ThisFrame | ImeInterruptionTime::LastFrame
|
|
||||||
) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
focus.focused() == Some(id)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Interrupt the current IME composition, if any.
|
/// Interrupt the current IME composition, if any.
|
||||||
///
|
|
||||||
/// This causes [`Self::owns_ime_events`] to return `false` for all widgets
|
|
||||||
/// for the remainder of this frame and the next frame, giving time
|
|
||||||
/// for the IME to be dismissed (by making `platform_output.ime` be `None`
|
|
||||||
/// for at least one frame).
|
|
||||||
pub fn interrupt_ime(&mut self) {
|
pub fn interrupt_ime(&mut self) {
|
||||||
self.ime_interruption_time = ImeInterruptionTime::ThisFrame;
|
self.requested_interrupt_ime = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn should_interrupt_ime(&self) -> bool {
|
||||||
|
self.requested_interrupt_ime
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -871,6 +871,7 @@ impl TextEdit<'_> {
|
|||||||
o.ime = Some(crate::output::IMEOutput {
|
o.ime = Some(crate::output::IMEOutput {
|
||||||
rect: to_global * inner_rect,
|
rect: to_global * inner_rect,
|
||||||
cursor_rect: to_global * primary_cursor_rect,
|
cursor_rect: to_global * primary_cursor_rect,
|
||||||
|
should_interrupt_composition: false,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user