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 multiple cursor invalidation issues #17181

Merged
merged 3 commits into from
May 2, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented May 2, 2024

There were multiple bugs:

  • GDI engine only paints whatever has been invalidated.
    This means we need to not just invalidate the old cursor rect
    but also the new one, or else movements may not be visible.
  • The composition should be drawn at the cursor position even if
    the cursor is invisible, but inside the renderer viewport.
  • Conceptually, scrolling the viewport moves the relative cursor
    position even if the cursor is invisible.
  • An invisible cursor is not the same as one that's outside the
    viewport. It's more like a cursor that's not turned on.

To resolve the first issue we simply need to call InvalidateCursor
again. To do so, it was abstracted into _invalidateCurrentCursor().

The next 2 issues are resolved by un-optional-izing CursorOptions.
After all, even an invisible or an out-of-bounds cursor still has a
coordinate and it may still be scrolled into view.
Instead, it has the new inViewport property as a replacement.
This allows for instance the IME composition code in the renderer
to use the cursor coordinate while the cursor is invisible.

The last issue is fixed by simply changing the .isOn logic.

Closes #17150

Validation Steps Performed

  • In conhost with the GDI renderer:
    printf "\e[2 q"; sleep 2; printf "\e[A"; sleep 2; printf "\e[B"
    Cursor moves up after 2s and then down again after 2s. ✅
  • Hide the cursor ("\e[?25l") and use a CJK IME.
    Words can still be written and deleted correctly. ✅
  • Turning the cursor back on ("\e[?25h") works ✅
  • Scrolling shows/hides the cursor ✅

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.

this is my only question. Rest looks good, mostly just mechanical code movement

src/renderer/base/renderer.cpp Show resolved Hide resolved
@DHowett DHowett merged commit 4fbcd65 into main May 2, 2024
20 checks passed
@DHowett DHowett deleted the dev/lhecker/17150-fix-cursor-invalidation branch May 2, 2024 23:33
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.

Issues with cursor invalidation in GDI
3 participants