Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the propagation of color attributes over ConPTY #6506

Merged
merged 15 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ WORD TextAttribute::GetLegacyAttributes(const WORD defaultAttributes) const noex
const BYTE fgIndex = _foreground.GetLegacyIndex(defaultAttributes & FG_ATTRS);
const BYTE bgIndex = _background.GetLegacyIndex((defaultAttributes & BG_ATTRS) >> 4);
const WORD metaAttrs = _wAttrLegacy & META_ATTRS;
const bool brighten = _foreground.IsIndex16() && IsBold();
const bool brighten = IsBold() && _foreground.CanBeBrightened();
return fgIndex | (bgIndex << 4) | metaAttrs | (brighten ? FOREGROUND_INTENSITY : 0);
}

Expand Down Expand Up @@ -75,6 +75,26 @@ COLORREF TextAttribute::_GetRgbBackground(std::basic_string_view<COLORREF> color
return _background.GetColor(colorTable, defaultColor, false);
}

TextColor TextAttribute::GetForeground() const noexcept
{
return _foreground;
}

TextColor TextAttribute::GetBackground() const noexcept
{
return _background;
}

void TextAttribute::SetForeground(const TextColor foreground) noexcept
{
_foreground = foreground;
}

void TextAttribute::SetBackground(const TextColor background) noexcept
{
_background = background;
}

void TextAttribute::SetForeground(const COLORREF rgbForeground) noexcept
{
_foreground = TextColor(rgbForeground);
Expand Down
4 changes: 4 additions & 0 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class TextAttribute final

ExtendedAttributes GetExtendedAttributes() const noexcept;

TextColor GetForeground() const noexcept;
TextColor GetBackground() const noexcept;
void SetForeground(const TextColor foreground) noexcept;
void SetBackground(const TextColor background) noexcept;
void SetForeground(const COLORREF rgbForeground) noexcept;
void SetBackground(const COLORREF rgbBackground) noexcept;
void SetIndexedForeground(const BYTE fgIndex) noexcept;
Expand Down
9 changes: 7 additions & 2 deletions src/buffer/out/TextColor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ constexpr std::array<BYTE, 256> Index256ToIndex16 = {

// clang-format on

bool TextColor::CanBeBrightened() const noexcept
{
return IsIndex16() || IsDefault();
}

bool TextColor::IsLegacy() const noexcept
{
return IsIndex16() || (IsIndex256() && _index < 16);
Expand Down Expand Up @@ -164,7 +169,7 @@ COLORREF TextColor::GetColor(std::basic_string_view<COLORREF> colorTable,
}
else if (IsRgb())
{
return _GetRGB();
return GetRGB();
}
else if (IsIndex16() && brighten)
{
Expand Down Expand Up @@ -214,7 +219,7 @@ BYTE TextColor::GetLegacyIndex(const BYTE defaultIndex) const noexcept
// - <none>
// Return Value:
// - a COLORREF containing our stored value
COLORREF TextColor::_GetRGB() const noexcept
COLORREF TextColor::GetRGB() const noexcept
{
return RGB(_red, _green, _blue);
}
7 changes: 4 additions & 3 deletions src/buffer/out/TextColor.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ struct TextColor
friend constexpr bool operator==(const TextColor& a, const TextColor& b) noexcept;
friend constexpr bool operator!=(const TextColor& a, const TextColor& b) noexcept;

bool CanBeBrightened() const noexcept;
bool IsLegacy() const noexcept;
bool IsIndex16() const noexcept;
bool IsIndex256() const noexcept;
Expand All @@ -98,6 +99,8 @@ struct TextColor
return _index;
}

COLORREF GetRGB() const noexcept;

private:
ColorType _meta : 2;
union
Expand All @@ -107,8 +110,6 @@ struct TextColor
BYTE _green;
BYTE _blue;

COLORREF _GetRGB() const noexcept;

#ifdef UNIT_TESTING
friend class TextBufferTests;
template<typename TextColor>
Expand Down Expand Up @@ -149,7 +150,7 @@ namespace WEX
}
else if (color.IsRgb())
{
return WEX::Common::NoThrowString().Format(L"{RGB:0x%06x}", color._GetRGB());
return WEX::Common::NoThrowString().Format(L"{RGB:0x%06x}", color.GetRGB());
}
else
{
Expand Down
4 changes: 1 addition & 3 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
Viewport initialViewport = currentBuffer.GetViewport();

auto vtRenderEngine = std::make_unique<Xterm256Engine>(std::move(hFile),
gci,
initialViewport,
gci.Get16ColorTable());
initialViewport);
auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2);
vtRenderEngine->SetTestCallback(pfn);

Expand Down
8 changes: 1 addition & 7 deletions src/host/VtIo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,16 @@ VtIo::VtIo() :
{
case VtIoMode::XTERM_256:
_pVtRenderEngine = std::make_unique<Xterm256Engine>(std::move(_hOutput),
gci,
initialViewport,
gci.Get16ColorTable());
initialViewport);
break;
case VtIoMode::XTERM:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.Get16ColorTable(),
false);
break;
case VtIoMode::XTERM_ASCII:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.Get16ColorTable(),
true);
break;
default:
Expand Down
117 changes: 0 additions & 117 deletions src/host/conattrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,100 +17,6 @@ Author(s):
#include "..\inc\conattrs.hpp"
#include <math.h>

struct _HSL
{
double h, s, l;

// constructs an HSL color from a RGB Color.
explicit _HSL(const COLORREF rgb)
{
const double r = (double)GetRValue(rgb);
const double g = (double)GetGValue(rgb);
const double b = (double)GetBValue(rgb);

const auto [min, max] = std::minmax({ r, g, b });

const auto diff = max - min;
const auto sum = max + min;
// Luminance
l = max / 255.0;

// Saturation
s = (max == 0) ? 0 : diff / max;

//Hue
double q = (diff == 0) ? 0 : 60.0 / diff;
if (max == r)
{
h = (g < b) ? ((360.0 + q * (g - b)) / 360.0) : ((q * (g - b)) / 360.0);
}
else if (max == g)
{
h = (120.0 + q * (b - r)) / 360.0;
}
else if (max == b)
{
h = (240.0 + q * (r - g)) / 360.0;
}
else
{
h = 0;
}
}
};

//Routine Description:
// Finds the "distance" between a given HSL color and an RGB color, using the HSL color space.
// This function is designed such that the caller would convert one RGB color to HSL ahead of time,
// then compare many RGB colors to that first color.
//Arguments:
// - phslColorA - a pointer to the first color, as a HSL color.
// - rgbColorB - The second color to compare, in RGB color.
// Return value:
// The "distance" between the two.
static double _FindDifference(const _HSL* const phslColorA, const COLORREF rgbColorB)
{
const _HSL hslColorB = _HSL(rgbColorB);
return sqrt(pow((hslColorB.h - phslColorA->h), 2) +
pow((hslColorB.s - phslColorA->s), 2) +
pow((hslColorB.l - phslColorA->l), 2));
}

//Routine Description:
// For a given RGB color Color, finds the nearest color from the array ColorTable, and returns the index of that match.
//Arguments:
// - Color - The RGB color to fine the nearest color to.
// - ColorTable - The array of colors to find a nearest color from.
// Return value:
// The index in ColorTable of the nearest match to Color.
WORD FindNearestTableIndex(const COLORREF Color, const std::basic_string_view<COLORREF> ColorTable)
{
// Quick check for an exact match in the color table:
for (WORD i = 0; i < ColorTable.size(); i++)
{
if (Color == ColorTable[i])
{
return i;
}
}

// Did not find an exact match - do an expensive comparison to the elements
// of the table to find the nearest color.
const _HSL hslColor = _HSL(Color);
WORD closest = 0;
double minDiff = _FindDifference(&hslColor, ColorTable[0]);
for (WORD i = 1; i < ColorTable.size(); i++)
{
double diff = _FindDifference(&hslColor, ColorTable[i]);
if (diff < minDiff)
{
minDiff = diff;
closest = i;
}
}
return closest;
}

// Function Description:
// - Converts the value of a xterm color table index to the windows color table equivalent.
// Arguments:
Expand Down Expand Up @@ -144,26 +50,3 @@ WORD Xterm256ToWindowsIndex(const size_t xtermTableEntry) noexcept
return xtermTableEntry < 16 ? XtermToWindowsIndex(xtermTableEntry) :
static_cast<WORD>(xtermTableEntry);
}

//Routine Description:
// Returns the exact entry from the color table, if it's in there.
//Arguments:
// - Color - The RGB color to fine the nearest color to.
// - ColorTable - The array of colors to find a nearest color from.
// Return value:
// The index in ColorTable of the nearest match to Color.
bool FindTableIndex(const COLORREF Color,
const std::basic_string_view<COLORREF> ColorTable,
_Out_ WORD* const pFoundIndex)
{
*pFoundIndex = 0;
for (WORD i = 0; i < ColorTable.size(); i++)
{
if (ColorTable[i] == Color)
{
*pFoundIndex = i;
return true;
}
}
return false;
}
5 changes: 1 addition & 4 deletions src/host/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ Revision History:

#include "..\host\RenderData.hpp"

#include "..\inc\IDefaultColorProvider.hpp"

// clang-format off
// Flags flags
#define CONSOLE_IS_ICONIC 0x00000001
Expand Down Expand Up @@ -75,8 +73,7 @@ class CommandHistory;

class CONSOLE_INFORMATION :
public Settings,
public Microsoft::Console::IIoProvider,
public Microsoft::Console::IDefaultColorProvider
public Microsoft::Console::IIoProvider
{
public:
CONSOLE_INFORMATION();
Expand Down
4 changes: 1 addition & 3 deletions src/host/ut_host/ConptyOutputTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ class ConptyOutputTests
Viewport initialViewport = currentBuffer.GetViewport();

auto vtRenderEngine = std::make_unique<Xterm256Engine>(std::move(hFile),
gci,
initialViewport,
gci.Get16ColorTable());
initialViewport);
auto pfn = std::bind(&ConptyOutputTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2);
vtRenderEngine->SetTestCallback(pfn);

Expand Down
Loading