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

Make certain that the cursor can invert/not-occlude/display the character under it #9610

Closed
Tracked by #13392
DHowett opened this issue Mar 25, 2021 · 60 comments · Fixed by #15283
Closed
Tracked by #13392

Make certain that the cursor can invert/not-occlude/display the character under it #9610

DHowett opened this issue Mar 25, 2021 · 60 comments · Fixed by #15283
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@DHowett
Copy link
Member

DHowett commented Mar 25, 2021

I've been using WSL for a while and it's been great, but using nvim with this issue still pending is at times unusable. Other terminals are not having this issue (see ConEmu or Hyper for example). I hope this issue gets re-opened.

Originally posted by @n1ghtmare in #1203 (comment)

I'm with @n1ghtmare on this one. Most syntax highlighting colors in Vim have a wide array of colors that very much clash with the cursor color when the cursor color isn't able to be swapped to a contrasted color dynamically. This has been demonstrated in the few screenshots I uploaded before. It affects everyone using terminal Vim or another terminal based text editor.

I know @pianocomposer321 said to pick a better color for the cursor but this doesn't make a difference in the end. If he adjusted his screenshot to be over a comment instead of the keyword then the comment's character under the cursor would be invisible. Also in his current screenshot the contrast is very low, it would certainly not be good enough for a video recording or giving a talk.

As good as the MS terminal is, I still tend to use wsltty (which dynamically sets its cursor color based on contrast calculations) in my day to day because having a block cursor in terminal Vim is much better than the vintage cursor in the MS terminal. Not just from a legibility POV, but it allows you to configure Vim so that your cursor shape changes from the block cursor to a line cursor depending on if you're in normal or insert mode.

Originally posted by @nickjj in #1203 (comment)

Subscribers, thumbs-uppers from that comment: @krage, @saurik, @krage

@ghost 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 Mar 25, 2021
@skyline75489
Copy link
Collaborator

Is this blocked by #6193 or other issues? Or we can just implement the algorithm for calculating the contrasted color and call it a day

@zadjii-msft
Copy link
Member

I KNEW THERE WAS AN EXISTING ISSUE!

Part of the trick here is that when the Renderer is drawing runs of text, it doesn't know where the cursor is. So it needs to be able to split the run with the cursor in it, so it can draw {text left of the cursor, text where the cursor is, text to the right of the cursor }.

Presumably, if there's a way of doing it without doing all the work in #6193, that'd be cool. I had all these ideas before in #6151 for different ways of specifying how the text at the cursor should look. Something like "cursorTextColor": null|"textForeground"|"#rrggbb".

@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Mar 25, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 25, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Mar 25, 2021
@DHowett
Copy link
Member Author

DHowett commented Mar 26, 2021

The thing I was toying with--which initially made me think that this would be easy--was drawing a D2D "fill effect" (that is: a command buffer that produces a flood fill) through the image/effect drawing pipeline with the XOR composition mode. It was promising, but it only worked where we weren't already drawing a background. Everything goes pear-shaped when you are drawing a background color. 😄

@DHowett
Copy link
Member Author

DHowett commented Mar 26, 2021

However, it looks like we can use the 2-pass cursor renderer @zadjii-msft wrote to backplate the color for a destination-inversion cursor.

@DHowett
Copy link
Member Author

DHowett commented Mar 26, 2021

it puts the cursor on its skin

@DHowett DHowett self-assigned this Mar 27, 2021
@nickjj
Copy link

nickjj commented Mar 27, 2021

Looks like a really good start, nice work. Could this be something that gets backported into the current stable release as a hotfix once you're happy with its performance and stability? It feels like such a huge feature that it transforms the entire experience for anyone who might be using Vim.

@Pirastrino
Copy link

Imo, there are 3 possible solution to this:

  • calculate contrast color from the current foreground
  • use terminal background color
  • define background and foreground color for the cursor

Calculated contrast color may not have enough contrast to the cursor's background color (even visible in the post above (purple cursor + inverted color - blue) and it may be distracting for some users. I agree it should be the default behavior but also, there should be a possibility to use a custom cursor foreground color defined by user or system background as default, something like: Cursor color = (contrast color | system background | custom cursor foreground) in profile settings.

But I see that the main issue here is the cursor position between renders so thank you for your work @DHowett!

