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 the DECTCEM reset position in the conpty stream #17148

Merged
merged 1 commit into from Apr 29, 2024

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Apr 27, 2024

Summary of the Pull Request

When the conpty renderer determines that it needs to hide the cursor,
it does so by inserting a DECTCEM reset sequence at the start of the
output buffer, assuming that is the start of the frame. But when the
_noFlushOnEnd flag is set, you can have multiple frames pending in the
buffer, and the DECTCEM sequence will then end up in the wrong place.

This PR fixes the issue by saving the buffer size at the start of the
frame, and using that saved offset as the insert position for the
DECTCEM sequence.

Validation Steps Performed

I have a game that was frequently affected by this issue (the cursor
would be visible when it was meant to be hidden). With this PR applied,
it now works perfectly.

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-VT Virtual Terminal sequence support Product-Conpty For console issues specifically related to conpty labels Apr 27, 2024
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

oh yea that's an obvious bug for sure

@zadjii-msft zadjii-msft added this pull request to the merge queue Apr 29, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 29, 2024
## Summary of the Pull Request

When the conpty renderer determines that it needs to hide the cursor,
it does so by inserting a `DECTCEM` reset sequence at the start of the
output buffer, assuming that is the start of the frame. But when the
`_noFlushOnEnd` flag is set, you can have multiple frames pending in the
buffer, and the `DECTCEM` sequence will then end up in the wrong place.

This PR fixes the issue by saving the buffer size at the start of the
frame, and using that saved offset as the insert position for the
`DECTCEM` sequence.

## Validation Steps Performed

I have a game that was frequently affected by this issue (the cursor
would be visible when it was meant to be hidden). With this PR applied,
it now works perfectly.

## PR Checklist
- [x] Closes #15449
Merged via the queue into microsoft:main with commit ef318a1 Apr 29, 2024
13 of 15 checks passed
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.

excellent catch, thanks!

@DHowett DHowett added this to To Cherry Pick in 1.19 Servicing Pipeline via automation Apr 29, 2024
@DHowett DHowett added this to To Cherry Pick in 1.20 Servicing Pipeline via automation Apr 29, 2024
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.19 Servicing Pipeline Apr 30, 2024
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.20 Servicing Pipeline Apr 30, 2024
DHowett pushed a commit that referenced this pull request Apr 30, 2024
## Summary of the Pull Request

When the conpty renderer determines that it needs to hide the cursor,
it does so by inserting a `DECTCEM` reset sequence at the start of the
output buffer, assuming that is the start of the frame. But when the
`_noFlushOnEnd` flag is set, you can have multiple frames pending in the
buffer, and the `DECTCEM` sequence will then end up in the wrong place.

This PR fixes the issue by saving the buffer size at the start of the
frame, and using that saved offset as the insert position for the
`DECTCEM` sequence.

## Validation Steps Performed

I have a game that was frequently affected by this issue (the cursor
would be visible when it was meant to be hidden). With this PR applied,
it now works perfectly.

## PR Checklist
- [x] Closes #15449

(cherry picked from commit ef318a1)
Service-Card-Id: 92457089
Service-Version: 1.19
DHowett pushed a commit that referenced this pull request Apr 30, 2024
## Summary of the Pull Request

When the conpty renderer determines that it needs to hide the cursor,
it does so by inserting a `DECTCEM` reset sequence at the start of the
output buffer, assuming that is the start of the frame. But when the
`_noFlushOnEnd` flag is set, you can have multiple frames pending in the
buffer, and the `DECTCEM` sequence will then end up in the wrong place.

This PR fixes the issue by saving the buffer size at the start of the
frame, and using that saved offset as the insert position for the
`DECTCEM` sequence.

## Validation Steps Performed

I have a game that was frequently affected by this issue (the cursor
would be visible when it was meant to be hidden). With this PR applied,
it now works perfectly.

## PR Checklist
- [x] Closes #15449

(cherry picked from commit ef318a1)
Service-Card-Id: 92457090
Service-Version: 1.20
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.20 Servicing Pipeline May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty
Development

Successfully merging this pull request may close these issues.

Hidden cursors sometimes reappear unexpectedly
4 participants