Initializes conhost's Campbell color scheme in conhost order instead of ANSI/VT order (microsoft#1237)

* Fix this

* Swap the elements instead of having two whole tables

* Add a unittest to make @miniksa happy

Author: zadjii-msft

src/host/settings.cpp

@@ -84,7 +84,7 @@ Settings::Settings() :
     gsl::span<COLORREF> tableView = { _ColorTable, gsl::narrow<ptrdiff_t>(COLOR_TABLE_SIZE) };
     gsl::span<COLORREF> xtermTableView = { _XtermColorTable, gsl::narrow<ptrdiff_t>(XTERM_COLOR_TABLE_SIZE) };
     ::Microsoft::Console::Utils::Initialize256ColorTable(xtermTableView);
-    ::Microsoft::Console::Utils::InitializeCampbellColorTable(tableView);
+    ::Microsoft::Console::Utils::InitializeCampbellColorTableForConhost(tableView);
 }
 
 // Routine Description:
@@ -124,7 +124,7 @@ void Settings::ApplyDesktopSpecificDefaults()
     _bHistoryNoDup = FALSE;
 
     gsl::span<COLORREF> tableView = { _ColorTable, gsl::narrow<ptrdiff_t>(COLOR_TABLE_SIZE) };
-    ::Microsoft::Console::Utils::InitializeCampbellColorTable(tableView);
+    ::Microsoft::Console::Utils::InitializeCampbellColorTableForConhost(tableView);
 
     _fTrimLeadingZeros = false;
     _fEnableColorSelection = false;

src/types/inc/utils.hpp

@@ -25,6 +25,8 @@ namespace Microsoft::Console::Utils
     COLORREF ColorFromHexString(const std::string wstr);
 
     void InitializeCampbellColorTable(gsl::span<COLORREF>& table);
+    void InitializeCampbellColorTableForConhost(gsl::span<COLORREF>& table);
+    void SwapANSIColorOrderForConhost(gsl::span<COLORREF>& table);
     void Initialize256ColorTable(gsl::span<COLORREF>& table);
     void SetColorTableAlpha(gsl::span<COLORREF>& table, const BYTE newAlpha);
 

src/types/ut_types/UtilsTests.cpp

@@ -6,6 +6,7 @@
 #include "..\..\inc\consoletaeftemplates.hpp"
 
 #include "..\inc\utils.hpp"
+#include <conattrs.hpp>
 
 using namespace WEX::Common;
 using namespace WEX::Logging;
@@ -17,28 +18,60 @@ class UtilsTests
 {
     TEST_CLASS(UtilsTests);
 
-    TEST_METHOD(TestClampToShortMax)
-    {
-        const short min = 1;
-
-        // Test outside the lower end of the range
-        const short minExpected = min;
-        auto minActual = ClampToShortMax(0, min);
-        VERIFY_ARE_EQUAL(minExpected, minActual);
-
-        // Test negative numbers
-        const short negativeExpected = min;
-        auto negativeActual = ClampToShortMax(-1, min);
-        VERIFY_ARE_EQUAL(negativeExpected, negativeActual);
-
-        // Test outside the upper end of the range
-        const short maxExpected = SHRT_MAX;
-        auto maxActual = ClampToShortMax(50000, min);
-        VERIFY_ARE_EQUAL(maxExpected, maxActual);
-
-        // Test within the range
-        const short withinRangeExpected = 100;
-        auto withinRangeActual = ClampToShortMax(withinRangeExpected, min);
-        VERIFY_ARE_EQUAL(withinRangeExpected, withinRangeActual);
-    }
+    TEST_METHOD(TestClampToShortMax);
+    TEST_METHOD(TestSwapColorPalette);
 };