@zadjii-msft
Copy link
Member

Well, what needs to happen now is for us to come up with a design that reconciles the current cursorColor with the proposed cursorTextColor, with the now possible invertCursor functionality. Once we've got a sensible way of expressing these options, it needs to make it into a PR, merge it, then selfhost it internally to make sure it feels right. That'll probably take right up to the next release anyways, so I'm not sure releasing as a hotfix would end up saving anyone any time 😄

DHowett added a commit that referenced this issue Mar 30, 2021
This commit introduces support for inverting all types of cursor.

To invert the display without re-rendering any text, we draw the cursor
into a command list and then compose the command list with the existing
renderer using the MASK_XOR composition flag.

This wouldn't normally work with our renderer because there is no
_background_ color to invert in some cases (such as when acrylic is in
use.)

To work around that, we're taking advantage of @zadjii-msft's two-pass
cursor renderer: First, we draw the cursor region in the background
color (to give the later pass something to work with), then after the
text is drawn we do the command list MASK_XOR.

Example cursor draw (for emptyBox cursor)
Therefore, cursor drawing looks like this (`_` is background, `@` is
cursor, `A` is glyph)

```
First pass (before text)

_____
_   _
_   _
_   _
_____

Text is added

__A__
_A A_
_AAA_
_A A_
_A_A_

Second pass (after text)

@@A@@ < Drawn with XOR mask; overlaps shown as [a]
@A A@
@aaa@
@A A@
@A@a@
```

The box, underline, etc. cursors are treated the same way, give or take
some pixels.

Fixes #9610
@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Mar 30, 2021
DHowett added a commit that referenced this issue Mar 30, 2021
This commit introduces support for inverting all types of cursor.

To invert the display without re-rendering any text, we draw the cursor
into a command list and then compose the command list with the existing
renderer using the MASK_XOR composition flag.

This wouldn't normally work with our renderer because there is no
_background_ color to invert in some cases (such as when acrylic is in
use.)

To work around that, we're taking advantage of @zadjii-msft's two-pass
cursor renderer: First, we draw the cursor region in the background
color (to give the later pass something to work with), then after the
text is drawn we do the command list MASK_XOR.

Example cursor draw (for emptyBox cursor)
Therefore, cursor drawing looks like this (`_` is background, `@` is
cursor, `A` is glyph)

```
First pass (before text)

_____
_   _
_   _
_   _
_____

Text is added

__A__
_A A_
_AAA_
_A A_
_A_A_

Second pass (after text)

@@A@@ < Drawn with XOR mask; overlaps shown as [a]
@A A@
@aaa@
@A A@
@A@a@
```

The box, underline, etc. cursors are treated the same way, give or take
some pixels.

Fixes #9610
@ghost ghost added the In-PR This issue has a related PR label Mar 30, 2021
@DHowett
Copy link
Member Author

DHowett commented Mar 30, 2021

For those of you who appreciate long commentary,

// CURSOR INVERSION
// We're trying to invert the cursor and the character underneath it without redrawing the text (as
// doing so would break up the run if it were part of a ligature). To do that, we're going to try
// to XOR the content of the screen where the cursor would have been.
//
// This renderer, however, supports transparency. In fact, in its default configuration it will not
// have a background at all (it delegates background handling to somebody else.) You can't XOR what
// isn't there.
//
// To properly invert the cursor in such a configuration, then, we have to play some tricks. Examples
// are given below for two cursor types, but this applies to all of them.
//
// First, we'll draw a "backplate" in the user's requested background color (with the alpha channel
// set to 0xFF). (firstPass == true)
//
// EMPTY BOX  FILLED BOX
// =====      =====
// =   =      =====
// =   =      =====
// =   =      =====
// =====      =====
//
// Then, outside of _drawCursor, the glyph is drawn:
//
// EMPTY BOX  FILLED BOX
// ==A==      ==A==
// =A A=      =A=A=
// AAAAA      AAAAA
// A   A      A===A
// A===A      A===A
//
// Last, we'll draw the cursor again in all white and use that as the *mask* for XORing the already-
// drawn pixels. (firstPass == false) (# = mask, a = inverted A)
//
// EMPTY BOX  FILLED BOX
// ##a##      ##a##
// #A A#      #a#a#
// aAAAa      aaaaa
// a   a      a###a
// a###a      a###a

