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.
This commit is contained in:
Mike Griese 2019-08-27 14:30:22 -05:00
parent 92830abfef
commit 608c660bfb
6 changed files with 40 additions and 19 deletions

View file

@ -161,14 +161,14 @@ VtIo::VtIo() :
gci, gci,
initialViewport, initialViewport,
gci.GetColorTable(), gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize())); static_cast<WORD>(gci.GetLegacyColorTableSize()));
break; break;
case VtIoMode::XTERM: case VtIoMode::XTERM:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput), _pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci, gci,
initialViewport, initialViewport,
gci.GetColorTable(), gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()), static_cast<WORD>(gci.GetLegacyColorTableSize()),
false); false);
break; break;
case VtIoMode::XTERM_ASCII: case VtIoMode::XTERM_ASCII:
@ -176,7 +176,7 @@ VtIo::VtIo() :
gci, gci,
initialViewport, initialViewport,
gci.GetColorTable(), gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()), static_cast<WORD>(gci.GetLegacyColorTableSize()),
true); true);
break; break;
case VtIoMode::WIN_TELNET: case VtIoMode::WIN_TELNET:
@ -184,7 +184,7 @@ VtIo::VtIo() :
gci, gci,
initialViewport, initialViewport,
gci.GetColorTable(), gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize())); static_cast<WORD>(gci.GetLegacyColorTableSize()));
break; break;
default: default:
return E_FAIL; return E_FAIL;

View file

@ -917,24 +917,38 @@ void DoSrvPrivateSetConsoleXtermTextAttribute(SCREEN_INFORMATION& screenInfo,
const int iXtermTableEntry, const int iXtermTableEntry,
const bool fIsForeground) const bool fIsForeground)
{ {
const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); // const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& buffer = screenInfo.GetActiveBuffer(); auto& buffer = screenInfo.GetActiveBuffer();
TextAttribute NewAttributes = buffer.GetAttributes(); TextAttribute NewAttributes = buffer.GetAttributes();
COLORREF rgbColor; // ////////////////////////
if (iXtermTableEntry < COLOR_TABLE_SIZE) // COLORREF rgbColor;
{ // if (iXtermTableEntry < COLOR_TABLE_SIZE)
//Convert the xterm index to the win index // {
WORD iWinEntry = ::XtermToWindowsIndex(iXtermTableEntry); // //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<BYTE> newFG{ std::nullopt }; // = fIsForeground ? iXtermTableEntry : std::nullopt;
std::optional<BYTE> newBG{ std::nullopt }; // = fIsForeground ? std::nullopt : iXtermTableEntry;
if (fIsForeground)
{
newFG = static_cast<BYTE>(iXtermTableEntry);
} }
else else
{ {
rgbColor = gci.GetColorTableEntry(iXtermTableEntry); newBG = static_cast<BYTE>(iXtermTableEntry);
} }
NewAttributes.SetColor(rgbColor, fIsForeground); NewAttributes.SetIndexedAttributes(newFG, newBG);
buffer.SetAttributes(NewAttributes); buffer.SetAttributes(NewAttributes);
} }

View file

@ -740,12 +740,18 @@ void Settings::UnsetStartupFlag(const DWORD dwFlagToUnset)
_dwStartupFlags &= ~dwFlagToUnset; _dwStartupFlags &= ~dwFlagToUnset;
} }
const size_t Settings::GetColorTableSize() const const size_t Settings::GetLegacyColorTableSize() const
{ {
return COLOR_TABLE_SIZE; return COLOR_TABLE_SIZE;
// return ARRAYSIZE(_ColorTable); // 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 COLORREF Settings::GetColorTableEntry(const size_t index) const
{ {
return _256ColorTable[index]; return _256ColorTable[index];
@ -913,7 +919,7 @@ bool Settings::GetUseDx() const noexcept
COLORREF Settings::CalculateDefaultForeground() const noexcept COLORREF Settings::CalculateDefaultForeground() const noexcept
{ {
const auto fg = GetDefaultForegroundColor(); const auto fg = GetDefaultForegroundColor();
return fg != INVALID_COLOR ? fg : ForegroundColor(GetFillAttribute(), GetColorTable(), GetColorTableSize()); return fg != INVALID_COLOR ? fg : ForegroundColor(GetFillAttribute(), GetColorTable(), GetLegacyColorTableSize());
} }
// Method Description: // Method Description:
@ -928,7 +934,7 @@ COLORREF Settings::CalculateDefaultForeground() const noexcept
COLORREF Settings::CalculateDefaultBackground() const noexcept COLORREF Settings::CalculateDefaultBackground() const noexcept
{ {
const auto bg = GetDefaultBackgroundColor(); const auto bg = GetDefaultBackgroundColor();
return bg != INVALID_COLOR ? bg : BackgroundColor(GetFillAttribute(), GetColorTable(), GetColorTableSize()); return bg != INVALID_COLOR ? bg : BackgroundColor(GetFillAttribute(), GetColorTable(), GetLegacyColorTableSize());
} }
// Method Description: // Method Description:

View file

@ -157,6 +157,7 @@ public:
void SetHistoryNoDup(const bool fHistoryNoDup); void SetHistoryNoDup(const bool fHistoryNoDup);
const COLORREF* const GetColorTable() const; const COLORREF* const GetColorTable() const;
const size_t GetLegacyColorTableSize() const;
const size_t GetColorTableSize() const; const size_t GetColorTableSize() const;
void SetColorTable(_In_reads_(cSize) const COLORREF* const pColorTable, const size_t cSize); void SetColorTable(_In_reads_(cSize) const COLORREF* const pColorTable, const size_t cSize);
void SetColorTableEntry(const size_t index, const COLORREF ColorValue); void SetColorTableEntry(const size_t index, const COLORREF ColorValue);

View file

@ -419,7 +419,7 @@ void Telemetry::WriteFinalTraceLog()
TraceLoggingBool(gci.GetQuickEdit(), "QuickEdit"), TraceLoggingBool(gci.GetQuickEdit(), "QuickEdit"),
TraceLoggingValue(gci.GetWindowAlpha(), "WindowAlpha"), TraceLoggingValue(gci.GetWindowAlpha(), "WindowAlpha"),
TraceLoggingBool(gci.GetWrapText(), "WrapText"), 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.CP, "CodePageInput"),
TraceLoggingValue(gci.OutputCP, "CodePageOutput"), TraceLoggingValue(gci.OutputCP, "CodePageOutput"),
TraceLoggingValue(gci.GetFontSize().X, "FontSizeX"), TraceLoggingValue(gci.GetFontSize().X, "FontSizeX"),

View file

@ -337,7 +337,7 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults)
pStateInfo->NumberOfHistoryBuffers = gci.GetNumberOfHistoryBuffers(); pStateInfo->NumberOfHistoryBuffers = gci.GetNumberOfHistoryBuffers();
pStateInfo->HistoryNoDup = !!(gci.Flags & CONSOLE_HISTORY_NODUP); 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. // Create mutable copies of the titles so the propsheet can do something with them.
if (gci.GetOriginalTitle().length() > 0) 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. // Ensure that attributes only contain color specification.
WI_ClearAllFlags(pStateInfo->ScreenAttributes, ~(FG_ATTRS | BG_ATTRS)); WI_ClearAllFlags(pStateInfo->ScreenAttributes, ~(FG_ATTRS | BG_ATTRS));