Repository: microsoft/terminal (language: cpp)

Issue:
Merge the legacy and extended attributes
# Description of the new feature/enhancement

In order to implement #14029, there was a need to add another bit in the `ExtendedAttributes` enum, and it occurred to me while looking at that code that it might be a good time to do something refactoring there first.

At the moment we've got two attribute fields in the `TextAttribute` class: `_wAttrLegacy`, which only uses 5 of its available 16 bits; and `_extendedAttrs`, which uses 8 of its available 16 bits. If we merged the two together into a single 16-bit field, we'd still have 3 bits to spare, and the `TextAttribute` class would be two bytes smaller.

# Proposed technical implementation details (optional)

The idea would be to drop the legacy attributes, and replace the existing [`ExtendedAttributes`](https://github.com/microsoft/terminal/blob/0d17769b89a77991ac26f3b24596c4262d9f9332/src/inc/conattrs.hpp#L11-L22) enum with something like this:

```cpp
enum class CharacterAttributes : uint16_t
{
    Normal = 0x00,
    Intense = 0x01,
    Italics = 0x02,
    Blinking = 0x04,
    Invisible = 0x08,
    CrossedOut = 0x10,
    Underlined = 0x20,
    DoublyUnderlined = 0x40,
    Faint = 0x80,
    Unused1 = 0x100,
    Unused2 = 0x200,
    TopGridline = COMMON_LVB_GRID_HORIZONTAL, // 0x400
    LeftGridline = COMMON_LVB_GRID_LVERTICAL, // 0x800
    RightGridline = COMMON_LVB_GRID_RVERTICAL, // 0x1000
    Unused3 = 0x2000,
    ReverseVideo = COMMON_LVB_REVERSE_VIDEO, // 0x4000
    BottomGridline = COMMON_LVB_UNDERSCORE // 0x8000
};
```

The added legacy bits are in the exact same position they were in the original field, so when initializing the `TextAttribute` class from a legacy value, we just have to mask out the relevant bits and write them directly to this field. Same approach works when converting from a `TextAttribute` back to a legacy value.

What do you think? Can I put together a PR with this change?


Discussion / hints:

Fix patch:
diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp
index 54bb9d9123b..e34a7625976 100644
--- a/src/buffer/out/TextAttribute.cpp
+++ b/src/buffer/out/TextAttribute.cpp
@@ -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) == 14);
+static_assert(sizeof(TextAttribute) == 12);
 static_assert(alignof(TextAttribute) == 2);
 // Ensure that we can memcpy() and memmove() the struct for performance.
 static_assert(std::is_trivially_copyable_v<TextAttribute>);
@@ -116,7 +116,7 @@ WORD TextAttribute::GetLegacyAttributes() const noexcept
 {
     const auto fgIndex = _foreground.GetLegacyIndex(s_legacyDefaultForeground);
     const auto bgIndex = _background.GetLegacyIndex(s_legacyDefaultBackground);
-    const WORD metaAttrs = _wAttrLegacy & META_ATTRS;
+    const WORD metaAttrs = static_cast<WORD>(_attrs) & USED_META_ATTRS;
     const auto brighten = IsIntense() && _foreground.CanBeBrightened();
     return fgIndex | (bgIndex << 4) | metaAttrs | (brighten ? FOREGROUND_INTENSITY : 0);
 }
@@ -217,151 +217,141 @@ void TextAttribute::SetHyperlinkId(uint16_t id) noexcept
     _hyperlinkId = id;
 }
 
