From 608c660bfb6263dd1b896538a11cffc1d6fb2d26 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 27 Aug 2019 14:30:22 -0500 Subject: [PATCH] Use the entire 256 table to lookup the color Fixes #1223 We'll lookup a color (for rendering) using the _entire_ 256 color table. When setting a 256 color, we'll set the TextColor to that index, instead of the current color at that index. Everything else is unchanged. **TODO**: This probably affects which TextAttributes will pass IsLegacy. 256 color ones will now be "legacy". This is bad, we should make sure to not leave it that way. --- src/host/VtIo.cpp | 8 ++++---- src/host/getset.cpp | 32 +++++++++++++++++++++++--------- src/host/settings.cpp | 12 +++++++++--- src/host/settings.hpp | 1 + src/host/telemetry.cpp | 2 +- src/interactivity/win32/menu.cpp | 4 ++-- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index cf7ab1ecb..b67ea8fbf 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -161,14 +161,14 @@ VtIo::VtIo() : gci, initialViewport, gci.GetColorTable(), - static_cast(gci.GetColorTableSize())); + static_cast(gci.GetLegacyColorTableSize())); break; case VtIoMode::XTERM: _pVtRenderEngine = std::make_unique(std::move(_hOutput), gci, initialViewport, gci.GetColorTable(), - static_cast(gci.GetColorTableSize()), + static_cast(gci.GetLegacyColorTableSize()), false); break; case VtIoMode::XTERM_ASCII: @@ -176,7 +176,7 @@ VtIo::VtIo() : gci, initialViewport, gci.GetColorTable(), - static_cast(gci.GetColorTableSize()), + static_cast(gci.GetLegacyColorTableSize()), true); break; case VtIoMode::WIN_TELNET: @@ -184,7 +184,7 @@ VtIo::VtIo() : gci, initialViewport, gci.GetColorTable(), - static_cast(gci.GetColorTableSize())); + static_cast(gci.GetLegacyColorTableSize())); break; default: return E_FAIL; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index f576c1d94..90112e1bf 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -917,24 +917,38 @@ void DoSrvPrivateSetConsoleXtermTextAttribute(SCREEN_INFORMATION& screenInfo, const int iXtermTableEntry, const bool fIsForeground) { - const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + // const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& buffer = screenInfo.GetActiveBuffer(); TextAttribute NewAttributes = buffer.GetAttributes(); - COLORREF rgbColor; - if (iXtermTableEntry < COLOR_TABLE_SIZE) - { - //Convert the xterm index to the win index - WORD iWinEntry = ::XtermToWindowsIndex(iXtermTableEntry); + // //////////////////////// + // COLORREF rgbColor; + // if (iXtermTableEntry < COLOR_TABLE_SIZE) + // { + // //Convert the xterm index to the win index + // WORD iWinEntry = ::XtermToWindowsIndex(iXtermTableEntry); - rgbColor = gci.GetColorTableEntry(iWinEntry); + // rgbColor = gci.GetColorTableEntry(iWinEntry); + // } + // else + // { + // rgbColor = gci.GetColorTableEntry(iXtermTableEntry); + // } + + // NewAttributes.SetColor(rgbColor, fIsForeground); + // //////////////////////// + std::optional newFG{ std::nullopt }; // = fIsForeground ? iXtermTableEntry : std::nullopt; + std::optional newBG{ std::nullopt }; // = fIsForeground ? std::nullopt : iXtermTableEntry; + if (fIsForeground) + { + newFG = static_cast(iXtermTableEntry); } else { - rgbColor = gci.GetColorTableEntry(iXtermTableEntry); + newBG = static_cast(iXtermTableEntry); } - NewAttributes.SetColor(rgbColor, fIsForeground); + NewAttributes.SetIndexedAttributes(newFG, newBG); buffer.SetAttributes(NewAttributes); } diff --git a/src/host/settings.cpp b/src/host/settings.cpp index 413809ecb..88d70c19d 100644 --- a/src/host/settings.cpp +++ b/src/host/settings.cpp @@ -740,12 +740,18 @@ void Settings::UnsetStartupFlag(const DWORD dwFlagToUnset) _dwStartupFlags &= ~dwFlagToUnset; } -const size_t Settings::GetColorTableSize() const +const size_t Settings::GetLegacyColorTableSize() const { return COLOR_TABLE_SIZE; // return ARRAYSIZE(_ColorTable); } +const size_t Settings::GetColorTableSize() const +{ + return XTERM_COLOR_TABLE_SIZE; + // return ARRAYSIZE(_ColorTable); +} + COLORREF Settings::GetColorTableEntry(const size_t index) const { return _256ColorTable[index]; @@ -913,7 +919,7 @@ bool Settings::GetUseDx() const noexcept COLORREF Settings::CalculateDefaultForeground() const noexcept { const auto fg = GetDefaultForegroundColor(); - return fg != INVALID_COLOR ? fg : ForegroundColor(GetFillAttribute(), GetColorTable(), GetColorTableSize()); + return fg != INVALID_COLOR ? fg : ForegroundColor(GetFillAttribute(), GetColorTable(), GetLegacyColorTableSize()); } // Method Description: @@ -928,7 +934,7 @@ COLORREF Settings::CalculateDefaultForeground() const noexcept COLORREF Settings::CalculateDefaultBackground() const noexcept { const auto bg = GetDefaultBackgroundColor(); - return bg != INVALID_COLOR ? bg : BackgroundColor(GetFillAttribute(), GetColorTable(), GetColorTableSize()); + return bg != INVALID_COLOR ? bg : BackgroundColor(GetFillAttribute(), GetColorTable(), GetLegacyColorTableSize()); } // Method Description: diff --git a/src/host/settings.hpp b/src/host/settings.hpp index 620f939a9..8bffd38b9 100644 --- a/src/host/settings.hpp +++ b/src/host/settings.hpp @@ -157,6 +157,7 @@ public: void SetHistoryNoDup(const bool fHistoryNoDup); const COLORREF* const GetColorTable() const; + const size_t GetLegacyColorTableSize() const; const size_t GetColorTableSize() const; void SetColorTable(_In_reads_(cSize) const COLORREF* const pColorTable, const size_t cSize); void SetColorTableEntry(const size_t index, const COLORREF ColorValue); diff --git a/src/host/telemetry.cpp b/src/host/telemetry.cpp index ca93c8ed1..ed6a69504 100644 --- a/src/host/telemetry.cpp +++ b/src/host/telemetry.cpp @@ -419,7 +419,7 @@ void Telemetry::WriteFinalTraceLog() TraceLoggingBool(gci.GetQuickEdit(), "QuickEdit"), TraceLoggingValue(gci.GetWindowAlpha(), "WindowAlpha"), TraceLoggingBool(gci.GetWrapText(), "WrapText"), - TraceLoggingUInt32Array((UINT32 const*)gci.GetColorTable(), (UINT16)gci.GetColorTableSize(), "ColorTable"), + TraceLoggingUInt32Array((UINT32 const*)gci.GetColorTable(), (UINT16)gci.GetLegacyColorTableSize(), "ColorTable"), TraceLoggingValue(gci.CP, "CodePageInput"), TraceLoggingValue(gci.OutputCP, "CodePageOutput"), TraceLoggingValue(gci.GetFontSize().X, "FontSizeX"), diff --git a/src/interactivity/win32/menu.cpp b/src/interactivity/win32/menu.cpp index b1d11151d..f923a7104 100644 --- a/src/interactivity/win32/menu.cpp +++ b/src/interactivity/win32/menu.cpp @@ -337,7 +337,7 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults) pStateInfo->NumberOfHistoryBuffers = gci.GetNumberOfHistoryBuffers(); pStateInfo->HistoryNoDup = !!(gci.Flags & CONSOLE_HISTORY_NODUP); - memmove(pStateInfo->ColorTable, gci.GetColorTable(), gci.GetColorTableSize() * sizeof(COLORREF)); + memmove(pStateInfo->ColorTable, gci.GetColorTable(), gci.GetLegacyColorTableSize() * sizeof(COLORREF)); // Create mutable copies of the titles so the propsheet can do something with them. if (gci.GetOriginalTitle().length() > 0) @@ -564,7 +564,7 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo) } } - gci.SetColorTable(pStateInfo->ColorTable, gci.GetColorTableSize()); + gci.SetColorTable(pStateInfo->ColorTable, gci.GetLegacyColorTableSize()); // Ensure that attributes only contain color specification. WI_ClearAllFlags(pStateInfo->ScreenAttributes, ~(FG_ATTRS | BG_ATTRS));