Skip to content

Commit

Permalink
Add support for passing through extended text attributes, like… (#2917)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Adds support for Italics, Blinking, Invisible, CrossedOut text, THROUGH CONPTY. This does **NOT** add support for those styles to conhost or the terminal.

We will store these "Extended Text Attributes" in a `TextAttribute`. When we go to render a line, we'll see if the state has changed from our previous state, and if so, we'll appropriately toggle that state with VT. Boldness has been moved from a `bool` to a single bit in these flags.

Technically, now that these are stored in the buffer, we only need to make changes to the renderers to be able to support them. That's not being done as a part of this PR however.

## References
See also #2915 and #2916, which are some follow-up tasks from this fix. I thought them too risky for 20H1.

## PR Checklist
* [x] Closes #2554
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

<hr>

* store text with extended attributes too

* Plumb attributes through all the renderers

* parse extended attrs, though we're not renderering them right

* Render these states correctly

* Add a very extensive test

* Cleanup for PR

* a block of PR feedback

* add 512 test cases

* Fix the build

* Fix @carlos-zamora's suggestions

* @miniksa's PR feedback

(cherry picked from commit dec5c11)
  • Loading branch information
zadjii-msft authored and DHowett committed Oct 17, 2019
1 parent 8b31460 commit e7f8d31
Show file tree
Hide file tree
Showing 35 changed files with 937 additions and 111 deletions.
14 changes: 7 additions & 7 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view<COLORREF>
COLORREF TextAttribute::_GetRgbForeground(std::basic_string_view<COLORREF> colorTable,
COLORREF defaultColor) const noexcept
{
return _foreground.GetColor(colorTable, defaultColor, _isBold);
return _foreground.GetColor(colorTable, defaultColor, IsBold());
}

// Routine Description:
Expand Down Expand Up @@ -155,11 +155,6 @@ void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground)
}
}

bool TextAttribute::IsBold() const noexcept
{
return _isBold;
}

bool TextAttribute::_IsReverseVideo() const noexcept
{
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
Expand Down Expand Up @@ -215,6 +210,11 @@ void TextAttribute::Debolden() noexcept
_SetBoldness(false);
}

void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept
{
_extendedAttrs = attrs;
}

// Routine Description:
// - swaps foreground and background color
void TextAttribute::Invert() noexcept
Expand All @@ -224,7 +224,7 @@ void TextAttribute::Invert() noexcept

void TextAttribute::_SetBoldness(const bool isBold) noexcept
{
_isBold = isBold;
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold);
}

void TextAttribute::SetDefaultForeground() noexcept
Expand Down
36 changes: 28 additions & 8 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,24 @@ Revision History:
#include "WexTestClass.h"
#endif

#pragma pack(push, 1)

class TextAttribute final
{
public:
constexpr TextAttribute() noexcept :
_wAttrLegacy{ 0 },
_foreground{},
_background{},
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
}

constexpr TextAttribute(const WORD wLegacyAttr) noexcept :
_wAttrLegacy{ gsl::narrow_cast<WORD>(wLegacyAttr & META_ATTRS) },
_foreground{ gsl::narrow_cast<BYTE>(wLegacyAttr & FG_ATTRS) },
_background{ gsl::narrow_cast<BYTE>((wLegacyAttr & BG_ATTRS) >> 4) },
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
// If we're given lead/trailing byte information with the legacy color, strip it.
WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS);
Expand All @@ -53,7 +55,7 @@ class TextAttribute final
_wAttrLegacy{ 0 },
_foreground{ rgbForeground },
_background{ rgbBackground },
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
}

Expand All @@ -62,7 +64,7 @@ class TextAttribute final
const BYTE fg = (_foreground.GetIndex() & FG_ATTRS);
const BYTE bg = (_background.GetIndex() << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
}

// Method Description:
Expand All @@ -85,7 +87,7 @@ class TextAttribute final
const BYTE fg = (fgIndex & FG_ATTRS);
const BYTE bg = (bgIndex << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
}

COLORREF CalculateRgbForeground(std::basic_string_view<COLORREF> colorTable,
Expand Down Expand Up @@ -131,7 +133,18 @@ class TextAttribute final
friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept;

bool IsLegacy() const noexcept;
bool IsBold() const noexcept;

constexpr bool IsBold() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold);
}

constexpr ExtendedAttributes GetExtendedAttributes() const noexcept
{
return _extendedAttrs;
}

void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept;

void SetForeground(const COLORREF rgbForeground) noexcept;
void SetBackground(const COLORREF rgbBackground) noexcept;
Expand Down Expand Up @@ -159,7 +172,7 @@ class TextAttribute final
WORD _wAttrLegacy;
TextColor _foreground;
TextColor _background;
bool _isBold;
ExtendedAttributes _extendedAttrs;

#ifdef UNIT_TESTING
friend class TextBufferTests;
Expand All @@ -169,6 +182,13 @@ class TextAttribute final
#endif
};

#pragma pack(pop)
// 2 for _wAttrLegacy
// 4 for _foreground
// 4 for _background
// 1 for _extendedAttrs
static_assert(sizeof(TextAttribute) <= 11 * sizeof(BYTE), "We should only need 11B for an entire TextColor. Any more than that is just waste");

enum class TextAttributeBehavior
{
Stored, // use contained text attribute
Expand All @@ -181,7 +201,7 @@ constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexce
return a._wAttrLegacy == b._wAttrLegacy &&
a._foreground == b._foreground &&
a._background == b._background &&
a._isBold == b._isBold;
a._extendedAttrs == b._extendedAttrs;
}

constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy
isForeground = false;
}

if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5)
if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5)
{
*pcOptionsConsumed = 5;
// ensure that each value fits in a byte
Expand All @@ -115,7 +115,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy

fSuccess = _terminalApi.SetTextRgbColor(color, isForeground);
}
else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3)
else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3)
{
*pcOptionsConsumed = 3;
if (rgOptions[2] <= 255) // ensure that the provided index is on the table
Expand Down
33 changes: 33 additions & 0 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,39 @@ void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded)
buffer.SetAttributes(attrs);
}

// Method Description:
// - Retrieves the active ExtendedAttributes (italic, underline, etc.) of the
// given screen buffer. Text written to this buffer will be written with these
// attributes.
// Arguments:
// - screenInfo: The buffer to get the extended attrs from.
// Return Value:
// - the currently active ExtendedAttributes.
ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo)
{
auto& buffer = screenInfo.GetActiveBuffer();
auto attrs = buffer.GetAttributes();
return attrs.GetExtendedAttributes();
}

// Method Description:
// - Sets the active ExtendedAttributes (italic, underline, etc.) of the given
// screen buffer. Text written to this buffer will be written with these
// attributes.
// Arguments:
// - screenInfo: The buffer to set the extended attrs for.
// - extendedAttrs: The new ExtendedAttributes to use
// Return Value:
// - <none>
void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo,
const ExtendedAttributes extendedAttrs)
{
auto& buffer = screenInfo.GetActiveBuffer();
auto attrs = buffer.GetAttributes();
attrs.SetExtendedAttributes(extendedAttrs);
buffer.SetAttributes(attrs);
}

// Routine Description:
// - Sets the codepage used for translating text when calling A versions of functions affecting the output buffer.
// Arguments:
Expand Down
3 changes: 3 additions & 0 deletions src/host/getset.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ void DoSrvPrivateSetConsoleRGBTextAttribute(SCREEN_INFORMATION& screenInfo,

void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded);

ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo);
void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, const ExtendedAttributes attrs);

[[nodiscard]] NTSTATUS DoSrvPrivateEraseAll(SCREEN_INFORMATION& screenInfo);

void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo,
Expand Down
26 changes: 26 additions & 0 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,32 @@ BOOL ConhostInternalGetSet::PrivateBoldText(const bool bolded)
return TRUE;
}

// Method Description:
// - Retrieves the currently active ExtendedAttributes. See also
// DoSrvPrivateGetExtendedTextAttributes
// Arguments:
// - pAttrs: Recieves the ExtendedAttributes value.
// Return Value:
// - TRUE if successful (see DoSrvPrivateGetExtendedTextAttributes). FALSE otherwise.
BOOL ConhostInternalGetSet::PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs)
{
*pAttrs = DoSrvPrivateGetExtendedTextAttributes(_io.GetActiveOutputBuffer());
return TRUE;
}

// Method Description:
// - Sets the active ExtendedAttributes of the active screen buffer. Text
// written to this buffer will be written with these attributes.
// Arguments:
// - extendedAttrs: The new ExtendedAttributes to use
// Return Value:
// - TRUE if successful (see DoSrvPrivateSetExtendedTextAttributes). FALSE otherwise.
BOOL ConhostInternalGetSet::PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs)
{
DoSrvPrivateSetExtendedTextAttributes(_io.GetActiveOutputBuffer(), attrs);
return TRUE;
}

// Routine Description:
// - Connects the WriteConsoleInput API call directly into our Driver Message servicing call inside Conhost.exe
// Arguments:
Expand Down
2 changes: 2 additions & 0 deletions src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
const bool fIsForeground) override;

BOOL PrivateBoldText(const bool bolded) override;
BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) override;
BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) override;

BOOL PrivateWriteConsoleInputW(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& events,
_Out_ size_t& eventsWritten) override;
Expand Down
Loading

0 comments on commit e7f8d31

Please sign in to comment.