-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
VT renderer works incorrectly when cursor is moved between writes #17270
Comments
Hi I'm an AI powered bot that finds similar issues based off the issue title. Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you! Closed similar issues:
|
P.S. I think the provided example reproduces the same issue.
|
This comment was marked as outdated.
This comment was marked as outdated.
FYI, I did a git bisect with the VtRendererBug test case, and for me the issue first showed up in commit 72b4488. However, I suspect it may be one of those things that's timing dependent, and that commit just exposed a bug that has always existed. |
I've just confirmed that reverting commit 72b4488 on the main branch is enough to fix the issue for me (i.e. moving the Again I'm not sure that's a real solution to the problem, but it might be worth considering as a quick fix, assuming it works for everyone else and not just me. |
Thanks James, reverting 72b4488 fixed it for me as well. |
As for what's going wrong, this is what I've been able to establish:
Ignoring bug number 1 for the moment, it seems to me that point 3 is also a bug, because the So another possible way to fix this issue would be to set That said, the conpty code is complicated, and I really don't know it very well, so there may be more to it than that. I'm just throwing this out there as potential solution. |
Another simple test case that can be run from a WSL bash shell:
The text "This should be line 3", should be on line 3 😄, but it usually ends up on line 2. This one is not fixed by reverting 72b4488, but it is fixed by the |
Whichever the fix ends up being, I'm going to defer it until the next servicing wave for 1.21 - thanks for all the great investigations, everyone 😄 |
## Summary of the Pull Request If the VT render engine is moving the cursor to the start of a row, and the previous row was marked as wrapped, it will assume that it doesn't need to do anything, because the next output should automatically move the cursor to the correct position anyway. However, if that cursor movement is coming from the final `PaintCursor` call for the frame, there isn't going to be any more output, so the cursor will be left in the wrong position. This PR fixes that issue by clearing the `_wrappedRow` field before the `_MoveCursor` call in the `PaintCursor` method. ## Validation Steps Performed I've confirmed that this fixes all the test cases mentioned in issue #17270, and issue #17013, and I've added a unit test to check the new behavior is working as expected. However, this change does break a couple of `ConptyRoundtripTests` that were expecting the terminal row to be marked as wrapped when writing a wrapped line in two parts using `WriteCharsLegacy`. This is because the legacy way of wrapping a line isn't the same as a VT delayed wrap, so it has to be emulated with cursor movement, and that can end up resetting the wrap flag. It's possible that could be fixed, but it's already broken in a number of other ways, so I don't think this makes things much worse. For now, I've just made the affected test cases skip the wrapping check. ## PR Checklist - [x] Closes #17013 - [x] Closes #17270 - [x] Tests added/passed
Windows Terminal version
Latest source
Windows build number
10.0.19045.4355
Other Software
No
Steps to reproduce
The original issue is described here: FarGroup/FarManager#841
According to the comments, it regressed somewhere between 1.18.3181.0 - 1.19.11213.0
MRE: VtRendererBug.zip
STR: compile, run, follow instructions
Expected Behavior
The vertical bar moves right without affecting the underlying text.
It works as expected in OpenConsole.
Actual Behavior
The vertical bar and the underlying text are broken.
The third line is written into the second.
It doesn't work as expected in WT and other terminals that use OpenConsole, e.g. WezTerm.
I've managed to trace it down to
terminal/src/renderer/vt/XtermEngine.cpp
Lines 281 to 282 in f62d2d5
It looks like specifying the position explicitly fixes the issue:
Which could mean that the cursor position in
_lastText
is tracked incorrectly / unreliably.The text was updated successfully, but these errors were encountered: