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 cursor invalidation when line renditions are used #17234

Merged
merged 1 commit into from
May 10, 2024

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented May 10, 2024

Summary of the Pull Request

When the renderer calculates the invalidate region for the cursor, it
needs to take the line rendition into account. But it was using a
relative coordinate rather than absolute coordinate when looking up the
line rendition for the row, so the calculated region could easily be
incorrect.

With this PR we now use the line rendition that was already being cached
in the CursorOptions structure, so we avoid needing to look it up
anyway. Similarly I've replaced the IsCursorDoubleWidth lookup with
the value that was already cached in the CursorOptions structure.

Validation Steps Performed

I've confirmed that the test case in issue #17226 is now working as
expected.

PR Checklist

@j4james
Copy link
Collaborator Author

j4james commented May 10, 2024

I'm just assuming there wasn't a good reason not to use the cached CursorOptions properties. If there was, this PR is obviously no good and we'll need to fix #17226 in some other way.

Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

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

Ah, of course! I'm not sure why I missed this. I think this is correct: _invalidateCurrentCursor is supposed to invalidate the last cursor position _PaintFrame. If the line rendition changes, it still needs to invalidate as if the previous line rendition was active.

Thanks for fixing this!

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.

@lhecker we may want to define DO_NOT_USE variants of those functions inside the renderer like you did for the cooked read code, just to audit cases like this!

@DHowett DHowett added this pull request to the merge queue May 10, 2024
Merged via the queue into microsoft:main with commit b6f5cbe May 10, 2024
15 checks passed
@j4james j4james deleted the fix-cursor-invalidation branch May 20, 2024 23:49
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 invalidation failing when line renditions are used
3 participants