-bool TextAttribute::IsLeadingByte() const noexcept
-{
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_LEADING_BYTE);
-}
-
-bool TextAttribute::IsTrailingByte() const noexcept
-{
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_LEADING_BYTE);
-}
-
 bool TextAttribute::IsTopHorizontalDisplayed() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::TopGridline);
 }
 
 bool TextAttribute::IsBottomHorizontalDisplayed() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_UNDERSCORE);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::BottomGridline);
 }
 
 bool TextAttribute::IsLeftVerticalDisplayed() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_LVERTICAL);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::LeftGridline);
 }
 
 bool TextAttribute::IsRightVerticalDisplayed() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_RVERTICAL);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::RightGridline);
 }
 
 void TextAttribute::SetLeftVerticalDisplayed(const bool isDisplayed) noexcept
 {
-    WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_LVERTICAL, isDisplayed);
+    WI_UpdateFlag(_attrs, CharacterAttributes::LeftGridline, isDisplayed);
 }
 
 void TextAttribute::SetRightVerticalDisplayed(const bool isDisplayed) noexcept
 {
-    WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_RVERTICAL, isDisplayed);
+    WI_UpdateFlag(_attrs, CharacterAttributes::RightGridline, isDisplayed);
 }
 
 bool TextAttribute::IsIntense() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Intense);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Intense);
 }
 
 bool TextAttribute::IsFaint() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Faint);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Faint);
 }
 
 bool TextAttribute::IsItalic() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Italics);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Italics);
 }
 
 bool TextAttribute::IsBlinking() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Blinking);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Blinking);
 }
 
 bool TextAttribute::IsInvisible() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Invisible);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Invisible);
 }
 
 bool TextAttribute::IsCrossedOut() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::CrossedOut);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::CrossedOut);
 }
 
 bool TextAttribute::IsUnderlined() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Underlined);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::Underlined);
 }
 
 bool TextAttribute::IsDoublyUnderlined() const noexcept
 {
-    return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::DoublyUnderlined);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::DoublyUnderlined);
 }
 
 bool TextAttribute::IsOverlined() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::TopGridline);
 }
 
 bool TextAttribute::IsReverseVideo() const noexcept
 {
-    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
+    return WI_IsFlagSet(_attrs, CharacterAttributes::ReverseVideo);
 }
 
 void TextAttribute::SetIntense(bool isIntense) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Intense, isIntense);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Intense, isIntense);
 }
 
 void TextAttribute::SetFaint(bool isFaint) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Faint, isFaint);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Faint, isFaint);
 }
 
 void TextAttribute::SetItalic(bool isItalic) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Italics, isItalic);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Italics, isItalic);
 }
 
 void TextAttribute::SetBlinking(bool isBlinking) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Blinking, isBlinking);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Blinking, isBlinking);
 }
 
 void TextAttribute::SetInvisible(bool isInvisible) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Invisible, isInvisible);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Invisible, isInvisible);
 }
 
 void TextAttribute::SetCrossedOut(bool isCrossedOut) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::CrossedOut, isCrossedOut);
+    WI_UpdateFlag(_attrs, CharacterAttributes::CrossedOut, isCrossedOut);
 }
 
 void TextAttribute::SetUnderlined(bool isUnderlined) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Underlined, isUnderlined);
+    WI_UpdateFlag(_attrs, CharacterAttributes::Underlined, isUnderlined);
 }
 
 void TextAttribute::SetDoublyUnderlined(bool isDoublyUnderlined) noexcept
 {
-    WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::DoublyUnderlined, isDoublyUnderlined);
+    WI_UpdateFlag(_attrs, CharacterAttributes::DoublyUnderlined, isDoublyUnderlined);
 }
 
 void TextAttribute::SetOverlined(bool isOverlined) noexcept
 {
-    WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL, isOverlined);
+    WI_UpdateFlag(_attrs, CharacterAttributes::TopGridline, isOverlined);
 }
 
 void TextAttribute::SetReverseVideo(bool isReversed) noexcept
 {
-    WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO, isReversed);
+    WI_UpdateFlag(_attrs, CharacterAttributes::ReverseVideo, isReversed);
 }
 
 // Routine Description:
 // - swaps foreground and background color
 void TextAttribute::Invert() noexcept
 {
-    WI_ToggleFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
+    WI_ToggleFlag(_attrs, CharacterAttributes::ReverseVideo);
 }
 
 void TextAttribute::SetDefaultForeground() noexcept
@@ -375,11 +365,10 @@ void TextAttribute::SetDefaultBackground() noexcept
 }
 
 // Method description:
-// - Resets only the meta and extended attributes
-void TextAttribute::SetDefaultMetaAttrs() noexcept
+// - Resets only the rendition character attributes
+void TextAttribute::SetDefaultRenditionAttributes() noexcept
 {
-    _extendedAttrs = ExtendedAttributes::Normal;
-    _wAttrLegacy = 0;
+    _attrs = CharacterAttributes::Normal;
 }
 
 // Method Description:
@@ -398,10 +387,11 @@ bool TextAttribute::BackgroundIsDefault() const noexcept
 }
 
 // Routine Description:
-// - Resets the meta and extended attributes, which is what the VT standard
-//      requires for most erasing and filling operations.
+// - Resets the character attributes, which is what the VT standard
+//      requires for most erasing and filling operations. In modern
+//      applications it is also expected that hyperlinks are erased.
 void TextAttribute::SetStandardErase() noexcept
 {
-    SetDefaultMetaAttrs();
+    _attrs = CharacterAttributes::Normal;
     _hyperlinkId = 0;
 }
diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp
index f43d5846169..83439190526 100644
--- a/src/buffer/out/TextAttribute.hpp
+++ b/src/buffer/out/TextAttribute.hpp
@@ -31,31 +31,26 @@ class TextAttribute final
 {
 public:
     constexpr TextAttribute() noexcept :
-        _wAttrLegacy{ 0 },
+        _attrs{ CharacterAttributes::Normal },
         _foreground{},
         _background{},
-        _extendedAttrs{ ExtendedAttributes::Normal },
         _hyperlinkId{ 0 }
     {
     }
 
     explicit constexpr TextAttribute(const WORD wLegacyAttr) noexcept :
-        _wAttrLegacy{ gsl::narrow_cast<WORD>(wLegacyAttr & META_ATTRS) },
+        _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) },
-        _extendedAttrs{ ExtendedAttributes::Normal },
         _hyperlinkId{ 0 }
     {
-        // If we're given lead/trailing byte information with the legacy color, strip it.
-        WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS);
     }
 
     constexpr TextAttribute(const COLORREF rgbForeground,
                             const COLORREF rgbBackground) noexcept :
-        _wAttrLegacy{ 0 },
+        _attrs{ CharacterAttributes::Normal },
         _foreground{ rgbForeground },
         _background{ rgbBackground },
-        _extendedAttrs{ ExtendedAttributes::Normal },
         _hyperlinkId{ 0 }
     {
     }
@@ -64,8 +59,6 @@ class TextAttribute final
     static TextAttribute StripErroneousVT16VersionsOfLegacyDefaults(const TextAttribute& attribute) noexcept;
     WORD GetLegacyAttributes() const noexcept;
 
-    bool IsLeadingByte() const noexcept;
-    bool IsTrailingByte() const noexcept;
     bool IsTopHorizontalDisplayed() const noexcept;
     bool IsBottomHorizontalDisplayed() const noexcept;
     bool IsLeftVerticalDisplayed() const noexcept;
@@ -109,9 +102,9 @@ class TextAttribute final
     void SetOverlined(bool isOverlined) noexcept;
     void SetReverseVideo(bool isReversed) noexcept;
 
-    constexpr ExtendedAttributes GetExtendedAttributes() const noexcept
+    constexpr CharacterAttributes GetCharacterAttributes() const noexcept
     {
-        return _extendedAttrs;
+        return _attrs;
     }
 
     bool IsHyperlink() const noexcept;
@@ -132,7 +125,7 @@ class TextAttribute final
 
     void SetDefaultForeground() noexcept;
     void SetDefaultBackground() noexcept;
-    void SetDefaultMetaAttrs() noexcept;
+    void SetDefaultRenditionAttributes() noexcept;
 
     bool BackgroundIsDefault() const noexcept;
 
@@ -149,38 +142,33 @@ class TextAttribute final
         const auto checkForeground = (inverted != IsReverseVideo());
         return !IsAnyGridLineEnabled() && // grid lines have a visual representation
                // crossed out, doubly and singly underlined have a visual representation
-               WI_AreAllFlagsClear(_extendedAttrs, ExtendedAttributes::CrossedOut | ExtendedAttributes::DoublyUnderlined | ExtendedAttributes::Underlined) &&
+               WI_AreAllFlagsClear(_attrs, CharacterAttributes::CrossedOut | CharacterAttributes::DoublyUnderlined | CharacterAttributes::Underlined) &&
                // hyperlinks have a visual representation
                !IsHyperlink() &&
                // all other attributes do not have a visual representation
-               (_wAttrLegacy & META_ATTRS) == (other._wAttrLegacy & META_ATTRS) &&
+               _attrs == other._attrs &&
                ((checkForeground && _foreground == other._foreground) ||
                 (!checkForeground && _background == other._background)) &&
-               _extendedAttrs == other._extendedAttrs &&
                IsHyperlink() == other.IsHyperlink();
     }
 
     constexpr bool IsAnyGridLineEnabled() const noexcept
     {
-        return WI_IsAnyFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_UNDERSCORE);
+        return WI_IsAnyFlagSet(_attrs, CharacterAttributes::TopGridline | CharacterAttributes::LeftGridline | CharacterAttributes::RightGridline | CharacterAttributes::BottomGridline);
     }
-    constexpr bool HasAnyExtendedAttributes() const noexcept
+    constexpr bool HasAnyVisualAttributes() const noexcept
     {
-        return GetExtendedAttributes() != ExtendedAttributes::Normal ||
-               IsAnyGridLineEnabled() ||
-               GetHyperlinkId() != 0 ||
-               IsReverseVideo();
+        return GetCharacterAttributes() != CharacterAttributes::Normal || GetHyperlinkId() != 0;
     }
 
 private:
     static std::array<TextColor, 16> s_legacyForegroundColorMap;
     static std::array<TextColor, 16> s_legacyBackgroundColorMap;
 
-    uint16_t _wAttrLegacy; // sizeof: 2, alignof: 2
+    CharacterAttributes _attrs; // sizeof: 2, alignof: 2
     uint16_t _hyperlinkId; // sizeof: 2, alignof: 2
     TextColor _foreground; // sizeof: 4, alignof: 1
     TextColor _background; // sizeof: 4, alignof: 1
-    ExtendedAttributes _extendedAttrs; // sizeof: 2, alignof: 2
 
 #ifdef UNIT_TESTING
     friend class TextBufferTests;
@@ -213,12 +201,11 @@ namespace WEX
             static WEX::Common::NoThrowString ToString(const TextAttribute& attr)
             {
                 return WEX::Common::NoThrowString().Format(
-                    L"{FG:%s,BG:%s,intense:%d,wLegacy:(0x%04x),ext:(0x%02x)}",
+                    L"{FG:%s,BG:%s,intense:%d,attrs:(0x%02x)}",
                     VerifyOutputTraits<TextColor>::ToString(attr._foreground).GetBuffer(),
                     VerifyOutputTraits<TextColor>::ToString(attr._background).GetBuffer(),
                     attr.IsIntense(),
-                    attr._wAttrLegacy,
-                    static_cast<DWORD>(attr._extendedAttrs));
+                    static_cast<DWORD>(attr._attrs));
             }
         };
     }
diff --git a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp
index b807590f1e6..98cdd28f83f 100644
--- a/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp
+++ b/src/buffer/out/ut_textbuffer/TextAttributeTests.cpp
@@ -75,7 +75,7 @@ void TextAttributeTests::TestRoundtripMetaBits()
         auto attr = TextAttribute(expectedLegacy);
         VERIFY_IS_TRUE(attr.IsLegacy());
         VERIFY_ARE_EQUAL(expectedLegacy, attr.GetLegacyAttributes());
-        VERIFY_ARE_EQUAL(flag, attr._wAttrLegacy);
+        VERIFY_ARE_EQUAL(flag, static_cast<WORD>(attr._attrs));
     }
 }
 
diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp
index 22c5df8c6a5..7900c2995f3 100644
--- a/src/host/ut_host/ScreenBufferTests.cpp
+++ b/src/host/ut_host/ScreenBufferTests.cpp
@@ -5406,54 +5406,54 @@ void ScreenBufferTests::TestExtendedTextAttributes()
     auto& stateMachine = si.GetStateMachine();
     auto& cursor = tbi.GetCursor();
 
-    auto expectedAttrs{ ExtendedAttributes::Normal };
+    auto expectedAttrs{ CharacterAttributes::Normal };
     std::wstring vtSeq = L"";
 
     // Collect up a VT sequence to set the state given the method properties
     if (intense)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Intense);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Intense);
         vtSeq += L"\x1b[1m";
     }
     if (faint)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Faint);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Faint);
         vtSeq += L"\x1b[2m";
     }
     if (italics)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Italics);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Italics);
         vtSeq += L"\x1b[3m";
     }
     if (underlined)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Underlined);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Underlined);
         vtSeq += L"\x1b[4m";
     }
     if (doublyUnderlined)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::DoublyUnderlined);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::DoublyUnderlined);
         vtSeq += L"\x1b[21m";
     }
     if (blink)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Blinking);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Blinking);
         vtSeq += L"\x1b[5m";
     }
     if (invisible)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::Invisible);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::Invisible);
         vtSeq += L"\x1b[8m";
     }
     if (crossedOut)
     {
-        WI_SetFlag(expectedAttrs, ExtendedAttributes::CrossedOut);
+        WI_SetFlag(expectedAttrs, CharacterAttributes::CrossedOut);
         vtSeq += L"\x1b[9m";
     }
 
     // Helper lambda to write a VT sequence, then an "X", then check that the
     // attributes of the "X" match what we think they should be.
-    auto validate = [&](const ExtendedAttributes expectedAttrs,
+    auto validate = [&](const CharacterAttributes expectedAttrs,
                         const std::wstring& vtSequence) {
         auto cursorPos = cursor.GetPosition();
 
@@ -5478,8 +5478,8 @@ void ScreenBufferTests::TestExtendedTextAttributes()
         stateMachine.ProcessString(L"X");
 
         auto iter = tbi.GetCellDataAt(cursorPos);
-        auto currentExtendedAttrs = iter->TextAttr().GetExtendedAttributes();
-        VERIFY_ARE_EQUAL(expectedAttrs, currentExtendedAttrs);
+        auto currentAttrs = iter->TextAttr().GetCharacterAttributes();
+        VERIFY_ARE_EQUAL(expectedAttrs, currentAttrs);
     };
 
     // Check setting all the states collected above
@@ -5490,38 +5490,38 @@ void ScreenBufferTests::TestExtendedTextAttributes()
     if (intense || faint)
     {
         // The intense and faint attributes share the same reset sequence.
-        WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Intense | ExtendedAttributes::Faint);
+        WI_ClearAllFlags(expectedAttrs, CharacterAttributes::Intense | CharacterAttributes::Faint);
         vtSeq = L"\x1b[22m";
         validate(expectedAttrs, vtSeq);
     }
     if (italics)
     {
-        WI_ClearFlag(expectedAttrs, ExtendedAttributes::Italics);
+        WI_ClearFlag(expectedAttrs, CharacterAttributes::Italics);
         vtSeq = L"\x1b[23m";
         validate(expectedAttrs, vtSeq);
     }
     if (underlined || doublyUnderlined)
     {
         // The two underlined attributes share the same reset sequence.
-        WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Underlined | ExtendedAttributes::DoublyUnderlined);
+        WI_ClearAllFlags(expectedAttrs, CharacterAttributes::Underlined | CharacterAttributes::DoublyUnderlined);
         vtSeq = L"\x1b[24m";
         validate(expectedAttrs, vtSeq);
     }
     if (blink)
     {
-        WI_ClearFlag(expectedAttrs, ExtendedAttributes::Blinking);
+        WI_ClearFlag(expectedAttrs, CharacterAttributes::Blinking);
         vtSeq = L"\x1b[25m";
         validate(expectedAttrs, vtSeq);
     }
     if (invisible)
     {
-        WI_ClearFlag(expectedAttrs, ExtendedAttributes::Invisible);
+        WI_ClearFlag(expectedAttrs, CharacterAttributes::Invisible);
         vtSeq = L"\x1b[28m";
         validate(expectedAttrs, vtSeq);
     }
     if (crossedOut)
     {
-        WI_ClearFlag(expectedAttrs, ExtendedAttributes::CrossedOut);
+        WI_ClearFlag(expectedAttrs, CharacterAttributes::CrossedOut);
         vtSeq = L"\x1b[29m";
         validate(expectedAttrs, vtSeq);
     }
diff --git a/src/inc/conattrs.hpp b/src/inc/conattrs.hpp
index 35543d25427..b7825db4493 100644
--- a/src/inc/conattrs.hpp
+++ b/src/inc/conattrs.hpp
@@ -7,8 +7,9 @@ Licensed under the MIT license.
 #define FG_ATTRS (FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_INTENSITY)
 #define BG_ATTRS (BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY)
 #define META_ATTRS (COMMON_LVB_LEADING_BYTE | COMMON_LVB_TRAILING_BYTE | COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE)
+#define USED_META_ATTRS (META_ATTRS & ~COMMON_LVB_SBCSDBCS) // We don't retain lead/trailing byte information in the TextAttribute class
 
-enum class ExtendedAttributes : uint16_t
+enum class CharacterAttributes : uint16_t
 {
     Normal = 0x00,
     Intense = 0x01,
@@ -19,8 +20,16 @@ enum class ExtendedAttributes : uint16_t
     Underlined = 0x20,
     DoublyUnderlined = 0x40,
     Faint = 0x80,
+    Unused1 = 0x100,
+    Unused2 = 0x200,
+    TopGridline = COMMON_LVB_GRID_HORIZONTAL, // 0x400
+    LeftGridline = COMMON_LVB_GRID_LVERTICAL, // 0x800
+    RightGridline = COMMON_LVB_GRID_RVERTICAL, // 0x1000
+    Unused3 = 0x2000,
+    ReverseVideo = COMMON_LVB_REVERSE_VIDEO, // 0x4000
+    BottomGridline = COMMON_LVB_UNDERSCORE // 0x8000
 };
-DEFINE_ENUM_FLAG_OPERATORS(ExtendedAttributes);
+DEFINE_ENUM_FLAG_OPERATORS(CharacterAttributes);
 
 enum class CursorType : unsigned int
 {
diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index 228ef858016..169bcb83449 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -256,7 +256,7 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_SetGraphicsDefault());
         _lastTextAttributes.SetDefaultBackground();
         _lastTextAttributes.SetDefaultForeground();
-        _lastTextAttributes.SetDefaultMetaAttrs();
+        _lastTextAttributes.SetDefaultRenditionAttributes();
         lastFg = {};
         lastBg = {};
     }
@@ -332,7 +332,7 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_SetGraphicsDefault());
         _lastTextAttributes.SetDefaultBackground();
         _lastTextAttributes.SetDefaultForeground();
-        _lastTextAttributes.SetDefaultMetaAttrs();
+        _lastTextAttributes.SetDefaultRenditionAttributes();
         lastFg = {};
         lastBg = {};
     }
@@ -476,14 +476,14 @@ using namespace Microsoft::Console::Types;
     // and it uses xterm-ascii. This ensures that xterm and -256color consumers
     // get the enhancements, and telnet isn't broken.
     //
-    // GH#13229: ECH and EL don't fill the space with "meta" attributes like
+    // GH#13229: ECH and EL don't fill the space with visual attributes like
     // underline, reverse video, hyperlinks, etc. If these spaces had those
     // attrs, then don't try and optimize them out.
     const auto optimalToUseECH = numSpaces > ERASE_CHARACTER_STRING_LENGTH;
     const auto useEraseChar = (optimalToUseECH) &&
                               (!_newBottomLine) &&
                               (!_clearedAllThisFrame) &&
-                              (!_lastTextAttributes.HasAnyExtendedAttributes());
+                              (!_lastTextAttributes.HasAnyVisualAttributes());
     const auto printingBottomLine = coord.Y == _lastViewport.BottomInclusive();
 
     // GH#5502 - If the background color of the "new bottom line" is different
diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp
index 12485f9227a..e1aaaaf110c 100644
--- a/src/terminal/adapter/adaptDispatchGraphics.cpp
+++ b/src/terminal/adapter/adaptDispatchGraphics.cpp
@@ -85,7 +85,7 @@ bool AdaptDispatch::SetGraphicsRendition(const VTParameters options)
         case Off:
             attr.SetDefaultForeground();
             attr.SetDefaultBackground();
-            attr.SetDefaultMetaAttrs();
+            attr.SetDefaultRenditionAttributes();
             break;
         case ForegroundDefault:
             attr.SetDefaultForeground();


Test patch: