fix: refresh IME cursor area after redraw#10443
fix: refresh IME cursor area after redraw#10443bet4it wants to merge 2 commits intowarpdotdev:masterfrom
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @bet4it on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
There was a problem hiding this comment.
Overview
This PR updates the winit redraw success path to notify frame_drawn and then refresh the IME cursor area when IME is enabled, keeping preedit/candidate UI aligned after redraws.
Concerns
- No blocking correctness or security concerns found in the changed hunk.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
vorporeal
left a comment
There was a problem hiding this comment.
my main question is whether this is the correct place to be doing this. feels a bit odd that we're unconditionally attempting to update the IME position after every frame we render, as opposed to this being reactive to a change in cursor position?
we do seem to have this reactive logic (the ActiveCursorPositionUpdated event) - do you have a sense as to why that event isn't sufficient?
|
i guess in some ways it ends up being a question of whether this is a flow that should be reactive (i.e.: focus and cursor position changes trigger a call into platform code to update the IME position) or whether we want to "poll" for an updated position on every frame when the IME is open. i'm doing some quick testing on macOS to see what the update behavior/frequency is like there, to get a better sense of what might be reasonable to do on linux. |
decf331 to
46e6c9b
Compare
|
I’ve updated the patch to remove the redraw-side IME update entirely. The The old event path wasn’t sufficient for terminal-style cursor movement because that movement doesn’t currently surface as a separate signal before layout/rendering. |
Description
This change refreshes the IME cursor area after a successful redraw when IME is enabled.
That keeps the IME preedit/candidate UI aligned with the terminal cursor during redraw-heavy terminal apps.
Linked Issue
ready-to-specorready-to-implement.Testing
./script/runScreenshots / Videos
Before this PR:


After this PR:
Agent Mode