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.

Sign up for GitHub

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

Consolidate the color palette APIs #11784

Merged
8 commits merged into from
Nov 23, 2021
Merged

Consolidate the color palette APIs #11784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
79859cf
Merge default colors into color table.
j4james Nov 7, 2021
05e20f6
Eliminate default color APIs in ConGetSet and ITerminalApi.
j4james Nov 7, 2021
a8bad33
Standardize color table APIs in ConGetSet and ITerminalApi.
j4james Nov 7, 2021
96dd85f
Use index values when referring to default colors.
j4james Nov 7, 2021
6ebcb5e
Cache the calculation of default color indices.
j4james Nov 13, 2021
7004ba7
Merge cursor color into color table.
j4james Nov 14, 2021
a1e98f5
Simplify registry queries for special colors.
j4james Nov 14, 2021
69f2b4f
Silence the spelling bot.
j4james Nov 18, 2021
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
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/apis.txt
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's by design that TextColor can still only refer to colors 0-255 as indexed colors, right? We're not going to expand the definition of a TextColor to store default-ness as indices 256, 257?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for now at least. I did think their might be some advantage to storing the default colors with their index already calculated (in which case we would need to increase the index range), but I'm not sure that would work correctly when the indexes are changed. I might reconsider this in the future though.


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
View file
Open in desktop
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
View file
Open in desktop
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