Skip to content

Commit

Permalink
Consolidate the color palette APIs (#11784)
Browse files Browse the repository at this point in the history
This PR merges the default colors and cursor color into the main color
table, enabling us to simplify the `ConGetSet` and `ITerminalApi`
interfaces, with just two methods required for getting and setting any
form of color palette entry.

The is a follow-up to the color table standardization in #11602, and a
another small step towards de-duplicating `AdaptDispatch` and
`TerminalDispatch` for issue #3849. It should also make it easier to
support color queries (#3718) and a configurable bold color (#5682) in
the future.

On the conhost side, default colors could originally be either indexed
positions in the 16-color table, or separate standalone RGB values. With
the new system, the default colors will always be in the color table, so
we just need to track their index positions.

To make this work, those positions need to be calculated at startup
based on the loaded registry/shortcut settings, and updated when
settings are changed (this is handled in
`CalculateDefaultColorIndices`). But the plus side is that it's now much
easier to lookup the default color values for rendering.

For now the default colors in Windows Terminal use hardcoded positions,
because it doesn't need indexed default colors like conhost. But in the
future I'd like to extend the index handling to both terminals, so we
can eventually support the VT525 indexed color operations.

As for the cursor color, that was previously stored in the `Cursor`
class, which meant that it needed to be copied around in various places
where cursors were being instantiated. Now that it's managed separately
in the color table, a lot of that code is no longer required.

## Validation
Some of the unit test initialization code needed to be updated to setup
the color table and default index values as required for the new system.
There were also some adjustments needed to account for API changes, in
particular for methods that now take index values for the default colors
in place of COLORREFs. But for the most part, the essential behavior of
the tests remains unchanged.

I've also run a variety of manual tests looking at the legacy console
APIs as well as the various VT color sequences, and checking that
everything works as expected when color schemes are changed, both in
Windows Terminal and conhost, and in the latter case with both indexed
colors and RGB values.

Closes #11768
  • Loading branch information
j4james committed Nov 23, 2021
1 parent df06c54 commit bb71179
Show file tree
Hide file tree
Showing 43 changed files with 351 additions and 647 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ toupper
TTask
TVal
UChar
UFIELD
ULARGE
UPDATEINIFILE
userenv
Expand Down
14 changes: 7 additions & 7 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,22 @@ bool TextAttribute::IsLegacy() const noexcept
// - Calculates rgb colors based off of current color table and active modification attributes.
// Arguments:
// - colorTable: the current color table rgb values.
// - defaultFgColor: the default foreground color rgb value.
// - defaultBgColor: the default background color rgb value.
// - defaultFgIndex: the color table index of the default foreground color.
// - defaultBgIndex: the color table index of the default background color.
// - reverseScreenMode: true if the screen mode is reversed.
// - blinkingIsFaint: true if blinking should be interpreted as faint. (defaults to false)
// - boldIsBright: true if "bold" should be interpreted as bright. (defaults to true)
// Return Value:
// - the foreground and background colors that should be displayed.
std::pair<COLORREF, COLORREF> TextAttribute::CalculateRgbColors(const std::array<COLORREF, 256>& colorTable,
const COLORREF defaultFgColor,
const COLORREF defaultBgColor,
std::pair<COLORREF, COLORREF> TextAttribute::CalculateRgbColors(const std::array<COLORREF, TextColor::TABLE_SIZE>& colorTable,
const size_t defaultFgIndex,
const size_t defaultBgIndex,
const bool reverseScreenMode,
const bool blinkingIsFaint,
const bool boldIsBright) const noexcept
{
auto fg = _foreground.GetColor(colorTable, defaultFgColor, boldIsBright && IsBold());
auto bg = _background.GetColor(colorTable, defaultBgColor);
auto fg = _foreground.GetColor(colorTable, defaultFgIndex, boldIsBright && IsBold());
auto bg = _background.GetColor(colorTable, defaultBgIndex);
if (IsFaint() || (IsBlinking() && blinkingIsFaint))
{
fg = (fg >> 1) & 0x7F7F7F; // Divide foreground color components by two.
Expand Down
6 changes: 3 additions & 3 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ class TextAttribute final
static TextAttribute StripErroneousVT16VersionsOfLegacyDefaults(const TextAttribute& attribute) noexcept;
WORD GetLegacyAttributes() const noexcept;

std::pair<COLORREF, COLORREF> CalculateRgbColors(const std::array<COLORREF, 256>& colorTable,
const COLORREF defaultFgColor,
const COLORREF defaultBgColor,
std::pair<COLORREF, COLORREF> CalculateRgbColors(const std::array<COLORREF, TextColor::TABLE_SIZE>& colorTable,
const size_t defaultFgIndex,
const size_t defaultBgIndex,
const bool reverseScreenMode = false,
const bool blinkingIsFaint = false,
const bool boldIsBright = true) const noexcept;
Expand Down
6 changes: 4 additions & 2 deletions src/buffer/out/TextColor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,16 @@ void TextColor::SetDefault() noexcept
// Arguments:
// - colorTable: The table of colors we should use to look up the value of
// an indexed attribute from.
// - defaultColor: The color value to use if we're a default attribute.
// - defaultIndex: The color table index to use if we're a default attribute.
// - brighten: if true, we'll brighten a dark color table index.
// Return Value:
// - a COLORREF containing the real value of this TextColor.
COLORREF TextColor::GetColor(const std::array<COLORREF, 256>& colorTable, const COLORREF defaultColor, bool brighten) const noexcept
COLORREF TextColor::GetColor(const std::array<COLORREF, TextColor::TABLE_SIZE>& colorTable, const size_t defaultIndex, bool brighten) const noexcept
{
if (IsDefault())
{
const auto defaultColor = til::at(colorTable, defaultIndex);

if (brighten)
{
// See MSFT:20266024 for context on this fix.
Expand Down
7 changes: 6 additions & 1 deletion src/buffer/out/TextColor.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ struct TextColor
static constexpr BYTE BRIGHT_CYAN = 14;
static constexpr BYTE BRIGHT_WHITE = 15;

static constexpr size_t DEFAULT_FOREGROUND = 256;
static constexpr size_t DEFAULT_BACKGROUND = 257;
static constexpr size_t CURSOR_COLOR = 258;
static constexpr size_t TABLE_SIZE = 259;

constexpr TextColor() noexcept :
_meta{ ColorType::IsDefault },
_red{ 0 },
Expand Down Expand Up @@ -103,7 +108,7 @@ struct TextColor
void SetIndex(const BYTE index, const bool isIndex256) noexcept;
void SetDefault() noexcept;

COLORREF GetColor(const std::array<COLORREF, 256>& colorTable, const COLORREF defaultColor, bool brighten = false) const noexcept;
COLORREF GetColor(const std::array<COLORREF, TABLE_SIZE>& colorTable, const size_t defaultIndex, bool brighten = false) const noexcept;
BYTE GetLegacyIndex(const BYTE defaultIndex) const noexcept;

constexpr BYTE GetIndex() const noexcept
Expand Down
23 changes: 2 additions & 21 deletions src/buffer/out/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ Cursor::Cursor(const ULONG ulSize, TextBuffer& parentBuffer) noexcept :
_fDeferCursorRedraw(false),
_fHaveDeferredCursorRedraw(false),
_ulSize(ulSize),
_cursorType(CursorType::Legacy),
_fUseColor(false),
_color(s_InvertCursorColor)
_cursorType(CursorType::Legacy)
{
}

Expand Down Expand Up @@ -143,10 +141,9 @@ void Cursor::SetSize(const ULONG ulSize) noexcept
_RedrawCursor();
}

void Cursor::SetStyle(const ULONG ulSize, const COLORREF color, const CursorType type) noexcept
void Cursor::SetStyle(const ULONG ulSize, const CursorType type) noexcept
{
_ulSize = ulSize;
_color = color;
_cursorType = type;

_RedrawCursor();
Expand Down Expand Up @@ -285,7 +282,6 @@ void Cursor::CopyProperties(const Cursor& OtherCursor) noexcept
// Size will be handled separately in the resize operation.
//_ulSize = OtherCursor._ulSize;
_cursorType = OtherCursor._cursorType;
_color = OtherCursor._color;
}

void Cursor::DelayEOLWrap(const COORD coordDelayedAt) noexcept
Expand Down Expand Up @@ -335,21 +331,6 @@ const CursorType Cursor::GetType() const noexcept
return _cursorType;
}

const bool Cursor::IsUsingColor() const noexcept
{
return GetColor() != INVALID_COLOR;
}

const COLORREF Cursor::GetColor() const noexcept
{
return _color;
}

void Cursor::SetColor(const unsigned int color) noexcept
{
_color = gsl::narrow_cast<COLORREF>(color);
}

void Cursor::SetType(const CursorType type) noexcept
{
_cursorType = type;
Expand Down
8 changes: 1 addition & 7 deletions src/buffer/out/cursor.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class TextBuffer;
class Cursor final
{
public:
static const unsigned int s_InvertCursorColor = INVALID_COLOR;
// the following values are used to create the textmode cursor.
static constexpr unsigned int CURSOR_SMALL_SIZE = 25; // large enough to be one pixel on a six pixel font

Expand All @@ -51,8 +50,6 @@ class Cursor final
COORD GetPosition() const noexcept;

const CursorType GetType() const noexcept;
const bool IsUsingColor() const noexcept;
const COLORREF GetColor() const noexcept;

void StartDeferDrawing() noexcept;
bool IsDeferDrawing() noexcept;
Expand All @@ -67,7 +64,7 @@ class Cursor final
void SetIsPopupShown(const bool fIsPopupShown) noexcept;
void SetDelay(const bool fDelay) noexcept;
void SetSize(const ULONG ulSize) noexcept;
void SetStyle(const ULONG ulSize, const COLORREF color, const CursorType type) noexcept;
void SetStyle(const ULONG ulSize, const CursorType type) noexcept;

void SetPosition(const COORD cPosition) noexcept;
void SetXPosition(const int NewX) noexcept;
Expand All @@ -84,7 +81,6 @@ class Cursor final
COORD GetDelayedAtPosition() const noexcept;
bool IsDelayedEOLWrap() const noexcept;

void SetColor(const unsigned int color) noexcept;
void SetType(const CursorType type) noexcept;

private:
Expand Down Expand Up @@ -117,6 +113,4 @@ class Cursor final
void _RedrawCursorAlways() noexcept;

CursorType _cursorType;
bool _fUseColor;
COLORREF _color;
};
Loading

0 comments on commit bb71179

Please sign in to comment.