DHowett added a commit that referenced this issue Mar 31, 2021
This commit introduces support for inverting all types of cursor.

To invert the display without re-rendering any text, we draw the cursor
into a command list and then compose the command list with the existing
renderer using the MASK_XOR composition flag.

This wouldn't normally work with our renderer because there is no
_background_ color to invert in some cases (such as when acrylic is in
use.)

To work around that, we're taking advantage of @zadjii-msft's two-pass
cursor renderer.

To properly invert the cursor over a transparent background:
(Examples are given below for two cursor types, but this applies to all
of them.)

First, we'll draw a "backplate" in the user's requested background color
(with the alpha channel set to 0xFF). (`firstPass` == true)

    EMPTY BOX  FILLED BOX
    =====      =====
    =   =      =====
    =   =      =====
    =   =      =====
    =====      =====

Second, the glyph is drawn (outside of the cursor renderer).

    EMPTY BOX  FILLED BOX
    ==A==      ==A==
    =A A=      =A=A=
    AAAAA      AAAAA
    A   A      A===A
    A===A      A===A

Last, we'll draw the cursor again in all white and use that as the
*mask* for XORing the already-drawn pixels. (`firstPass` == false) (# =
mask, a = inverted A)

    EMPTY BOX  FILLED BOX
    ##a##      ##a##
    #A A#      #a#a#
    aAAAa      aaaaa
    a   a      a###a
    a###a      a###a

Fixes #9610
@pianocomposer321
Copy link

Just wanted to pipe up here for a second, specifically replying to @DHowett's last message. I always considered ligatures being drawn as their individual characters when the cursor is over them as a feature, not a bug. It's nice to be able to see what characters comprise the ligature if you need to, and have it display as a ligature otherwise.

That said, I can see it the other way too, but I'm just saying that if it was up to me I wouldn't jump through too many hoops to make ligatures display "correctly".

@DHowett
Copy link
Member Author

DHowett commented Mar 31, 2021

Just wanted to pipe up here for a second, specifically replying to @DHowett's last message. I always considered ligatures being drawn as their individual characters when the cursor is over them as a feature, not a bug. It's nice to be able to see what characters comprise the ligature if you need to, and have it display as a ligature otherwise.

That said, I can see it the other way too, but I'm just saying that if it was up to me I wouldn't jump through too many hoops to make ligatures display "correctly".

Thanks for the input! Yeah... I waffled back and forth on this. Right now, given the architecture of our renderer, it's actually harder for us to split the ligature and re-strike the character 😄

We condense full runs of text into as few rendering calls as possible, and we don't have support for splitting a run where we think the cursor is ... which is 100% something we need to fix later.

@zhihaoy
Copy link

zhihaoy commented Feb 3, 2023

#6337 made an assumption that foreground color is sufficiently different from the block cursor's background, which doesn't hold true. I still need a feature to change the TEXT color when the block cursor is over it. Inverting is too smart for me; I will set it to white in a light theme to distinguish the dark text from my dark block cursor if possible.

image

@james1236
Copy link

james1236 commented Feb 23, 2023

In the preview version with the new AtlasEngine renderer enabled:

image

Before

image

After

image

