feat(windows): Add IME support for CJK text input by gqf2008 · Pull Request #591 · not-fl3/miniquad

gqf2008 · 2025-11-30T14:22:17Z

Fix IME initialization by moving ShowWindow() after WGL context creation
Add window::set_ime_position(x, y) API for IME candidate window positioning
Add window::set_ime_enabled(enabled) API to toggle IME for text input vs game controls
Handle WM_IME_COMPOSITION, WM_IME_STARTCOMPOSITION, WM_IME_ENDCOMPOSITION messages
Properly manage IME context on window focus changes
Add placeholder implementations for Linux, macOS, iOS, and Android
Add ime_test.rs example demonstrating IME input with text rendering

This enables proper Chinese/Japanese/Korean text input on Windows, which was previously broken due to IME not initializing correctly when the window was shown before the OpenGL context was created.

- Fix IME initialization by moving ShowWindow() after WGL context creation - Add window::set_ime_position(x, y) API for IME candidate window positioning - Add window::set_ime_enabled(enabled) API to toggle IME for text input vs game controls - Handle WM_IME_COMPOSITION, WM_IME_STARTCOMPOSITION, WM_IME_ENDCOMPOSITION messages - Properly manage IME context on window focus changes - Add placeholder implementations for Linux, macOS, iOS, and Android - Add ime_test.rs example demonstrating IME input with text rendering This enables proper Chinese/Japanese/Korean text input on Windows, which was previously broken due to IME not initializing correctly when the window was shown before the OpenGL context was created.

Copilot

Pull request overview

This PR adds Input Method Editor (IME) support for Chinese, Japanese, and Korean text input on Windows, with placeholder implementations for other platforms. The key change is fixing IME initialization by deferring ShowWindow() until after the WGL context is created, which allows the IME subsystem to properly initialize when the window gains focus.

Key Changes

Fixed IME initialization timing by moving ShowWindow() to after WGL context creation
Added two new public APIs: window::set_ime_position(x, y) for positioning the IME candidate window and window::set_ime_enabled(enabled) for toggling IME on/off
Implemented comprehensive Windows message handling for IME events (WM_IME_COMPOSITION, WM_IME_STARTCOMPOSITION, WM_IME_ENDCOMPOSITION, WM_SETFOCUS, WM_KILLFOCUS)

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
src/native/windows.rs	Core IME implementation with FFI bindings, message handlers, position control, and window creation timing fix
src/native/macos.rs	Placeholder IME API handlers (no-op implementations)
src/native/linux_x11.rs	Placeholder IME API handlers (no-op implementations)
src/native/ios.rs	Placeholder IME API handlers (no-op implementations)
src/native/android.rs	Placeholder IME API handlers (no-op implementations)
src/native.rs	Added SetImePosition and SetImeEnabled variants to Request enum
src/lib.rs	Added public `set_ime_position` and `set_ime_enabled` functions to window module
examples/ime_test.rs	Comprehensive example demonstrating IME text input with fontdue-based text rendering
Cargo.toml	Added winapi "imm" feature and fontdue dev-dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-30T14:26:51Z

src/native/windows.rs

+        }
+        WM_IME_COMPOSITION => {
+            let flags = lparam as u32;
+            const GCS_RESULTSTR: u32 = 0x800;


The GCS_RESULTSTR constant is defined twice - once at line 29 and again at line 593. The duplicate definition at line 593 should be removed and use the constant defined at the top of the file instead.

Suggested change

const GCS_RESULTSTR: u32 = 0x800;

Copilot · 2025-11-30T14:26:51Z

src/native/windows.rs

+}
+
+// IME Association flags
+const IACE_CHILDREN: DWORD = 0x0001;


The constant IACE_CHILDREN is defined at line 90 but never used in the code. Consider removing it or if it's intended for future use, add a comment explaining its purpose.

Suggested change

const IACE_CHILDREN: DWORD = 0x0001;

Copilot · 2025-11-30T14:26:51Z

src/native/windows.rs

+                        ImmAssociateContext(hwnd, new_himc);
+                        ImmSetOpenStatus(new_himc, 1);


Potential resource leak: When creating a new IME context at line 779, if ImmAssociateContext succeeds but ImmSetOpenStatus fails, the context is never released. Consider calling ImmReleaseContext after ImmSetOpenStatus, or better yet, check the return values and handle failures appropriately.

Suggested change

