-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
More cursor droppings #8213
Labels
Area-Rendering
Text rendering, emoji, complex glyph & font-fallback issues
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-3
A description (P3)
Product-Conhost
For issues in the Console codebase
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
ghost
added
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
Needs-Tag-Fix
Doesn't match tag requirements
labels
Nov 10, 2020
Oh gosh, welcome to the cursor turds rabbit hole 😄. Thanks for the investigation and the write up - best of luck to any who dare venture forth into madness |
zadjii-msft
added
Area-Rendering
Text rendering, emoji, complex glyph & font-fallback issues
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-3
A description (P3)
Product-Conhost
For issues in the Console codebase
labels
Nov 12, 2020
DHowett
removed
the
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
label
Nov 12, 2020
ghost
pushed a commit
that referenced
this issue
Nov 30, 2020
When the viewport is moved to the "virtual bottom" of the buffer (via the `MoveToBottom` method), it is important that the horizontal viewport offset be left as it is, otherwise that can result in some undesirable side effects. Since the VT coordinate system is relative to the top of the viewport, many VT operations call the `MoveToBottom` method to make sure the viewport is correctly positioned before proceeding. There is no need for the horizontal position to be adjusted, though, since the X coordinates are not constrained by the viewport, but are instead relative to the underlying buffer. Setting the viewport X coordinate to 0 in `MoveToBottom` (as we were previously doing) could result in the cursor being pushed off screen. And if the operation itself was moving the cursor, that would then trigger another viewport move to bring the cursor back into view. These conflicting movements meant the viewport was always forced as far left as possible, and could also result in cursor "droppings" as the cursor lost track of where it had been. I've now fixed this by updating the `GetVirtualViewport` method to match the horizontal offset of the active viewport, instead of having the X coordinate hardcoded to 0. ## Validation Steps Performed I've manually confirmed that this fixes the cursor "droppings" test case reported in issue #8213. I've also added a screen buffer test that makes sure the `MoveToBottom` method is working as expected, and not changing the horizontal viewport offset when it moves down. Closes #8213
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
Nov 30, 2020
🎉This issue was addressed in #8434, which has now been successfully released as Handy links: |
This was referenced May 9, 2024
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area-Rendering
Text rendering, emoji, complex glyph & font-fallback issues
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Priority-3
A description (P3)
Product-Conhost
For issues in the Console codebase
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Environment
Windows build number: Version 10.0.18362.1082
Windows Terminal version (if applicable): commit 64aa911
Steps to reproduce
What this sequence is doing:
Expected behavior
When the cursor moves down, it should be erased from line 1, and then redrawn on line 5.
Actual behavior
When the cursor moves down, it isn't erased from line 1, so you end up with two copies of the cursor.
What's happening is that when the cursor is moved, the
SetConsoleCursorPositionImpl
method adjusts the viewport to make sure the new position will be visible onscreen. Before that happens, though, there's a call toSCREEN_INFORMATION::MoveToBottom
which forces the viewport X coordinate to zero. So at the time of the actual move calculation, it briefly appears as if the cursor is offscreen, and thus not visible, so the code doesn't trigger a repaint.So in step 5 of the test case, when the cursor moves down, there's no repaint to hide the old position, and also actually no repaint to render the new position. It's only the output in step 6 that forces the cursor to repaint, but by that point it's too late to erase the old cursor position.
I think the fix for this is just to make
MoveToBottom
preserve the current X coordinate. This would actually improve the behaviour of the unwrapped viewport in general, so either way I think it's a good thing. However, the catch is that it's then more likely for the console to trigger the crash from issue #1976, so that needs to be fixed first (I think I have a solution for that too).The text was updated successfully, but these errors were encountered: