Skip to content

Commit

Permalink
Use the entire 256 table to lookup the color
Browse files Browse the repository at this point in the history
  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.
  • Loading branch information
zadjii-msft committed Aug 27, 2019
1 parent 92830ab commit 608c660
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 19 deletions.
8 changes: 4 additions & 4 deletions src/host/VtIo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,30 +161,30 @@ VtIo::VtIo() :
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()));
static_cast<WORD>(gci.GetLegacyColorTableSize()));
break;
case VtIoMode::XTERM:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()),
static_cast<WORD>(gci.GetLegacyColorTableSize()),
false);
break;
case VtIoMode::XTERM_ASCII:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()),
static_cast<WORD>(gci.GetLegacyColorTableSize()),
true);
break;
case VtIoMode::WIN_TELNET:
_pVtRenderEngine = std::make_unique<WinTelnetEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()));
static_cast<WORD>(gci.GetLegacyColorTableSize()));
break;
default:
return E_FAIL;
Expand Down
32 changes: 23 additions & 9 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
// }
// else
// {
// rgbColor = gci.GetColorTableEntry(iXtermTableEntry);
// }

rgbColor = gci.GetColorTableEntry(iWinEntry);
// 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
{
rgbColor = gci.GetColorTableEntry(iXtermTableEntry);
newBG = static_cast<BYTE>(iXtermTableEntry);
}

NewAttributes.SetColor(rgbColor, fIsForeground);
NewAttributes.SetIndexedAttributes(newFG, newBG);

buffer.SetAttributes(NewAttributes);
}
Expand Down
12 changes: 9 additions & 3 deletions src/host/settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/host/settings.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class Settings
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);
Expand Down
2 changes: 1 addition & 1 deletion src/host/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
4 changes: 2 additions & 2 deletions src/interactivity/win32/menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 608c660

Please sign in to comment.