Skip to content

Commit

Permalink
Reintroduce a color compatibility hack, but only for the BG color
Browse files Browse the repository at this point in the history
There is going to be a very long tail of applications that will
explicitly request VT SGR 40 when what they really want is to
SetConsoleTextAttribute() with a black background. Instead of making
those applications look bad (and therefore making us look bad, because
we're releasing this as an update to something that "looks good"
already), we're introducing this compatibility hack. Before the color
reckoning in #6698 + #6506, *every* color was subject to being
spontaneously and erroneously turned into the default color. Now, only
the 16-color palette value that matches the active console background
color will be destroyed.  This is not intended to be a long-term
solution. This comment will be discovered in forty years(*) time and
people will laugh at my hubris.

Removal, or final remediation, will be tracked by #6807.

*it doesn't matter when you're reading this, it will always be 40 years
from now.
  • Loading branch information
DHowett committed Jul 7, 2020
1 parent d350a89 commit 2d48d29
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ void TextAttribute::SetLegacyDefaultAttributes(const WORD defaultAttributes) noe
s_legacyDefaultBackground = (defaultAttributes & BG_ATTRS) >> 4;
}

// Routine Description:
// - Returns whether the color passed in is an analogue of the default background color
// represented as a 16-color palette index.
// - Do not use this function.
bool TextAttribute::IsColorVT16VersionOfLegacyDefaultBackground(const TextColor& color) noexcept {
return color.IsIndex16() && color.GetIndex() == s_legacyDefaultBackground;
}

// Routine Description:
// - Returns a WORD with legacy-style attributes for this textattribute.
// Parameters:
Expand Down
1 change: 1 addition & 0 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class TextAttribute final
}

static void SetLegacyDefaultAttributes(const WORD defaultAttributes) noexcept;
static bool IsColorVT16VersionOfLegacyDefaultBackground(const TextColor& textColor) noexcept;
WORD GetLegacyAttributes() const noexcept;

COLORREF CalculateRgbForeground(std::basic_string_view<COLORREF> colorTable,
Expand Down
34 changes: 32 additions & 2 deletions src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,34 @@ using namespace Microsoft::Console::Types;
[[nodiscard]] HRESULT VtEngine::_RgbUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept
{
const auto fg = textAttributes.GetForeground();
const auto bg = textAttributes.GetBackground();
auto bg = textAttributes.GetBackground();
auto lastFg = _lastTextAttributes.GetForeground();
auto lastBg = _lastTextAttributes.GetBackground();

// GH#6807
// Replace a background color that explicitly matches the console host's
// active background with the "default" background.
//
// There is going to be a very long tail of applications that will
// explicitly request VT SGR 40 when what they really want is to
// SetConsoleTextAttribute() with a black background. Instead of making
// those applications look bad (and therefore making us look bad, because
// we're releasing this as an update to something that "looks good"
// already), we're introducing this compatibility hack. Before the color
// reckoning in GH#6698 + GH#6506, *every* color was subject to being
// spontaneously and erroneously turned into the default color. Now, only
// the 16-color palette value that matches the active console background
// color will be destroyed.
// This is not intended to be a long-term solution. This comment will be
// discovered in forty years(*) time and people will laugh at our hubris.
//
// *it doesn't matter when you're reading this, it will always be 40 years
// from now.
if (TextAttribute::IsColorVT16VersionOfLegacyDefaultBackground(bg))
{
bg = {};
}

// If both the FG and BG should be the defaults, emit a SGR reset.
if (fg.IsDefault() && bg.IsDefault() && !(lastFg.IsDefault() && lastBg.IsDefault()))
{
Expand Down Expand Up @@ -261,10 +285,16 @@ using namespace Microsoft::Console::Types;
[[nodiscard]] HRESULT VtEngine::_16ColorUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept
{
const auto fg = textAttributes.GetForeground();
const auto bg = textAttributes.GetBackground();
auto bg = textAttributes.GetBackground();
auto lastFg = _lastTextAttributes.GetForeground();
auto lastBg = _lastTextAttributes.GetBackground();

// As above in _RgbUpdateDrawingBrushes; see GH#6807 for details.
if (TextAttribute::IsColorVT16VersionOfLegacyDefaultBackground(bg))
{
bg = {};
}

// If either FG or BG have changed to default, emit a SGR reset.
// We can't reset FG and BG to default individually.
if ((fg.IsDefault() && !lastFg.IsDefault()) || (bg.IsDefault() && !lastBg.IsDefault()))
Expand Down

0 comments on commit 2d48d29

Please sign in to comment.