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.

Sign up for GitHub

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

Jump to bottom

Cursor droppings on wide chars in Windows Terminal #14657

Closed
j4james opened this issue Jan 10, 2023 · 2 comments · Fixed by #14661
Closed

Cursor droppings on wide chars in Windows Terminal #14657

j4james opened this issue Jan 10, 2023 · 2 comments · Fixed by #14661
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@j4james
Copy link
Collaborator

j4james commented Jan 10, 2023

Windows Terminal version

1.16.2641.0

Windows build number

10.0.19044.2364

Other Software

No response

Steps to reproduce

  1. Open a WSL bash shell in Windows Terminal.
  2. Execute printf "\e[2 q\e[2J\e[1;999H\e[D\u306F"; sleep 1; printf "\e[B\n"

This sequence is performing the following steps:

  1. The cursor style is set to a non-blinking block (just to make the bug more obvious).
  2. The screen is cleared, and the cursor moved to the top right corner.
  3. The cursor is then moved one column back and a wide character (は) is written out.
  4. We wait for a second, to give the cursor a chance to render.
  5. The cursor is moved down a line.
  6. We output a newline (just so the shell prompt is back at the start of the line).

Expected Behavior

At the end of all that, there should only be one cursor visible on the screen.

Actual Behavior

In addition to the real cursor at the shell prompt, there is a second cursor left behind overlapping the は character.

image

I realise this is probably not a typical scenario that you're likely to encounter, but it'll become more common once Windows Terminal implements delayed EOL wrap correctly (which is one of the thing I'm hoping to fix in PR #14640).
@j4james j4james added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jan 10, 2023
@j4james
Copy link
Collaborator Author

j4james commented Jan 10, 2023

Note that this is not the same thing as #12739, which was the result of deferred cursor drawing. This is the result of a miscalculation in the cursor bounds checking, as explained in #13001 (comment).

@j4james j4james mentioned this issue Jan 10, 2023
Merged
@ghost ghost added the In-PR This issue has a related PR label Jan 10, 2023
@ghost ghost closed this as completed in #14661 Jan 11, 2023
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jan 11, 2023
ghost pushed a commit that referenced this issue Jan 11, 2023
@j4james
Correct the cursor invalidation region (#14661)
f1090a0 
Depending on the line rendition, and whether the cursor is over a wide
character or not, it's possible for the width to take up anywhere from 1
to 4 cells. And when it's more than 1 cell wide, part of the cursor may
end up off screen. However, our bounds check requires the entire cursor
to be on screen, otherwise it doesn't render anything, and that can
result in cursor droppings being left behind. This PR fixes that.

The bounds check that is causing this issue was introduced in #13001 to
fix a debug assertion.

To fix this, I've removed the bounds checking, and instead clip the
cursor rect to the boundaries of the viewport. This is what the original
code was trying to do prior to the #13001 fix, but was mistakenly using
the `Viewport:Clamp` method, instead of `TrimToViewport`. Since this
implementation doesn't require a clamp, there should be no risk of the
debug assertion returning.

## Validation Steps Performed

I've confirmed that the test case in #14657 is now working correctly,
and not leaving cursor droppings behind. I've also tested under conhost
with buffer sizes wider than the viewport, and confirmed it can handle a
wide cursor partially scrolled off screen.

Closes #14657
@ghost
Copy link

ghost commented Jan 24, 2023

🎉This issue was addressed in #14661, which has now been successfully released as Windows Terminal Preview v1.17.1023.:tada:

Handy links:

This was referenced May 9, 2024
Closed
Closed
@github-actions github-actions bot mentioned this issue May 23, 2024
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correct the cursor invalidation region j4james/terminal
1 participant
@j4james