+
+void UtilsTests::TestClampToShortMax()
+{
+    const short min = 1;
+
+    // Test outside the lower end of the range
+    const short minExpected = min;
+    auto minActual = ClampToShortMax(0, min);
+    VERIFY_ARE_EQUAL(minExpected, minActual);
+
+    // Test negative numbers
+    const short negativeExpected = min;
+    auto negativeActual = ClampToShortMax(-1, min);
+    VERIFY_ARE_EQUAL(negativeExpected, negativeActual);
+
+    // Test outside the upper end of the range
+    const short maxExpected = SHRT_MAX;
+    auto maxActual = ClampToShortMax(50000, min);
+    VERIFY_ARE_EQUAL(maxExpected, maxActual);
+
+    // Test within the range
+    const short withinRangeExpected = 100;
+    auto withinRangeActual = ClampToShortMax(withinRangeExpected, min);
+    VERIFY_ARE_EQUAL(withinRangeExpected, withinRangeActual);
+}
+void UtilsTests::TestSwapColorPalette()
+{
+    std::array<COLORREF, COLOR_TABLE_SIZE> terminalTable;
+    std::array<COLORREF, COLOR_TABLE_SIZE> consoleTable;
+
+    gsl::span<COLORREF> terminalTableView = { &terminalTable[0], gsl::narrow<ptrdiff_t>(terminalTable.size()) };
+    gsl::span<COLORREF> consoleTableleView = { &consoleTable[0], gsl::narrow<ptrdiff_t>(consoleTable.size()) };
+
+    // First set up the colors
+    InitializeCampbellColorTable(terminalTableView);
+    InitializeCampbellColorTableForConhost(consoleTableleView);
+
+    VERIFY_ARE_EQUAL(terminalTable[0], consoleTable[0]);
+    VERIFY_ARE_EQUAL(terminalTable[1], consoleTable[4]);
+    VERIFY_ARE_EQUAL(terminalTable[2], consoleTable[2]);
+    VERIFY_ARE_EQUAL(terminalTable[3], consoleTable[6]);
+    VERIFY_ARE_EQUAL(terminalTable[4], consoleTable[1]);
+    VERIFY_ARE_EQUAL(terminalTable[5], consoleTable[5]);
+    VERIFY_ARE_EQUAL(terminalTable[6], consoleTable[3]);
+    VERIFY_ARE_EQUAL(terminalTable[7], consoleTable[7]);
+    VERIFY_ARE_EQUAL(terminalTable[8], consoleTable[8]);
+    VERIFY_ARE_EQUAL(terminalTable[9], consoleTable[12]);
+    VERIFY_ARE_EQUAL(terminalTable[10], consoleTable[10]);
+    VERIFY_ARE_EQUAL(terminalTable[11], consoleTable[14]);
+    VERIFY_ARE_EQUAL(terminalTable[12], consoleTable[9]);
+    VERIFY_ARE_EQUAL(terminalTable[13], consoleTable[13]);
+    VERIFY_ARE_EQUAL(terminalTable[14], consoleTable[11]);
+    VERIFY_ARE_EQUAL(terminalTable[15], consoleTable[15]);
+}

src/types/utils.cpp

@@ -117,7 +117,8 @@ bool Utils::IsValidHandle(const HANDLE handle) noexcept
 }
 
 // Function Description:
-// - Fill the first 16 entries of a given color table with the Campbell color scheme
+// - Fill the first 16 entries of a given color table with the Campbell color
+//   scheme, in the ANSI/VT RGB order.
 // Arguments:
 // - table: a color table with at least 16 entries
 // Return Value:
@@ -146,6 +147,35 @@ void Utils::InitializeCampbellColorTable(gsl::span<COLORREF>& table)
     // clang-format on
 }
 
+// Function Description:
+// - Fill the first 16 entries of a given color table with the Campbell color
+//   scheme, in the Windows BGR order.
+// Arguments:
+// - table: a color table with at least 16 entries
+// Return Value:
+// - <none>, throws if the table has less that 16 entries
+void Utils::InitializeCampbellColorTableForConhost(gsl::span<COLORREF>& table)
+{
+    THROW_HR_IF(E_INVALIDARG, table.size() < 16);
+    InitializeCampbellColorTable(table);
+    SwapANSIColorOrderForConhost(table);
+}
+
+// Function Description:
+// - modifies in-place the given color table from ANSI (RGB) order to Console order (BRG).
+// Arguments:
+// - table: a color table with at least 16 entries
+// Return Value:
+// - <none>, throws if the table has less that 16 entries
+void Utils::SwapANSIColorOrderForConhost(gsl::span<COLORREF>& table)
+{
+    THROW_HR_IF(E_INVALIDARG, table.size() < 16);
+    std::swap(table[1], table[4]);
+    std::swap(table[3], table[6]);
+    std::swap(table[9], table[12]);
+    std::swap(table[11], table[14]);
+}
+
 // Function Description:
 // - Fill the first 255 entries of a given color table with the default values
 //      of a full 256-color table