diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index 3673aa09d0a..feeb0413979 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -166,7 +166,8 @@ namespace Microsoft::Console::VirtualTerminal constexpr VTParameter at(const size_t index) const noexcept { - return til::at(_subParams, index); + // If the index is out of range, we return a sub parameter with no value. + return index < _subParams.size() ? til::at(_subParams, index) : defaultParameter; } VTSubParameters subspan(const size_t offset, const size_t count) const noexcept @@ -191,6 +192,8 @@ namespace Microsoft::Console::VirtualTerminal } private: + static constexpr VTParameter defaultParameter{}; + std::span _subParams; }; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 3f80931a3d7..3282a2eae90 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -4117,14 +4117,14 @@ void AdaptDispatch::_ReportSGRSetting() const else if (color.IsIndex256()) { const auto index = color.GetIndex(); - fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};5;{}"), base + 8, index); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}:5:{}"), base + 8, index); } else if (color.IsRgb()) { const auto r = GetRValue(color.GetRGB()); const auto g = GetGValue(color.GetRGB()); const auto b = GetBValue(color.GetRGB()); - fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};2;{};{};{}"), base + 8, r, g, b); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}:2::{}:{}:{}"), base + 8, r, g, b); } }; addColor(30, attr.GetForeground()); diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index e0c346dc9d0..87f5f11ba4d 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -297,12 +297,15 @@ namespace Microsoft::Console::VirtualTerminal size_t _SetRgbColorsHelper(const VTParameters options, TextAttribute& attr, const bool isForeground) noexcept; + void _SetRgbColorsHelperFromSubParams(const VTParameter colorItem, + const VTSubParameters options, + TextAttribute& attr) noexcept; size_t _ApplyGraphicsOption(const VTParameters options, const size_t optionIndex, TextAttribute& attr) noexcept; - void _ApplyGraphicsOptionSubParam(const VTParameter option, - const VTSubParameters subParams, - TextAttribute& attr) noexcept; + void _ApplyGraphicsOptionWithSubParams(const VTParameter option, + const VTSubParameters subParams, + TextAttribute& attr) noexcept; void _ApplyGraphicsOptions(const VTParameters options, TextAttribute& attr) noexcept; diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index 5259cf8ce64..a6567abd09e 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -35,7 +35,8 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, const size_t red = options.at(1).value_or(0); const size_t green = options.at(2).value_or(0); const size_t blue = options.at(3).value_or(0); - // ensure that each value fits in a byte + // We only apply the color if the R, G, B values fit within a byte. + // This is to match XTerm's and VTE's behavior. if (red <= 255 && green <= 255 && blue <= 255) { const auto rgbColor = RGB(red, green, blue); @@ -46,6 +47,9 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, { optionsConsumed = 2; const size_t tableIndex = options.at(1).value_or(0); + + // We only apply the color if the index value fit within a byte. + // This is to match XTerm's and VTE's behavior. if (tableIndex <= 255) { const auto adjustedIndex = gsl::narrow_cast(tableIndex); @@ -62,6 +66,77 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options, return optionsConsumed; } +// Routine Description: +// - Helper to parse extended graphics options, which start with 38 (FG) or 48 (BG) +// - These options are followed by either a 2 (RGB) or 5 (xterm index): +// - RGB sequences then take 4 MORE options to designate the ColorSpaceID, R, G, B parts +// of the color. +// - Xterm index will use the option that follows to use a color from the +// preset 256 color xterm color table. +// Arguments: +// - colorItem - One of FG(38) and BG(48), indicating which color we're setting. +// - 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. +// Return Value: +// - +void AdaptDispatch::_SetRgbColorsHelperFromSubParams(const VTParameter colorItem, + const VTSubParameters options, + TextAttribute& attr) noexcept +{ + // This should be called for applying FG and BG colors only. + assert(colorItem == GraphicsOptions::ForegroundExtended || + colorItem == GraphicsOptions::BackgroundExtended); + + const bool isForeground = (colorItem == GraphicsOptions::ForegroundExtended); + const DispatchTypes::GraphicsOptions typeOpt = options.at(0); + + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint) + { + // sub params are in the order: + // :2:::: + + // We treat a color as invalid, if it has a color space ID, as some + // applications that support non-standard ODA color sequence may send + // the red value in its place. + const bool hasColorSpaceId = options.at(1).has_value(); + + // Skip color-space-id. + const size_t red = options.at(2).value_or(0); + const size_t green = options.at(3).value_or(0); + const size_t blue = options.at(4).value_or(0); + + // We only apply the color if the R, G, B values fit within a byte. + // This is to match XTerm's and VTE's behavior. + if (!hasColorSpaceId && red <= 255 && green <= 255 && blue <= 255) + { + const auto rgbColor = RGB(red, green, blue); + attr.SetColor(rgbColor, isForeground); + } + } + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index) + { + // sub params are in the order: + // :5: + // where 'n' is the index into the xterm color table. + const size_t tableIndex = options.at(1).value_or(0); + + // We only apply the color if the index value fit within a byte. + // This is to match XTerm's and VTE's behavior. + if (tableIndex <= 255) + { + const auto adjustedIndex = gsl::narrow_cast(tableIndex); + if (isForeground) + { + attr.SetIndexedForeground256(adjustedIndex); + } + else + { + attr.SetIndexedBackground256(adjustedIndex); + } + } + } +} + // Routine Description: // - Helper to apply a single graphic rendition option to an attribute. // - Calls appropriate helper to apply the option with sub parameters when necessary. @@ -80,7 +155,7 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options, if (options.hasSubParamsFor(optionIndex)) { const auto subParams = options.subParamsFor(optionIndex); - _ApplyGraphicsOptionSubParam(opt, subParams, attr); + _ApplyGraphicsOptionWithSubParams(opt, subParams, attr); return 1; } @@ -260,20 +335,30 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options, } // Routine Description: -// - This is a no-op until we have something meaningful to do with sub parameters. // - Helper to apply a single graphic rendition option with sub parameters to an attribute. // Arguments: // - option - An option to apply. +// - subParams - Sub parameters associated with the option. // - attr - The attribute that will be updated with the applied option. // Return Value: // - -void AdaptDispatch::_ApplyGraphicsOptionSubParam(const VTParameter /* option */, - const VTSubParameters /* subParam */, - TextAttribute& /* attr */) noexcept +void AdaptDispatch::_ApplyGraphicsOptionWithSubParams(const VTParameter option, + const VTSubParameters subParams, + TextAttribute& attr) noexcept { // here, we apply our "best effort" rule, while handling sub params if we don't // recognise the parameter substring (parameter and it's sub parameters) then // we should just skip over them. + switch (option) + { + case ForegroundExtended: + case BackgroundExtended: + _SetRgbColorsHelperFromSubParams(option, subParams, attr); + break; + default: + /* do nothing */ + break; + } } // Routine Description: diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index c0be3a74e0f..162142ae3e4 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -1699,7 +1699,7 @@ class AdapterTest attribute.SetIndexedBackground256(45); _testGetSet->_textBuffer->SetCurrentAttributes(attribute); requestSetting(L"m"); - _testGetSet->ValidateInputEvent(L"\033P1$r0;38;5;123;48;5;45m\033\\"); + _testGetSet->ValidateInputEvent(L"\033P1$r0;38:5:123;48:5:45m\033\\"); Log::Comment(L"Requesting SGR attributes (ITU RGB colors)."); _testGetSet->PrepData(); @@ -1708,7 +1708,7 @@ class AdapterTest attribute.SetBackground(RGB(65, 43, 21)); _testGetSet->_textBuffer->SetCurrentAttributes(attribute); requestSetting(L"m"); - _testGetSet->ValidateInputEvent(L"\033P1$r0;38;2;12;34;56;48;2;65;43;21m\033\\"); + _testGetSet->ValidateInputEvent(L"\033P1$r0;38:2::12:34:56;48:2::65:43:21m\033\\"); Log::Comment(L"Requesting DECSCA attributes (unprotected)."); _testGetSet->PrepData(); @@ -2464,6 +2464,119 @@ class AdapterTest rgOptions[4] = 123; _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 123)); VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 5 })); + + Log::Comment(L"Test 6: Ignore Rgb color when R, G or B is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgOptions[1] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgOptions[2] = 283; + rgOptions[3] = 182; + rgOptions[4] = 123; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 5 })); + + Log::Comment(L"Test 7: Ignore indexed color when index is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgOptions[2] = 283; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, 3 })); + } + + TEST_METHOD(XtermExtendedSubParameterColorTest) + { + Log::Comment(L"Starting test..."); + + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + + VTParameter rgOptions[1]; + VTParameter rgSubParamOpts[16]; + std::pair subParamRanges[1]; + + _testGetSet->_expectedAttribute = _testGetSet->_textBuffer->GetCurrentAttributes(); + + Log::Comment(L"Test 1: Change Indexed Foreground with missing index sub parameter"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + subParamRanges[0] = { (BYTE)0, (BYTE)1 }; + _testGetSet->_expectedAttribute.SetIndexedForeground256(TextColor::DARK_BLACK); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 2: Change Indexed Background with default index sub parameter"); + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgSubParamOpts[1] = {}; + subParamRanges[0] = { (BYTE)0, (BYTE)2 }; + _testGetSet->_expectedAttribute.SetIndexedBackground256(TextColor::DARK_BLACK); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 3: Change RGB Foreground with all RGB sub parameters missing"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + subParamRanges[0] = { (BYTE)0, (BYTE)1 }; + _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 0)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 4: Change RGB Background with some missing RGB sub parameters"); + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + rgSubParamOpts[2] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)3 }; + _testGetSet->_expectedAttribute.SetBackground(RGB(123, 0, 0)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 5: Change RGB Foreground with some default RGB sub parameters"); + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + rgSubParamOpts[2] = {}; + rgSubParamOpts[3] = {}; + rgSubParamOpts[4] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + _testGetSet->_expectedAttribute.SetForeground(RGB(0, 0, 123)); + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 6: Ignore color when ColorSpaceID is not empty"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = 7; // color-space-id + rgSubParamOpts[2] = 182; + rgSubParamOpts[3] = 182; + rgSubParamOpts[4] = 123; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 7: Ignore Rgb color when R, G or B is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::RGBColorOrFaint; + rgSubParamOpts[1] = {}; // color-space-id + // Ensure r, g and b set a color that is different from current color. + // Otherwise, the test will pass even if the color is not ignored. + rgSubParamOpts[2] = 128; + rgSubParamOpts[3] = 283; + rgSubParamOpts[4] = 155; + subParamRanges[0] = { (BYTE)0, (BYTE)5 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); + + Log::Comment(L"Test 8: Ignore indexed color when index is out of range (>255)"); + _testGetSet->PrepData(); // default color from here is gray on black, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED + rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; + rgSubParamOpts[0] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; + rgSubParamOpts[1] = 283; + subParamRanges[0] = { (BYTE)0, (BYTE)2 }; + // expect no change + _testGetSet->_expectedAttribute = TextAttribute{ FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED }; + VERIFY_IS_TRUE(_pDispatch->SetGraphicsRendition({ rgOptions, rgSubParamOpts, subParamRanges })); } TEST_METHOD(SetColorTableValue)