Make sure the inverted cursor is always readable (#3647)

[alabuzhev]

Currently "Inverse Cursor" is actually simply bitwise inversed.
It works fine, except when it does not, namely in the middle of the spectrum:
Anything close enough to dark grey (index 8, RGB(128, 128, 128) in the
classic palette) will for obvious reasons become almost the same dark grey
again after the inversion.

The issue is addressed by additionally XORing the inverted color with
RGB(63, 63, 63). This distorts the result enough to avoid collisions
in the middle. Ultimately this restores the behavior that was in
Windows Console since the Middle Ages (and still exists in ConhostV1.dll).

PR Checklist

• Closes Inverse cursor color logic is (sometimes) broken #3647
• CLA signed
• Tests added/passed
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: Inverse cursor color logic is (sometimes) broken #3647

Validation Steps Performed

1. Open OpenConsole.
2. Properties, Terminal, Cursor Colors, choose Inverse Color.
3. Optionally set the RGB value of the 8th color in the standard palette to RGB(128, 128, 128) on the Colors tab, but the default one will do too.
4. Type color 80 to see black text on dark grey background.
5. Make sure the inverted cursor is visible.
6. Repeat in WT with both default and experimental renderers.

[DHowett]

I'm cool with this, thanks for doing it!

[DHowett]

@msftbot make sure @lhecker signs off

[ghost]

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I'll only merge this pull request if it's approved by @lhecker

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[lhecker]

Original PR comment:

Summary of the Pull Request

PR Checklist

• Closes Inverse cursor color logic is (sometimes) broken #3647
• CLA signed
• Tests added/passed
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: Inverse cursor color logic is (sometimes) broken #3647

Detailed Description of the Pull Request / Additional comments

Currently "Inverse Cursor" is actually simply bitwise inversed. It works fine, except when it does not, namely in the middle of the spectrum: anything close enough to dark grey (index 8, RGB(128, 128, 128) in the classic palette) will for obvious reasons become almost the same dark grey again after the inversion:

The issue is addressed by additionally XORing the inverted color with RGB(63, 63, 63). This distorts the result enough to avoid collisions in the middle:

Ultimately this restores the behavior that was in Windows Console since the Middle Ages (and still exists in ConhostV1.dll).

Validation Steps Performed

Before:

After:

Manual steps:

1. Open OpenConsole.
2. Properties, Terminal, Cursor Colors, choose Inverse Color.
3. Optionally set the RGB value of the 8th color in the standard palette to RGB(128, 128, 128) on the Colors tab, but the default one will do too.
4. Type color 80 to see black text on dark grey background.
5. Make sure the inverted cursor is visible.
6. Repeat in WT with both default and experimental renderers.

[lhecker]

I've been thinking about whether this can be expressed in floating point since GPUs prefer floating point over integer operations and because it would allow us to retain the full "color accuracy" up to the end of the shader. Here's what I came up with:

Suggested change

	color = float4(1 - color.rgb, 1);
	// Make sure the cursor is always readable (see gh-3647)
	color = float4((((uint3(color.rgb * 255) ^ 0x3f) & 0xff) / 255.0).rgb, 1);
	// Make sure the cursor is always readable (see gh-3647)
	// If we imagine color to be in 0-255 instead of 0-1,
	// this effectively XORs color with 63.
	float3 ip; // integral part
	float3 frac = modf(color.rgb * (255.0f / 64.0f), ip);
	color = float4((3.0f - ip + frac) * (64.0f / 255.0f), 1);

If you're curious, this is how it changes the assembly to be more compact:

[lhecker]

Unfortunately I found in testing that this doesn't work well with ClearType. Here's how it gets messed up:

Same for GdiEngine:

Edit: I've originally tested GdiEngine incorrectly and edited my comments accordingly. I've pushed a fix for AtlasEngine at least though - see below.

[lhecker]

@alabuzhev I've pushed a change which fixes the color inversion for AtlasEngine. Here's how it looks like:

Let me know what you think, if you get a chance. I hope you didn't mind this! 🙂

[alabuzhev]

GPUs prefer floating point over integer operations

Looks interesting. I played with it a bit here:
https://trinket.io/python3/a8ae80c43e

Note that I amended both versions:

• Bits fiddling works better when divided by 256 rather than 255
• Floats version works better when 0.0000001 is added to the result.

The results are very close, except for the corner cases at 64, 128 and 192, where the floats version flips just a bit later:

I hope you didn't mind this!

Of course I don't mind, that's the whole point of "Allow edits from maintainers" :) Thank you.

BTW (rhetorically asking): How does PATINVERT actually use XOR? It can't possibly just do color ^ 63 like we do in DxEngine and AtlasEngine

Supposedly it uses it literally and color ^ 63 is exactly what happens there:

PATINVERT
Combines the colors of the specified pattern with the colors of the destination rectangle by using the Boolean XOR operator.

LTGRAY_BRUSH: A light gray, solid-color brush that is equivalent to a logical brush with the following properties:
BrushStyle: BS_SOLID
Color: 0x00C0C0C0

So, LTGRAY_BRUSH ^ 0x0C = 0xC0 ^ 0x0C = 0xCC.
This is equivalent to XORing the inverted background with inverted LTGRAY_BRUSH (or 0x3F, or 63):
~0xC0 ^ ~0x0C = 0x3F ^ 0xF3 = 0xCC.

[lhecker]

The results are very close, except for the corner cases at 64, 128 and 192, where the floats version flips just a bit later:

The algorithm I built yesterday was a bit flawed and the one I pushed into your branch today should not be off by 1 anymore. 🙂 I'm not sure about it, but I think the x array in the python script was initialized incorrectly. If I do it this way, then dividing by 255 produces the expected results: https://trinket.io/python3/3c9e9c2833

---

As an aside, I'm a bit worried that this PR might pose a bit of a visual regression for conhost, due to the worse ClearType rendering in GdiEngine:

I'm not 100% sure what to do about that yet.

[alabuzhev]

Thank you.

worse ClearType rendering in GdiEngine

It looks a bit noisy indeed, however, it is not something new.
Left to right: before, after, "legacy console", Windows 7, Windows XP:

E.g. now it is exactly how it was since at least 2009.

Interestingly, the XP version looks somewhat better, it could be worth checking how it was implemented back then.

[DHowett]

I'd guess that XP did not use ClearType, but merely Grayscale AA? Ignore me, that's clearly color fringing...

[ghost]

🎉Windows Terminal Preview v1.16.252 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download