ImmAssociateContext(hwnd, new_himc);

ImmSetOpenStatus(new_himc, 1);

let assoc_result = ImmAssociateContext(hwnd, new_himc);

let set_status_result = ImmSetOpenStatus(new_himc, 1);

if assoc_result == 0 || set_status_result == 0 {

// If either association or open status fails, release the context

ImmReleaseContext(hwnd, new_himc);

}

Copilot · 2025-11-30T14:26:52Z

src/native/windows.rs

+                    let cand_form = CANDIDATEFORM {
+                        dwIndex: i,
+                        dwStyle: CFS_CANDIDATEPOS,
+                        ptCurrentPos: POINT { x: pt.x, y: pt.y + 20 },


[nitpick] The hard-coded offset y: pt.y + 20 for candidate window positioning at line 658 uses a magic number. Consider defining this as a named constant (e.g., const CANDIDATE_WINDOW_OFFSET: i32 = 20;) to make the code more maintainable and document the purpose of this offset.

Copilot · 2025-11-30T14:26:52Z

src/native/windows.rs

+            // Pass to DefWindowProc for IME to work properly
+            return DefWindowProcW(hwnd, umsg, wparam, lparam);
        }
        WM_KEYUP | WM_SYSKEYUP => {
            let keycode = HIWORD(lparam as _) as u32 & 0x1FF;
            let keycode = keycodes::translate_keycode(keycode);
            let mods = key_mods();
            event_handler.key_up_event(keycode, mods);
+            // IMPORTANT: Pass to DefWindowProc for IME to work properly
+            return DefWindowProcW(hwnd, umsg, wparam, lparam);


[nitpick] The WM_KEYDOWN and WM_KEYUP handlers now return the result of DefWindowProcW instead of 0. This changes the previous behavior where these messages were marked as handled (by returning 0). While this change is necessary for IME to work, it could potentially affect other keyboard handling. Ensure that returning DefWindowProcW result doesn't cause issues with key event propagation or handling in the rest of the application.

Copilot · 2025-11-30T14:26:53Z

src/native/windows.rs

+            ImmSetCompositionWindow(himc, &comp_form);
+
+            // Set candidate window position (the popup with character choices)
+            let cand_form = CANDIDATEFORM {
+                dwIndex: 0,
+                dwStyle: CFS_CANDIDATEPOS,
+                ptCurrentPos: POINT { x, y },
+                rcArea: std::mem::zeroed(),
+            };
+            ImmSetCandidateWindow(himc, &cand_form);


[nitpick] Missing error handling: If ImmSetCompositionWindow or ImmSetCandidateWindow fail, there's no indication to the caller. While silent failure might be acceptable for optional positioning hints to the IME, consider logging failures for debugging purposes.

Copilot · 2025-11-30T14:26:53Z

src/native/windows.rs

+            // Set candidate window position when IME starts composition
+            let himc = ImmGetContext(hwnd);
+            if !himc.is_null() {
+                let mut pt = POINT { x: 100, y: 100 };


[nitpick] The hard-coded default position POINT { x: 100, y: 100 } at line 643 will be immediately overwritten by GetCaretPos. This default value serves no purpose and could be confusing. Consider initializing with std::mem::zeroed() or the actual caret position without a default.

Suggested change

let mut pt = POINT { x: 100, y: 100 };

let mut pt: POINT = unsafe { std::mem::zeroed() };

Copilot · 2025-11-30T14:26:54Z

src/native/windows.rs

+                // Set candidate window position for all 4 possible candidate windows
+                for i in 0..4 {
+                    let cand_form = CANDIDATEFORM {
+                        dwIndex: i,
+                        dwStyle: CFS_CANDIDATEPOS,
+                        ptCurrentPos: POINT { x: pt.x, y: pt.y + 20 },
+                        rcArea: RECT { left: 0, top: 0, right: 0, bottom: 0 },
+                    };
+                    ImmSetCandidateWindow(himc, &cand_form);
+                }


[nitpick] Setting candidate window position for indices 0-3 in a loop assumes there are always 4 candidate windows, but only one candidate window (index 0) is typically used by most IMEs. The loop iterations for indices 1-3 are likely unnecessary and could be removed to simplify the code, or at minimum should have a comment explaining why all 4 are set.

Suggested change

// Set candidate window position for all 4 possible candidate windows

for i in 0..4 {

let cand_form = CANDIDATEFORM {

dwIndex: i,

dwStyle: CFS_CANDIDATEPOS,

ptCurrentPos: POINT { x: pt.x, y: pt.y + 20 },

rcArea: RECT { left: 0, top: 0, right: 0, bottom: 0 },

};

ImmSetCandidateWindow(himc, &cand_form);

}

// Set candidate window position for candidate window index 0 (most IMEs only use index 0)

let cand_form = CANDIDATEFORM {

dwIndex: 0,

dwStyle: CFS_CANDIDATEPOS,

ptCurrentPos: POINT { x: pt.x, y: pt.y + 20 },

rcArea: RECT { left: 0, top: 0, right: 0, bottom: 0 },

};

ImmSetCandidateWindow(himc, &cand_form);

Copilot · 2025-11-30T14:26:54Z

examples/ime_test.rs

+        if self.cursor_y + m.height as u32 > self.atlas_size {
+            eprintln!("Atlas full!");
+            return;
+        }
+
+        for y in 0..m.height {
+            for x in 0..m.width {
+                let src = y * m.width + x;
+                let dx = self.cursor_x + x as u32;
+                let dy = self.cursor_y + y as u32;
+                let dst = ((dy * self.atlas_size + dx) * 4) as usize;
+                self.atlas_data[dst..dst+3].copy_from_slice(&[255, 255, 255]);
+                self.atlas_data[dst + 3] = bmp[src];
+            }
+        }
+
+        let s = self.atlas_size as f32;
+        self.cache.insert(ch, GlyphInfo {
+            uv: [self.cursor_x as f32 / s, self.cursor_y as f32 / s,
+                 m.width as f32 / s, m.height as f32 / s],
+            size: [m.width as f32, m.height as f32],
+            offset: [m.xmin as f32, m.ymin as f32],
+            advance: m.advance_width,
+        });
+
+        self.cursor_x += m.width as u32 + 1;
+        self.row_h = self.row_h.max(m.height as u32);
+        self.dirty = true;


The dirty flag is set to true when caching a character but there's no bounds checking to prevent the atlas from overflowing. While line 78-81 checks if the atlas is full and prints an error, the dirty flag has already been set to true at line 105, and the GlyphInfo is still inserted into the cache even when the atlas is full. This could lead to rendering issues or confusion. Consider setting dirty only after successful atlas insertion.

Copilot · 2025-11-30T14:26:54Z

src/native/windows.rs

+    rcArea: RECT,
+}
+
+// CANDIDATEFORM structure  


Trailing whitespace at the end of the comment.

Suggested change

// CANDIDATEFORM structure

// CANDIDATEFORM structure

- Remove duplicate GCS_RESULTSTR constant definition - Remove trailing whitespace in comments - Use std::mem::zeroed() instead of arbitrary default POINT values - Define CANDIDATE_WINDOW_Y_OFFSET constant instead of magic number - Only set candidate window position for index 0 (most IMEs only use this) - Add error handling for ImmSetOpenStatus failure in WM_SETFOCUS - Add clarifying comment for dirty flag in ime_test.rs

not-fl3 · 2025-12-23T08:55:42Z

Great work, thanks for PR!

Copilot AI review requested due to automatic review settings November 30, 2025 14:22

Copilot started reviewing on behalf of gqf2008 November 30, 2025 14:22 View session

Copilot finished reviewing on behalf of gqf2008 November 30, 2025 14:24

Copilot AI reviewed Nov 30, 2025

View reviewed changes

not-fl3 merged commit a7d68bb into not-fl3:master Dec 23, 2025
11 checks passed

		ImmAssociateContext(hwnd, new_himc);
		ImmSetOpenStatus(new_himc, 1);

-                        ImmAssociateContext(hwnd, new_himc);
-                        ImmSetOpenStatus(new_himc, 1);
+                        let assoc_result = ImmAssociateContext(hwnd, new_himc);
+                        let set_status_result = ImmSetOpenStatus(new_himc, 1);
+                        if assoc_result == 0 || set_status_result == 0 {
+                            // If either association or open status fails, release the context
+                            ImmReleaseContext(hwnd, new_himc);
+                        }

	let mut pt = POINT { x: 100, y: 100 };
	let mut pt: POINT = unsafe { std::mem::zeroed() };

Uh oh!

Conversation

gqf2008 commented Nov 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

not-fl3 commented Dec 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants