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

Add support for SGR 58/59 (underline coloring) #15599

Closed
wants to merge 3 commits into from
Closed

Add support for SGR 58/59 (underline coloring) #15599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1e40bc
add support for SGR 58/59 (underline coloring) in `--headless` mode
tusharsnx Jun 25, 2023
99bf720
fix line endings
tusharsnx Jun 25, 2023
4630ac2
run format
tusharsnx Jun 25, 2023
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
35 changes: 34 additions & 1 deletion src/buffer/out/TextAttribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// Keeping TextColor compact helps us keeping TextAttribute compact,
// which in turn ensures that our buffer memory usage is low.
static_assert(sizeof(TextAttribute) == 12);
static_assert(sizeof(TextAttribute) == 16);
static_assert(alignof(TextAttribute) == 2);
// Ensure that we can memcpy() and memmove() the struct for performance.
static_assert(std::is_trivially_copyable_v<TextAttribute>);
Expand Down Expand Up @@ -147,6 +147,11 @@ TextColor TextAttribute::GetBackground() const noexcept
return _background;
}

TextColor TextAttribute::GetUnderlineColor() const noexcept
{
return _underlineColor;
}

// Method description:
// - Retrieves the hyperlink ID of the text
// Return value:
Expand All @@ -166,6 +171,11 @@ void TextAttribute::SetBackground(const TextColor background) noexcept
_background = background;
}

void TextAttribute::SetUnderlineColor(const TextColor color) noexcept
{
_underlineColor = color;
}

void TextAttribute::SetForeground(const COLORREF rgbForeground) noexcept
{
_foreground = TextColor(rgbForeground);
Expand All @@ -176,6 +186,11 @@ void TextAttribute::SetBackground(const COLORREF rgbBackground) noexcept
_background = TextColor(rgbBackground);
}

void TextAttribute::SetUnderlineColor(const COLORREF rgbColor) noexcept
{
_underlineColor = TextColor(rgbColor);
}

void TextAttribute::SetIndexedForeground(const BYTE fgIndex) noexcept
{
_foreground = TextColor(fgIndex, false);
Expand All @@ -186,6 +201,14 @@ void TextAttribute::SetIndexedBackground(const BYTE bgIndex) noexcept
_background = TextColor(bgIndex, false);
}

// Method description:
// - No-op, underlines only support index256 and rgb coloring schemes.
// Arguments:
// - cIndex - index16 based color index.
void TextAttribute::SetIndexedUnderlineColor(const BYTE /*cIndex*/) noexcept
{
}

void TextAttribute::SetIndexedForeground256(const BYTE fgIndex) noexcept
{
_foreground = TextColor(fgIndex, true);
Expand All @@ -196,6 +219,11 @@ void TextAttribute::SetIndexedBackground256(const BYTE bgIndex) noexcept
_background = TextColor(bgIndex, true);
}

void TextAttribute::SetIndexedUnderlineColor256(const BYTE cIndex) noexcept
{
_underlineColor = TextColor(cIndex, true);
}

void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept
{
if (fIsForeground)
Expand Down Expand Up @@ -374,6 +402,11 @@ void TextAttribute::SetDefaultBackground() noexcept
_background = TextColor();
}

void TextAttribute::SetDefaultUnderlineColor() noexcept
{
_underlineColor = TextColor();
}

// Method description:
// - Resets only the rendition character attributes, which includes everything
// except the Protected attribute.
Expand Down
10 changes: 10 additions & 0 deletions src/buffer/out/TextAttribute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class TextAttribute final
_attrs{ CharacterAttributes::Normal },
_foreground{},
_background{},
_underlineColor{},
_hyperlinkId{ 0 }
{
}
Expand All @@ -42,6 +43,7 @@ class TextAttribute final
_attrs{ gsl::narrow_cast<WORD>(wLegacyAttr & USED_META_ATTRS) },
_foreground{ gsl::at(s_legacyForegroundColorMap, wLegacyAttr & FG_ATTRS) },
_background{ gsl::at(s_legacyBackgroundColorMap, (wLegacyAttr & BG_ATTRS) >> 4) },
_underlineColor{},
_hyperlinkId{ 0 }
{
}
Expand All @@ -51,6 +53,7 @@ class TextAttribute final
_attrs{ CharacterAttributes::Normal },
_foreground{ rgbForeground },
_background{ rgbBackground },
_underlineColor{},
_hyperlinkId{ 0 }
{
}
Expand Down Expand Up @@ -117,20 +120,26 @@ class TextAttribute final

TextColor GetForeground() const noexcept;
TextColor GetBackground() const noexcept;
TextColor GetUnderlineColor() const noexcept;
uint16_t GetHyperlinkId() const noexcept;
void SetForeground(const TextColor foreground) noexcept;
void SetBackground(const TextColor background) noexcept;
void SetUnderlineColor(const TextColor color) noexcept;
void SetForeground(const COLORREF rgbForeground) noexcept;
void SetBackground(const COLORREF rgbBackground) noexcept;
void SetUnderlineColor(const COLORREF rgbColor) noexcept;
void SetIndexedForeground(const BYTE fgIndex) noexcept;
void SetIndexedBackground(const BYTE bgIndex) noexcept;
void SetIndexedUnderlineColor(const BYTE /*cIndex*/) noexcept;
void SetIndexedForeground256(const BYTE fgIndex) noexcept;
void SetIndexedBackground256(const BYTE bgIndex) noexcept;
void SetIndexedUnderlineColor256(const BYTE cIndex) noexcept;
void SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept;
void SetHyperlinkId(uint16_t id) noexcept;

void SetDefaultForeground() noexcept;
void SetDefaultBackground() noexcept;
void SetDefaultUnderlineColor() noexcept;
void SetDefaultRenditionAttributes() noexcept;

bool BackgroundIsDefault() const noexcept;
Expand Down Expand Up @@ -175,6 +184,7 @@ class TextAttribute final
uint16_t _hyperlinkId; // sizeof: 2, alignof: 2
TextColor _foreground; // sizeof: 4, alignof: 1
TextColor _background; // sizeof: 4, alignof: 1
TextColor _underlineColor; // sizeof: 4, alignof: 1

#ifdef UNIT_TESTING
friend class TextBufferTests;
Expand Down
69 changes: 69 additions & 0 deletions src/renderer/vt/Xterm256Engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
RETURN_IF_FAILED(_SetUnderlined(true));
_lastTextAttributes.SetUnderlined(true);
}

if (textAttributes.IsDoublyUnderlined() && !_lastTextAttributes.IsDoublyUnderlined())
{
RETURN_IF_FAILED(_SetDoublyUnderlined(true));
Expand Down Expand Up @@ -145,9 +146,77 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
_lastTextAttributes.SetReverseVideo(textAttributes.IsReverseVideo());
}

// Update underline color.
if (textAttributes.GetUnderlineColor() != _lastTextAttributes.GetUnderlineColor())
{
RETURN_IF_FAILED(_RgbUpdateUnderlineDrawingBrushes(textAttributes.GetUnderlineColor()));
_lastTextAttributes.SetUnderlineColor(textAttributes.GetUnderlineColor());
}

return S_OK;
}

// Routine Description:
// - Write a VT sequence to change the current color for underlines. Writes true RGB
// color sequences.
// Arguments:
// - ulColor: Underline color which needs to be applied.
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT Xterm256Engine::_RgbUpdateUnderlineDrawingBrushes(const TextColor ulColor) noexcept
{
if (ulColor.IsDefault())
{
RETURN_IF_FAILED(_UnderlineDefaultColor());
}
// index16 isn't supported.
else if (ulColor.IsIndex16())
{
return S_FALSE;
}
else if (ulColor.IsIndex256())
{
RETURN_IF_FAILED(_Underline256Color(ulColor.GetIndex()));
}
else if (ulColor.IsRgb())
{
RETURN_IF_FAILED(_UnderlineRGBColor(ulColor.GetRGB()));
}
return S_OK;
}

// Method Description:
// - Formats and writes a sequence to change the current underline color to an
// indexed color from the 256-color table.
// Arguments:
// - index: color table index to emit as a VT sequence
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT Xterm256Engine::_Underline256Color(const BYTE index) noexcept
{
return _WriteFormatted(FMT_COMPILE("\x1b[58;5;{}m"), index);
}

// Method Description:
// - Formats and writes a sequence to change the current underline color to an
// RGB color.
// Arguments:
// - color: The color to emit a VT sequence for.
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT Xterm256Engine::_UnderlineRGBColor(const COLORREF color) noexcept
{
const auto r = GetRValue(color);
const auto g = GetGValue(color);
const auto b = GetBValue(color);
return _WriteFormatted(FMT_COMPILE("\x1b[58;2;{};{};{}m"), r, g, b);
Copy link

Choose a reason for hiding this comment

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

Can these use colon-separated subparameters? So as to minimise the risk that applications ignorant of underline colors treat the RGB numbers as separate attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean, may be we can do only : instead, but that would require removing the invalidation of : as a CSI parameter delimiter (conditionally). Thanks for the heads up.

Copy link

Choose a reason for hiding this comment

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

I see, it's blocked by #4321.

}

[[nodiscard]] HRESULT Xterm256Engine::_UnderlineDefaultColor() noexcept
{
return _Write("\x1b[59m");
}

// Routine Description:
// - Write a VT sequence to start/stop a hyperlink
// Arguments:
Expand Down
4 changes: 4 additions & 0 deletions src/renderer/vt/Xterm256Engine.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ namespace Microsoft::Console::Render
friend class ::VtApiRoutines;

private:
[[nodiscard]] HRESULT _Underline256Color(const BYTE index) noexcept;
[[nodiscard]] HRESULT _UnderlineRGBColor(const COLORREF color) noexcept;
[[nodiscard]] HRESULT _UnderlineDefaultColor() noexcept;
[[nodiscard]] HRESULT _RgbUpdateUnderlineDrawingBrushes(const TextColor ulColor) noexcept;
[[nodiscard]] HRESULT _UpdateExtendedAttrs(const TextAttribute& textAttributes) noexcept;
[[nodiscard]] HRESULT _UpdateHyperlinkAttr(const TextAttribute& textAttributes,
const gsl::not_null<IRenderData*> pData) noexcept;
Expand Down
9 changes: 9 additions & 0 deletions src/terminal/adapter/DispatchTypes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
BackgroundDefault = 49,
Overline = 53,
NoOverline = 55,
UnderlineColor = 58,
UnderlineColorDefault = 59,
BrightForegroundBlack = 90,
BrightForegroundRed = 91,
BrightForegroundGreen = 92,
Expand All @@ -366,6 +368,13 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
Unprotected = 2
};

enum class TextProp : VTInt
{
Foreground = 0,
Background = 1,
Underline = 2
};

// Many of these correspond directly to SGR parameters (the GraphicsOptions enum), but
// these are distinct (notably 10 and 11, which as SGR parameters would select fonts,
// are used here to indicate that the foreground/background colors should be saved).
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/adaptDispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ namespace Microsoft::Console::VirtualTerminal

size_t _SetRgbColorsHelper(const VTParameters options,
TextAttribute& attr,
const bool isForeground) noexcept;
const DispatchTypes::TextProp propType) noexcept;
size_t _ApplyGraphicsOption(const VTParameters options,
const size_t optionIndex,
TextAttribute& attr) noexcept;
Expand Down
44 changes: 33 additions & 11 deletions src/terminal/adapter/adaptDispatchGraphics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,21 @@ using namespace Microsoft::Console::VirtualTerminal;
using namespace Microsoft::Console::VirtualTerminal::DispatchTypes;

// Routine Description:
// - Helper to parse extended graphics options, which start with 38 (FG) or 48 (BG)
// - Helper to parse extended graphics options, which start with 38 (FG) or 48 (BG) or 58 (underline)
// These options are followed by either a 2 (RGB) or 5 (xterm index)
// RGB sequences then take 3 MORE params to designate the R, G, B parts of the color
// Xterm index will use the param that follows to use a color from the preset 256 color xterm color table.
// Arguments:
// - options - An array of options that will be used to generate the RGB color
// - attr - The attribute that will be updated with the parsed color.
// - isForeground - Whether or not the parsed color is for the foreground.
// - propType - text property to apply the parsed color on.
// Return Value:
// - The number of options consumed, not including the initial 38/48.
// - The number of options consumed, not including the initial 38/48/58.
size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options,
TextAttribute& attr,
const bool isForeground) noexcept
TextProp propType) noexcept
{
using enum TextProp;
size_t optionsConsumed = 1;
const DispatchTypes::GraphicsOptions typeOpt = options.at(0);
if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint)
Expand All @@ -39,7 +40,18 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options,
if (red <= 255 && green <= 255 && blue <= 255)
{
const auto rgbColor = RGB(red, green, blue);
attr.SetColor(rgbColor, isForeground);
switch (propType)
{
case Foreground:
attr.SetColor(rgbColor, true);
break;
case Background:
attr.SetColor(rgbColor, false);
break;
case Underline:
attr.SetUnderlineColor(rgbColor);
break;
}
}
}
else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index)
Expand All @@ -49,13 +61,17 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options,
if (tableIndex <= 255)
{
const auto adjustedIndex = gsl::narrow_cast<BYTE>(tableIndex);
if (isForeground)
switch (propType)
{
case Foreground:
attr.SetIndexedForeground256(adjustedIndex);
}
else
{
break;
case Background:
attr.SetIndexedBackground256(adjustedIndex);
break;
case Underline:
attr.SetIndexedUnderlineColor256(adjustedIndex);
break;
}
}
}
Expand All @@ -80,6 +96,7 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options,
case Off:
attr.SetDefaultForeground();
attr.SetDefaultBackground();
attr.SetDefaultUnderlineColor();
attr.SetDefaultRenditionAttributes();
return 1;
case ForegroundDefault:
Expand Down Expand Up @@ -242,9 +259,14 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options,
attr.SetIndexedBackground(TextColor::BRIGHT_WHITE);
return 1;
case ForegroundExtended:
return 1 + _SetRgbColorsHelper(options.subspan(optionIndex + 1), attr, true);
return 1 + _SetRgbColorsHelper(options.subspan(optionIndex + 1), attr, TextProp::Foreground);
case BackgroundExtended:
return 1 + _SetRgbColorsHelper(options.subspan(optionIndex + 1), attr, false);
return 1 + _SetRgbColorsHelper(options.subspan(optionIndex + 1), attr, TextProp::Background);
case UnderlineColor:
return 1 + _SetRgbColorsHelper(options.subspan(optionIndex + 1), attr, TextProp::Underline);
case UnderlineColorDefault:
attr.SetDefaultUnderlineColor();
return 1;
default:
return 1;
}
Expand Down