Almost fixed, but the cursor is much darker for some reason (even though cursorColor is #FFFFFF), why is the cursor color changing instead of the text? This is more annoying than the invisible text issue, so I'll be going back to the old version.

If only there was an option to set the cursor text color to black, as was suggested in the original issue 3 years ago...

@paulharris
Copy link

This did not look the same for me, the cursor was white and the text was slightly not white - not inverted - so was very hard to see the text behind the cursor.

@MustafaFayez
Copy link

what do i do to fix this issue?
I use 1.17 preview version now

@sitompul
Copy link

@lhecker
Copy link
Member

lhecker commented May 3, 2023

It's a little difficult for me to say whether I should close this issue with the PR that I've just opened... If I'm not misunderstanding the original issue description at the top, then it'll fix the issue. The PR uses an algorithm to determine the contrast between the cursor and the underlying background and the text on top of it, and ensure it always remains "distinguishable".

In other words: After that PR, if you use AtlasEngine, no matter whether you use inversed cursors or if you use colored cursors, the text will always remain visible.

The discussion however has drifted away quite a bit since then, which I can fully understand, given that our cursors are kinda... meh. But I feel like that these issues are not part of the original issue anymore. They're also much more easily to address. Among those are suggestion to allow users to configure the cursor background and foreground color individually, as well as to implement reversed cursors. I hope to get to that in the near term, although community contributions for that would be greatly appreciated, because I'm not sure if I'll have time myself.

On that note, it might be interesting to mention that the new colored cursors will be sort of "semi-reverse" cursors and look like this (this is when I set my cursor to #ff0000):
image

@3N4N
Copy link

3N4N commented May 4, 2023

Thank you! @lhecker. I hope to soon use block cursor like god intended (I'm currently using vintage shape because of this indistinguishable-character issue).

[The other issues/feature-requests] are also much more easily to address. Among those are suggestion to allow users to configure the cursor background and foreground color individually.

This really seems like a no-brainer. Since it's easier to address, as you say, why not implement that before the automated inversion solutions? I am repeating @avih here, I know, but this easy-to-address feature could help me use Vim with proper guicursor, instead of having to use the boring vintage cursor.

@lhecker
Copy link
Member

lhecker commented May 4, 2023

Since it's easier to address, as you say, why not implement that before the automated inversion solutions?

The "automated" mode is ~4 lines of code and took me <5min to write and test, whereas the other issues/feature-requests require a lot more work. Given that I'm technically already 110% booked out with work for months on end, I think you can see why I've picked the easy option for now. 🥲

I hope to soon use block cursor like god intended (I'm currently using vintage shape because of this indistinguishable-character issue).

You could try god's intended cursors right now, if you're curious. 😏
Here is a preview build based on that PR. I put it on my own host for convenience (and because the file will automatically expire in a week that way), but it's properly built and digitally signed by Microsoft. Please let me know if you find any issues with the way the cursors work now!

@3N4N
Copy link

3N4N commented May 5, 2023

I hope to soon use block cursor like god intended (I'm currently using vintage shape because of this indistinguishable-character issue).

You could try god's intended cursors right now, if you're curious. 😏 Here is a preview build based on that PR.

Awesome! Been using for a few hours. Works great! Thanks a bunch.

@avih
Copy link

avih commented May 6, 2023

IMHO, if you insist on choosing the FG color yourself and not letting the user specify it, then just pick either black or white - whichever has the better contrast to the chosen cursor BG color.

IMO it would pretty much gurante to look better than any attempt to preserve the underlaying FG color but still modifying it so that it has some contrast to the BG.

Just see how many failed atempts there were at that, to realise that it's not as simple as you think to do what the user prefer automatically.

@nickjj
Copy link

nickjj commented May 6, 2023

I think if we stick with setting it manually then we're not going to address the root issue where characters are invisible based on syntax highlighting in terminal based editors such as Vim. You're going to have a wide range of dark and light text based on any variant of color themes.

3N4N added a commit to 3N4N/dotfiles that referenced this issue May 7, 2023
ms-terminal has a bug (microsoft/terminal#9610) where cursor hides the
character underneath.  To circumvent, I used vintage cursor shape
(basically thicker version of underscore), so it didn't cover the
character.  guicursor changed the cursor shape to block in normal mode,
so I had to disable it.

There has been development in the ms-terminal repo
(microsoft/terminal/#15283).  The cursor now inverts the character color
underneath so the character is now visible.  No need to disable
guicursor on ms-terminal anymore.
microsoft-github-policy-service bot pushed a commit that referenced this issue May 8, 2023
Oklab by Björn Ottosson is a color space that has less irregularities
than the CIELAB color space used for ΔE2000. The distance metric for
Oklab (ΔEOK) is defined by CSS4 as the simple euclidian distance.
This allows us to drastically simplify the code needed to determine
a color that has enough contrast. The new implementation still lacks
proper gamut mapping, but that's another and less important issue.
I also made it so that text with the dim attribute gets adjusted just
like regular text, since this is an accessibility feature after all.

The new code is so much faster than the old code (12-125x) that I
dropped any caching code we had. While this increases the CPU overhead
when printing lots of indexed colors, the code is way less complex now.
"Increases" in this case however means something in the order of 15-60ns
per color change (as measured on my CPU). It's possible to further
improve the performance using explicit SIMD instructions, but I've
left that as a future improvement, since that will make the code quite
a bit more verbose and I didn't want to hinder the initial review.

Finally, these new routines are also used for ensuring that the
AtlasEngine cursors remains visible at all times.

Closes #9610

## Validation Steps Performed
* When `adjustIndistinguishableColors` is enabled
  colors are distinguishable ✅
* An inverted cursor on top of a `#7f7f7f` foreground & background
  is still visible ✅
* A colored cursor on top of a background with identical color
  is still visible ✅
* Cursors on a transparent background are visible ✅
@habamax
Copy link

habamax commented May 8, 2023

I think if we stick with setting it manually then we're not going to address the root issue where characters are invisible based on syntax highlighting in terminal based editors such as Vim. You're going to have a wide range of dark and light text based on any variant of color themes.

I have used vim for a long time in different terminals that allow setting bg and fg for a cursor -- never had an issue with syntax highlighting and cursor. If your terminal background is black, set cursor colors to white and black, do the opposite if terminal background is light/white. It always worked, it would have worked in windows terminal too.

But anyway, if that new implementation would let us distinguish what is under cursor, I am all for it.

@nickjj
Copy link

nickjj commented May 9, 2023

I have used vim for a long time in different terminals that allow setting bg and fg for a cursor -- never had an issue with syntax highlighting and cursor.

@habamax This issue has multiple screenshots showing how it would be an issue with a statically configured cursor color.

Here's a screenshot you posted a while back:
image

  • If you configured a static black cursor, reading your comments would be really hard because it's medium grey
  • If you configured a static white cursor, reading your function definitions would be really hard because it's almost white
  • Your current set up with a purple cursor on medium grey is also very hard to read

Your screenshot is also a near-best case scenario because you have pretty bright colors and there's only a few colors shown at once.

@habamax
Copy link

habamax commented May 9, 2023

@nickjj it is about other terminals that allow setup of cursor background AND FOREGROUND colors. Like gnome-terminal, for example:
image

DHowett pushed a commit that referenced this issue May 16, 2023
Oklab by Björn Ottosson is a color space that has less irregularities
than the CIELAB color space used for ΔE2000. The distance metric for
Oklab (ΔEOK) is defined by CSS4 as the simple euclidian distance.
This allows us to drastically simplify the code needed to determine
a color that has enough contrast. The new implementation still lacks
proper gamut mapping, but that's another and less important issue.
I also made it so that text with the dim attribute gets adjusted just
like regular text, since this is an accessibility feature after all.

The new code is so much faster than the old code (12-125x) that I
dropped any caching code we had. While this increases the CPU overhead
when printing lots of indexed colors, the code is way less complex now.
"Increases" in this case however means something in the order of 15-60ns
per color change (as measured on my CPU). It's possible to further
improve the performance using explicit SIMD instructions, but I've
left that as a future improvement, since that will make the code quite
a bit more verbose and I didn't want to hinder the initial review.

Finally, these new routines are also used for ensuring that the
AtlasEngine cursors remains visible at all times.

Closes #9610

* When `adjustIndistinguishableColors` is enabled
  colors are distinguishable ✅
* An inverted cursor on top of a `#7f7f7f` foreground & background
  is still visible ✅
* A colored cursor on top of a background with identical color
  is still visible ✅
* Cursors on a transparent background are visible ✅

(cherry picked from commit 4dd9493)
Service-Card-Id: 89215984
Service-Version: 1.17
@gaoqiangks
Copy link

So, what's the solution? Many issues are closed but I still cann't find a solution.

@lhecker
Copy link
Member

lhecker commented Jun 24, 2023

Did you try Windows Terminal Preview 1.18 yet? If that doesn’t work for you it’d be great if you could open a new issue with a screenshot showing the problem and I‘ll try to fix it ASAP. 🙂

If you’re asking about setting individual foreground and background colors for the cursor, I don’t think we have an issue tracking specifically that yet, but I‘ve already made it my goal to implement that for 1.19 anyways.

@gaoqiangks
Copy link

WT Preview 1.18 is much better. Thank you!

@weibeld
Copy link

weibeld commented Jul 26, 2023

This is now being discusses in #15766.

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 In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.