Optimize the binary size of the XOrg color table

[skyline75489]

Summary of the Pull Request

This optimizes the binary size of the xorg color table.

References

PR Checklist

• Closes #xxx
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated.
• 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: #xxx

Detailed Description of the Pull Request / Additional comments

I couldn't sleep at night thinking that after years of accusing Windows being bloated and literally making it even more bloated with my hands. So here you go. The mediocre yet working solution. This reduces the binary size to 1051k (1067k before) while keeping the code maintainable for human beings.

Validation Steps Performed

Tests added.

[skyline75489]

You know why I don't use "yello3w"? Because spell check complains about "yello" is not a word...Thank god we have "royal blue"

[DHowett]

Hey, this is real clever.

I'm wondering. Can we use an algorithmic transformation from grayX to a color? If so, we can remove 100 color quads and save an additional 400 bytes.

It looks like the formula is, roughly...

component = ((255 * X) + 50) / 100
grayX = { component, component, component }

We don't have to match xorg perfectly, but I included the 50 there so that when we do integer truncation (like floor), the colors would match up better.

[DHowett]

Here's my proposal!

[DHowett]

From e29981662b23e23dd6c3bba1dd75f6d69f543364 Mon Sep 17 00:00:00 2001
From: Dustin Howett <duhowett@microsoft.com>
Date: Thu, 15 Oct 2020 11:34:52 -0700
Subject: [PATCH] remove all the grays

---
 src/types/colorTable.cpp | 122 ++++-----------------------------------
 1 file changed, 12 insertions(+), 110 deletions(-)

diff --git a/src/types/colorTable.cpp b/src/types/colorTable.cpp
index c716e7296..c4d18297e 100644
--- a/src/types/colorTable.cpp
+++ b/src/types/colorTable.cpp
@@ -396,7 +396,9 @@ static constexpr til::presorted_static_map xorgAppColorTable{
     std::pair{ "fuchsia"sv, til::color{ 255, 0, 255 } },
     std::pair{ "gainsboro"sv, til::color{ 220, 220, 220 } },
     std::pair{ "ghostwhite"sv, til::color{ 248, 248, 255 } },
+    std::pair{ "gray"sv, til::color{ 190, 190, 190 } },
     std::pair{ "greenyellow"sv, til::color{ 173, 255, 47 } },
+    std::pair{ "grey"sv, til::color{ 190, 190, 190 } },
     std::pair{ "indigo"sv, til::color{ 75, 0, 130 } },
     std::pair{ "lavender"sv, til::color{ 230, 230, 250 } },
     std::pair{ "lawngreen"sv, til::color{ 124, 252, 0 } },
@@ -452,108 +454,6 @@ static constexpr til::presorted_static_map xorgAppColorTable{
     std::pair{ "yellowgreen"sv, til::color{ 154, 205, 50 } }
 };
 
-static constexpr std::array<til::color, 100> xorgAppGrayColorTable{
-    /* delete this entire thing */
-};
 // Function Description:
 // - Fill the first 16 entries of a given color table with the Campbell color
 //   scheme, in the ANSI/VT RGB order.
@@ -671,18 +571,20 @@ try
         }
     }
 
-    const auto colorIter = xorgAppColorTable.find(name);
-    if (colorIter != xorgAppColorTable.end())
+    if ((name == "gray" || name == "grey") && foundVariant)
     {
-        return colorIter->second;
+        if (variantIndex > 100) // size_t is unsigned, so >=0 is implicit
+        {
+            return std::nullopt;
+        }
+        const auto component{ ::base::saturated_cast<uint8_t>(((variantIndex * 255) + 50) / 100) };
+        return til::color{ component, component, component };
     }
 
-    if (name == "gray" || name == "grey")
+    const auto colorIter = xorgAppColorTable.find(name);
+    if (colorIter != xorgAppColorTable.end())
     {
-        if (variantIndex < xorgAppGrayColorTable.size())
-        {
-            return xorgAppGrayColorTable.at(variantIndex);
-        }
+        return colorIter->second;
     }
 
     return std::nullopt;
-- 
2.28.0.windows.1

[DHowett]

With the above diff, colorTable.obj contributes less than 5kb to the final product! 😄

# 78 colors ...
#   8 bytes each for pointer+size
#   5 variants, 4 bytes each for the color data
# 718 bytes for 0-terminated color names
# --AND--
# 84 colors ...
#   8 bytes each for pointer+size
#   4 bytes each for the color data
# 955 bytes for 8-terminated color names
#
  2902 = (78 * 8) + (78 * 5 * 4) + 718
+ 1963 = (84 * 8) + (84   *   4) + 955
------
  4865

This is really excellent work you've done shrinking the color table.

[skyline75489]

@DHowett That's even better. I'd say it's pretty impressive to have 600+ colors in just 5kb.

And I just realize that, this PR also speeds up the lookup process, because it checks foundVariant prematurely to determine which is the target color table. Well at least for the gray colors.

[skyline75489]

Should I maybe also do a premature check for color with variants also? You know, for a string like something1, it can only be found possibly in xorgAppVariantColorTable, because xorgAppColorTable contains nothing numerical. Will this be too aggressive?

[DHowett]

There's probably no reason to check 😄

[skyline75489]

Yeah, I tried something like that but it will complicate the logic. I'd rather keep the logic simple and clean. And the search space has reduced from 600+ to ~160. So it's probably not urgent to try to make it even faster.

So I guess it's done. Now I can sleep at night 😄

[miniksa]

I am very excited about this. Thank you for all the effort here.

[DHowett]

This is amazing. Thanks again!

[DHowett]

@msftbot merge this in minutes 3

[ghost]

Hello @DHowett!

I think you told me that you want to delay the approval for a certain amount of time, but I am not confident that I have understood you correctly.

Please try rephrasing your instruction to me.

[DHowett]

@msftbot merge 3 in minutes this

[ghost]

Hello @DHowett!

I think you told me that you want to delay the approval for a certain amount of time, but I am not confident that I have understood you correctly.

Please try rephrasing your instruction to me.

[DHowett]

@msftbot merge this in 3 minutes

[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 won't merge this pull request until after the UTC date Fri, 16 Oct 2020 00:09:05 GMT, which is in 3 minutes

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".

[ghost]

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

Handy links:

• Release Notes
• Store Download