Use Oklab for text and cursor contrast adjustments #15283

lhecker · 2023-05-03T18:51:18Z

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 ✅

lhecker · 2023-05-03T19:32:01Z

Here's a (temporary) preview of the interactive color_nudging.html file: https://blob.hecker.io/files/color_nudging.html

Before:

After:

lhecker · 2023-05-04T00:08:00Z

doc/color_nudging.html

@@ -0,0 +1,235 @@
+<!DOCTYPE html>


Preview: https://blob.hecker.io/files/color_nudging.html

lhecker · 2023-05-04T00:08:45Z

src/buffer/out/TextColor.cpp

@@ -64,7 +64,7 @@ bool TextColor::CanBeBrightened() const noexcept

 bool TextColor::IsLegacy() const noexcept
 {
-    return IsIndex16() || (IsIndex256() && _index < 16);
+    return (IsIndex16() || IsIndex256()) && _index < 16;


This allows for better compiler optimizations, because Index16 and Index256 are two neighboring values in the enum. The compiler optimizes the || away entirely.

Nit: This is the sort of scenario where I'd be inclined to add a static_assert checking that those two enums are in fact neighboring values, and a comment explaining why that matters. That way if someone ever messes with that enum in the future, they'll be notified that they'll be losing this optimization.

Since this is a enum class, there are no implicit operators to turn them into integers. I don't really want to write the rather verbose code required to do static_assert checks, so I've instead opted to add a comment to the declaration instead, which points this out.

lhecker · 2023-05-04T00:09:21Z

src/buffer/out/TextColor.cpp

@@ -82,6 +82,11 @@ bool TextColor::IsDefault() const noexcept
    return _meta == ColorType::IsDefault;
 }

