Skip to content

Fix InputBox drag cursor regression and IME selection-delete#66

Open
Copilot wants to merge 6 commits intomainfrom
copilot/implement-range-selection-inputbox
Open

Fix InputBox drag cursor regression and IME selection-delete#66
Copilot wants to merge 6 commits intomainfrom
copilot/implement-range-selection-inputbox

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

  • Fix Issue 1: cursor follows drag
  • Fix Issue 2: IME clears selection before composition
  • Extract applyDragMove helper
  • Fix cursor forced to end of selection after mouse release
  • Fix unwanted selection after click-to-cancel-IME
    • Root cause: ImmNotifyIME(CPS_COMPLETE) in killIME() triggers WM_IME_COMPOSITION(GCS_RESULTSTR), which makes the EDIT insert the committed text and move its cursor to after the inserted text; the draw-loop EM_GETSEL sync then sees a displaced cursor, decides selMatchesOurs is false, and creates an unwanted [click_pos, commit_end] selection
    • Fix: capture getCursorPos() (= click position) before killIME(), then call movecursor(savedPos, savedPos) after — the draw-loop sync now sees EM_GETSEL == (dragBegin, dragEnd) so selMatchesOurs is true and no spurious selection is applied

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

@FeJS8888 FeJS8888 marked this pull request as ready for review March 27, 2026 08:51
Copilot AI review requested due to automatic review settings March 27, 2026 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes InputBox drag-selection cursor behavior and IME composition behavior by removing a destructive async cursor update from EM_SETSEL, and by explicitly deleting the active selection before IME composition begins. Also refactors drag-move handling into a shared helper to keep selection updates working when the mouse leaves nested Panel bounds.

Changes:

  • Stop posting async WM_USER+100+1 after EM_SETSEL to prevent cursor being reset to the selection anchor during drag.
  • Add InputBox::deleteSelectedText() and invoke it from WM_IME_STARTCOMPOSITION so IME composition correctly replaces the active selection.
  • Refactor and expand drag-move handling via applyDragMove() across handleEvent, catchMouseOwningFlag, and releaseMouseOwningFlag, plus draw-time sync/highlight updates.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/sys_edit.cpp IME start-composition now deletes selection; EM_SETSEL no longer triggers async cursor reset.
src/Widget.cpp Drag selection refactor (applyDragMove, charPositionFromLocalX), draw-time selection sync/highlight, selection deletion helper, updated mouse owning behavior.
include/Widget.h Declares new InputBox helper APIs (charPositionFromLocalX, applyDragMove, deleteSelectedText).

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

Comment on lines +1142 to +1144
float sel_s_px, sel_e_px, sel_tmp;
measuretext(content.substr(0, sel_s).c_str(), &sel_s_px, &sel_tmp, btnLayer);
measuretext(content.substr(0, sel_e).c_str(), &sel_e_px, &sel_tmp, btnLayer);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

Selection highlight drawing measures content.substr(0, sel_s/sel_e) every frame while focused with a selection. For long strings, this adds extra per-frame substring creation + text measurement on top of the existing cursor/IME measurements. Consider reusing the existing measurement cache (or caching prefix widths / last selection bounds) to avoid redundant measuretext() work during repaint.

Suggested change
float sel_s_px, sel_e_px, sel_tmp;
measuretext(content.substr(0, sel_s).c_str(), &sel_s_px, &sel_tmp, btnLayer);
measuretext(content.substr(0, sel_e).c_str(), &sel_e_px, &sel_tmp, btnLayer);
// 缓存选区起止位置对应的像素宽度,避免每帧重复 substr + measuretext
static std::wstring lastSelMeasuredContent;
static int lastSelS = -1;
static int lastSelE = -1;
static bool lastSelScaleChanged = false;
static bool lastSelPanelScaleChanged = false;
static float cachedSelSPx = 0.0f;
static float cachedSelEPx = 0.0f;
bool needRemeasureSelection =
lastSelMeasuredContent != content ||
lastSelS != sel_s ||
lastSelE != sel_e ||
lastSelScaleChanged != scaleChanged ||
lastSelPanelScaleChanged != PanelScaleChanged;
float sel_s_px;
float sel_e_px;
float sel_tmp;
if (needRemeasureSelection) {
measuretext(content.substr(0, sel_s).c_str(), &sel_s_px, &sel_tmp, btnLayer);
measuretext(content.substr(0, sel_e).c_str(), &sel_e_px, &sel_tmp, btnLayer);
cachedSelSPx = sel_s_px;
cachedSelEPx = sel_e_px;
lastSelMeasuredContent = content;
lastSelS = sel_s;
lastSelE = sel_e;
lastSelScaleChanged = scaleChanged;
lastSelPanelScaleChanged = PanelScaleChanged;
} else {
sel_s_px = cachedSelSPx;
sel_e_px = cachedSelEPx;
}

Copilot uses AI. Check for mistakes.
Comment on lines +1551 to +1576
int InputBox::charPositionFromLocalX(float localX) const {
const float padding = 14 * scale;
float click_x = localX - padding + scroll_offset;
int l = 0, r = (int)content.length();
int best_pos = 0;
float min_dist = 1e9f, tmp, char_x = 0;
while (l <= r) {
int mid = (l + r) / 2;
measuretext(content.substr(0, mid).c_str(), &char_x, &tmp, btnLayer);
float dist = fabsf(char_x - click_x);
if (dist < min_dist) { min_dist = dist; best_pos = mid; }
if (char_x < click_x) l = mid + 1;
else if (char_x > click_x) r = mid - 1;
else { best_pos = mid; break; }
}
return best_pos;
}

void InputBox::applyDragMove(int mouseX) {
float localX = (float)(mouseX - (int)left);
int best_pos = charPositionFromLocalX(localX);
dragEnd = best_pos;
// 光标跟随选区末端
if (cursor_pos != dragEnd) moveCursor(dragEnd);
// 同步选区到 sys_edit(EM_SETSEL),保证后续键盘操作在正确范围内进行
inv.movecursor(dragBegin, dragEnd);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

charPositionFromLocalX() does repeated content.substr(0, mid) allocations + measuretext() calls in a binary search. This used to run only on mouse-down, but it’s now called on every mouse-move during selection (applyDragMove), which can become noticeably expensive for longer text / high-frequency mouse events. Consider caching prefix pixel widths when content changes (or otherwise avoid repeated substring allocations/measures) so drag-selection stays smooth.

Copilot uses AI. Check for mistakes.
@FeJS8888
Copy link
Copy Markdown
Owner

@copilot 现在鼠标松开后,光标会被强制定位到选区末尾,修复这个问题

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 27, 2026

@copilot 现在鼠标松开后,光标会被强制定位到选区末尾,修复这个问题

Fixed in 212303c.

Root cause: EM_GETSEL always returns a normalized (start ≤ end) pair. After a right-to-left drag (dragBegin=5, dragEnd=2), the draw-loop sync block was reading newEnd=5 and calling moveCursor(5), snapping the cursor to the far end instead of where the mouse was released.

Fix: skip the cursor+selection sync when EM_GETSEL returns exactly the normalized form of our (dragBegin, dragEnd) — this means no keyboard action has changed the selection, so cursor_pos is already correct at dragEnd (the actual release point).

Copilot AI requested a review from FeJS8888 March 27, 2026 09:05
@FeJS8888
Copy link
Copy Markdown
Owner

@copilot 我刚刚修复了有IME的时候点击输入框取消IME,并且提交未完成字符串,但是现在点击输入框其他位置取消IME后,会选中从提交字符串开头到点击位置的字符串,修复这个问题

Agent-Logs-Url: https://github.com/FeJS8888/FeEGELib/sessions/a610d953-f4f5-46c5-9958-691a25cb3fdc

Co-authored-by: FeJS8888 <110683147+FeJS8888@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 31, 2026

@copilot 我刚刚修复了有IME的时候点击输入框取消IME,并且提交未完成字符串,但是现在点击输入框其他位置取消IME后,会选中从提交字符串开头到点击��...

Fixed in e207e21.

Root cause: killIME() calls ImmNotifyIME(CPS_COMPLETE), which causes the EDIT control to process WM_IME_COMPOSITION(GCS_RESULTSTR) — inserting the committed text and moving the EDIT cursor to after the inserted text. The draw-loop EM_GETSEL sync then sees this displaced cursor, finds selMatchesOurs is false, and overwrites dragBegin/dragEnd with a range spanning from the click position to the commit-end position, creating the unwanted selection highlight.

Fix: in the WM_USER+100+2 handler, capture the EDIT cursor position (= the click position — WM_SETTEXT doesn't move the cursor so it's still correct) before calling killIME(), then restore it with movecursor(savedPos, savedPos) afterwards. The draw-loop sync then reads EM_GETSEL == (dragBegin, dragEnd), selMatchesOurs is true, and no spurious selection is applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants