Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix conpty cursor movement detection on double-width lines #17233

Merged
merged 1 commit into from
May 10, 2024

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented May 9, 2024

When the VT render engine checks whether the cursor has moved in the
InvalidateCursor method, it does so by comparing the origin of the
given cursor region with the last text output coordinates. But these two
values are actually from different coordinate systems, and when on a
double-width line, the x text coordinate is half of the corresponding
screen coordinate. As a result, the movement detection is sometimes
incorrect.

This PR fixes the issue by adding another field to track the last cursor
origin in screen coordinates, so we have a meaningful value to compare
against.

References and Relevant Issues

The previous cursor movement detection was added in PR #17194 to fix
issue #17117.

Validation Steps Performed

I've confirmed that the test case from issue #17232 is now fixed, and
the test case from issue #17117 is still working as expected.

PR Checklist

@j4james
Copy link
Collaborator Author

j4james commented May 10, 2024

Given the number of times I've got this wrong already, I wouldn't be surprised if there is still some edge case that I'm missing, but I think this should be correct now.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

Hey, bugs is bugs. Thanks for being on top of these, James!

@DHowett DHowett added this pull request to the merge queue May 10, 2024
Merged via the queue into microsoft:main with commit 34ecc5b May 10, 2024
15 checks passed
@j4james j4james deleted the fix-conpty-move-detection branch May 20, 2024 23:50
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.

Cursor movement is not always detected on double-width lines
3 participants