+bool TextColor::IsDefaultOrIndex16() const noexcept
+{
+    return _meta != ColorType::IsRgb && _index < 16;


ColorType::IsDefault has a _index of 0 at all times as mandated by the constructor.

Shouldn't this be named IsDefaultOrLegacy? It's not only matching IsIndex16 - it's also matching IsIndex256 with an index < 16 - which is exactly what IsLegacy covers.

lhecker · 2023-05-04T00:10:10Z

src/buffer/out/TextColor.h

+    IsDefault,
+    IsIndex16,
+    IsIndex256,
+    IsRgb


The more consistent ordering from "less available colors" to "more" allows for better compiler optimizations, as mentioned above, and also IMO make more sense subjectively speaking.

lhecker · 2023-05-04T00:10:46Z

src/renderer/atlas/BackendD3D.cpp

+        if (cursorColor == 0xffffffff)
+        {
+            background = bg ^ 0xffffff;
+            foreground = 0xffffffff;
+        }


This is the first inverted cursor handling block.

lhecker · 2023-05-04T00:12:31Z

src/renderer/atlas/BackendD3D.cpp

+
+        // The legacy console used to invert colors by just doing `bg ^ 0xc0c0c0`. This resulted
+        // in a minimum squared distance of just 0.029195 across all possible color combinations.
+        background = ColorFix::GetPerceivableColor(background, bg, 0.25f * 0.25f);


The minimum distance given to GetPerceivableColor is the squared distance. That way we don't need to do any sqrt() inside it (which is very slow relatively speaking). The background of the cursor gets half the min. distance the text gets, because I noticed that the background color really just doesn't need it.

Is the 0.029195 number in the comment valid for Oklab?

Oh, I see you added that number in another commit of this PR, so I presume it is valid.

Yeah, you could use it as an argument to that function.

BTW I've just rechecked it and the average ΔEOK of the previous XOR method is 0.432123. It's just that the minimum is 0.170865 (which results in 0.029195 if squared). It might be worth it to increase the 0.25f a bit, but that looked a little weird at times, because it resulted in a way too strong contrast. Or maybe I should combine the XOR and Oklab methods together? Argh I'm not sure what's best.

lhecker · 2023-05-04T00:14:23Z

src/renderer/base/RenderSettings.cpp

@@ -11,10 +11,6 @@
 using namespace Microsoft::Console::Render;
 using Microsoft::Console::Utils::InitializeColorTable;

-static constexpr size_t AdjustedFgIndex{ 16 };
-static constexpr size_t AdjustedBgIndex{ 17 };
-static constexpr size_t AdjustedBrightFgIndex{ 18 };


Suppress white space changes if you haven't done so already. It'll effectively remove almost all changes in this file.

lhecker · 2023-05-04T00:18:40Z

src/types/ColorFix.cpp

+    r = (r > 0.0031308f ? (1.055f * powf(r, 1.0f / 2.4f) - 0.055f) : 12.92f * r) * 255.0f;
+    g = (g > 0.0031308f ? (1.055f * powf(g, 1.0f / 2.4f) - 0.055f) : 12.92f * g) * 255.0f;
+    b = (b > 0.0031308f ? (1.055f * powf(b, 1.0f / 2.4f) - 0.055f) : 12.92f * b) * 255.0f;


Fabian Giesen (ryg) has also written some great linear -> sRGB routines and published them here: https://gist.github.com/rygorous/2203834
But I felt like that's a bit too soon for now just like my other SIMD optimizations for this. I'll work towards that in a follow-up PR, because it requires me to write NEON intrinsics and I'm not feeling comfortable doing that without extra validation.

j4james · 2023-05-04T09:23:31Z

src/buffer/out/TextColor.cpp

@@ -64,7 +64,7 @@ bool TextColor::CanBeBrightened() const noexcept

 bool TextColor::IsLegacy() const noexcept
 {
-    return IsIndex16() || (IsIndex256() && _index < 16);
+    return (IsIndex16() || IsIndex256()) && _index < 16;


Nit: This is the sort of scenario where I'd be inclined to add a static_assert checking that those two enums are in fact neighboring values, and a comment explaining why that matters. That way if someone ever messes with that enum in the future, they'll be notified that they'll be losing this optimization.

j4james · 2023-05-04T09:23:55Z

src/buffer/out/TextColor.cpp

@@ -82,6 +82,11 @@ bool TextColor::IsDefault() const noexcept
    return _meta == ColorType::IsDefault;
 }

+bool TextColor::IsDefaultOrIndex16() const noexcept
+{
+    return _meta != ColorType::IsRgb && _index < 16;


Shouldn't this be named IsDefaultOrLegacy? It's not only matching IsIndex16 - it's also matching IsIndex256 with an index < 16 - which is exactly what IsLegacy covers.

j4james · 2023-05-04T10:14:37Z

src/renderer/base/RenderSettings.cpp

+        if (
+            _renderMode.any(Mode::IndexedDistinguishableColors, Mode::AlwaysDistinguishableColors) &&
            fg != bg &&
-            GetRenderMode(Mode::AlwaysDistinguishableColors))
+            (_renderMode.test(Mode::AlwaysDistinguishableColors) || (fgTextColor.IsDefaultOrIndex16() && bgTextColor.IsDefaultOrIndex16())))
        {
-            fg = ColorFix::GetPerceivableColor(fg, bg);
+            fg = ColorFix::GetPerceivableColor(fg, bg, 0.5f * 0.5f);
        }


Note that there's a slight change in behavior here from the previous implementation (I think). When you don't have the AlwaysDistinguishableColors mode set, it would previously not have applied to dimmed colors, but it will do now (assuming I'm reading this correctly). I'm not sure whether anyone would care either way, but I thought it worth mentioning.

Oh wow, I totally forgot to mention that in this PR!

I did it intentionally. Since GetPerceivableColor could be considered a very important accessibility feature, dim colors need to be just as visible as non-dim colors are for people relying on this feature. Since the new GetPerceivableColor function is fairly "accurate" at not changing the lightness more than necessary, the result still looks quite okay to me.

zadjii-msft

this is insane but also amazingly effective

zadjii-msft · 2023-05-04T19:26:31Z

src/types/ColorFix.cpp

+    0x1.7da5a4p-1f,  0x1.8181a8p-1f,  0x1.85637cp-1f,   0x1.894b2p-1f,  0x1.8d3896p-1f,  0x1.912be2p-1f,  0x1.952504p-1f,  0x1.992402p-1f,  0x1.9d28dcp-1f,  0x1.a13396p-1f,   0x1.a5443p-1f,  0x1.a95aaep-1f,  0x1.ad7714p-1f,   0x1.b1996p-1f,  0x1.b5c19ap-1f,   0x1.b9efcp-1f,
+    0x1.be23d8p-1f,  0x1.c25ddcp-1f,  0x1.c69ddep-1f,  0x1.cae3d2p-1f,  0x1.cf2fc6p-1f,  0x1.d381acp-1f,  0x1.d7d99ap-1f,  0x1.dc377ep-1f,   0x1.e09b7p-1f,  0x1.e5055ep-1f,  0x1.e9755cp-1f,  0x1.edeb5cp-1f,  0x1.f26772p-1f,  0x1.f6e98cp-1f,   0x1.fb71cp-1f,         0x1p+0f,
+};
+// clang-format on


for the record, this whole table looks insane and no human can actually read that.

but it works

src/types/ColorFix.cpp

zadjii-msft · 2023-05-04T19:30:12Z

@msftbot make sure @PankajBhojwani signs off

/rip msftbot

DHowett · 2023-05-08T18:50:02Z

src/types/inc/ColorFix.hpp

-    void _ToLab() noexcept;
-    void _ToRGB();
-};
+    COLORREF GetPerceivableColor(COLORREF color, COLORREF reference, float minSquaredDistance) noexcept;


no til :(

DHowett · 2023-05-08T18:51:07Z

src/renderer/atlas/BackendD3D.cpp

+        }
+
+        // The legacy console used to invert colors by just doing `bg ^ 0xc0c0c0`. This resulted
+        // in a minimum squared distance of just 0.029195 across all possible color combinations.


so this ensures that all cursor foregrounds on all colors are visible?

It should! ...in theory at least.

DHowett · 2023-05-08T18:54:10Z

src/types/ColorFix.cpp

+    // It's similar to the well known "fast inverse square root" trick. Lots of numbers around 709921077 perform
+    // at least equally well to 709921077, and it is unknown how and why 709921077 was chosen specifically.
+    FP32 fp{ .f = a }; // evil floating point bit level hacking
+    fp.u = fp.u / 3 + 709921077; // what the fuck?


lmao you guys

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

microsoft-github-policy-service bot added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. labels May 3, 2023

This comment has been minimized.

Sign in to view

lhecker added this to the Terminal v1.18 milestone May 3, 2023

This comment has been minimized.

Sign in to view

Use Oklab for text and cursor contrast adjustments

40c8729

lhecker force-pushed the dev/lhecker/oklab branch from 4537da6 to 40c8729 Compare May 3, 2023 19:02

Avoid Unicode in HTML

47e341e

lhecker mentioned this pull request May 3, 2023

Refine customization around text color adjustment #14940

Open

Simplify minSquaredDistance parameter

da130ca

lhecker commented May 4, 2023

View reviewed changes

j4james reviewed May 4, 2023

View reviewed changes

Address feedback

7eebcf2

lhecker force-pushed the dev/lhecker/oklab branch from 807e830 to 7eebcf2 Compare May 4, 2023 12:33

3x speedup

9fec45f

carlos-zamora requested a review from PankajBhojwani May 4, 2023 16:39

Fix AuditMode failures

1477c0d

lhecker added a commit that referenced this pull request May 4, 2023

PRE-MERGE #15283

957a229

zadjii-msft approved these changes May 4, 2023

View reviewed changes

Prettify the sRGB table, Address feedback

192ec2b

zadjii-msft added the zBugBash-Consider label May 5, 2023

DHowett approved these changes May 8, 2023

View reviewed changes

lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 8, 2023

microsoft-github-policy-service bot merged commit 4dd9493 into main May 8, 2023

microsoft-github-policy-service bot deleted the dev/lhecker/oklab branch May 8, 2023 19:16

3N4N mentioned this pull request May 9, 2023

TUI: cursor "reverse" does not work in Windows Terminal neovim/neovim#21526

Closed

lhecker mentioned this pull request May 24, 2023

Something wrong with colors #15415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Oklab for text and cursor contrast adjustments #15283

Use Oklab for text and cursor contrast adjustments #15283

lhecker commented May 3, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

lhecker commented May 3, 2023

lhecker May 4, 2023

lhecker May 4, 2023 •

edited

Loading

j4james May 4, 2023

lhecker May 4, 2023

lhecker May 4, 2023

j4james May 4, 2023

lhecker May 4, 2023

lhecker May 4, 2023

lhecker May 4, 2023 •

edited

Loading

KalleOlaviNiemitalo May 4, 2023

KalleOlaviNiemitalo May 4, 2023

lhecker May 4, 2023 •

edited

Loading

lhecker May 4, 2023

lhecker May 4, 2023 •

edited

Loading

j4james May 4, 2023

j4james May 4, 2023

j4james May 4, 2023

lhecker May 4, 2023 •

edited

Loading

zadjii-msft left a comment

zadjii-msft May 4, 2023

zadjii-msft commented May 4, 2023

DHowett May 8, 2023

DHowett May 8, 2023

lhecker May 8, 2023

DHowett May 8, 2023

Use Oklab for text and cursor contrast adjustments #15283

Use Oklab for text and cursor contrast adjustments #15283

Conversation

lhecker commented May 3, 2023 • edited Loading

Validation Steps Performed

This comment has been minimized.

This comment has been minimized.

lhecker commented May 3, 2023

Choose a reason for hiding this comment

lhecker May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker May 4, 2023 • edited Loading

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft commented May 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker commented May 3, 2023 •

edited

Loading

lhecker May 4, 2023 •

edited

Loading

lhecker May 4, 2023 •

edited

Loading

lhecker May 4, 2023 •

edited

Loading

lhecker May 4, 2023 •

edited

Loading

lhecker May 4, 2023 •

edited

Loading