From 581c69739dab6232050e5f9282e3cb1fa7f56efc Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Fri, 11 Jun 2021 17:20:40 +0800 Subject: [PATCH 01/12] Draft implementation for fast string formatting --- src/inc/til/format.h | 57 +++++++++++++++++++++++++++++++++ src/renderer/vt/VtSequences.cpp | 18 ++++------- 2 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 src/inc/til/format.h diff --git a/src/inc/til/format.h b/src/inc/til/format.h new file mode 100644 index 00000000000..5cae32e9df8 --- /dev/null +++ b/src/inc/til/format.h @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include +#include + +#ifdef UNIT_TESTING +#endif + +namespace til // Terminal Implementation Library. Also: "Today I Learned" +{ + namespace format + { + inline char* format_uint8(char* ptr, uint8_t number) + { + if (number >= 100) + { + *ptr++ = static_cast((number / 100) + '0'); + } + if (number >= 10) + { + *ptr++ = static_cast(((number / 10) % 10) + '0'); + } + + *ptr++ = static_cast((number % 10) + '0'); + return ptr; + } + + std::string_view format(unsigned int maxLength, const char* pFormat, ...) + { + va_list args; + va_start(args, pFormat); + + char* buf = new char[maxLength]; + char* pDst = buf; + + const char* pSrc = pFormat; + while (*pSrc != '\0') + { + if (*pSrc == '{' && *(pSrc + 1) == '}') + { + pDst = format_uint8(pDst, va_arg(args, uint8_t)); + pSrc += 2; + continue; + } + + *pDst++ = *pSrc++; + } + + va_end(args); + + return { buf, static_cast(pDst - buf) }; + } + } +} diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 157d951029f..5395031a878 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -4,6 +4,7 @@ #include "precomp.h" #include "vtrenderer.hpp" #include "../../inc/conattrs.hpp" +#include "til/format.h" #pragma hdrstop using namespace Microsoft::Console::Render; @@ -234,7 +235,8 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); - return _WriteFormattedString(&fmt, vtIndex); + // An example string with max length would be "\x1b[100m", which has length = 5. + return _Write(til::format::format(6, "\x1b[{}m", vtIndex)); } // Method Description: @@ -248,11 +250,8 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { - const std::string fmt = fIsForeground ? - "\x1b[38;5;%dm" : - "\x1b[48;5;%dm"; - - return _WriteFormattedString(&fmt, ::Xterm256ToWindowsIndex(index)); + // An example string with max length would be "\x1b[38;5;128m", which has length = 10. + return _Write(til::format::format(11, "\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); } // Method Description: @@ -266,15 +265,12 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRenditionRGBColor(const COLORREF color, const bool fIsForeground) noexcept { - const std::string fmt = fIsForeground ? - "\x1b[38;2;%d;%d;%dm" : - "\x1b[48;2;%d;%d;%dm"; - DWORD const r = GetRValue(color); DWORD const g = GetGValue(color); DWORD const b = GetBValue(color); - return _WriteFormattedString(&fmt, r, g, b); + // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 18. + return _Write(til::format::format(19, "\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); } // Method Description: From 1ca22cd8d61b7515876c21e89fb18cd3a4df6d27 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Fri, 11 Jun 2021 18:31:20 +0800 Subject: [PATCH 02/12] why not work --- src/inc/til/format.h | 5 +++-- src/renderer/vt/VtSequences.cpp | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/inc/til/format.h b/src/inc/til/format.h index 5cae32e9df8..7b0f8d26a43 100644 --- a/src/inc/til/format.h +++ b/src/inc/til/format.h @@ -28,12 +28,13 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return ptr; } - std::string_view format(unsigned int maxLength, const char* pFormat, ...) + template + constexpr std::string_view format(const char* pFormat, ...) { va_list args; va_start(args, pFormat); - char* buf = new char[maxLength]; + char buf[N]; char* pDst = buf; const char* pSrc = pFormat; diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 5395031a878..f7f2cb1a4a1 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -236,7 +236,7 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); // An example string with max length would be "\x1b[100m", which has length = 5. - return _Write(til::format::format(6, "\x1b[{}m", vtIndex)); + return _Write(til::format::format<6>("\x1b[{}m", vtIndex)); } // Method Description: @@ -251,7 +251,7 @@ using namespace Microsoft::Console::Render; const bool fIsForeground) noexcept { // An example string with max length would be "\x1b[38;5;128m", which has length = 10. - return _Write(til::format::format(11, "\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); + return _Write(til::format::format<11>("\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); } // Method Description: @@ -270,7 +270,7 @@ using namespace Microsoft::Console::Render; DWORD const b = GetBValue(color); // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 18. - return _Write(til::format::format(19, "\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); + return _Write(til::format::format<19>("\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); } // Method Description: From 7231332377db11c4d024a58c782179836cada5d6 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Sat, 12 Jun 2021 09:33:17 +0800 Subject: [PATCH 03/12] OK now it works --- src/inc/til/format.h | 2 +- src/renderer/vt/VtSequences.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/inc/til/format.h b/src/inc/til/format.h index 7b0f8d26a43..862877e9061 100644 --- a/src/inc/til/format.h +++ b/src/inc/til/format.h @@ -29,7 +29,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } template - constexpr std::string_view format(const char* pFormat, ...) + constexpr std::string format(const char* pFormat, ...) { va_list args; va_start(args, pFormat); diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index f7f2cb1a4a1..a2175c73040 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -235,8 +235,8 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); - // An example string with max length would be "\x1b[100m", which has length = 5. - return _Write(til::format::format<6>("\x1b[{}m", vtIndex)); + // An example string with max length would be "\x1b[100m", which has length = 6. + return _Write(til::format::format<7>("\x1b[{}m", vtIndex)); } // Method Description: @@ -250,8 +250,8 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { - // An example string with max length would be "\x1b[38;5;128m", which has length = 10. - return _Write(til::format::format<11>("\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); + // An example string with max length would be "\x1b[38;5;128m", which has length = 11. + return _Write(til::format::format<12>("\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); } // Method Description: @@ -269,8 +269,8 @@ using namespace Microsoft::Console::Render; DWORD const g = GetGValue(color); DWORD const b = GetBValue(color); - // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 18. - return _Write(til::format::format<19>("\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); + // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 19. + return _Write(til::format::format<20>("\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); } // Method Description: From d03136e90f26650c4f9ea46e44c631ccb53e87c2 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Tue, 15 Jun 2021 11:46:02 +0800 Subject: [PATCH 04/12] Stack allocation only --- src/inc/til/format.h | 8 ++------ src/renderer/vt/VtSequences.cpp | 13 ++++++++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/inc/til/format.h b/src/inc/til/format.h index 862877e9061..b1c492fd460 100644 --- a/src/inc/til/format.h +++ b/src/inc/til/format.h @@ -28,15 +28,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return ptr; } - template - constexpr std::string format(const char* pFormat, ...) + inline char* format(char* pDst, const char* pFormat, ...) { va_list args; va_start(args, pFormat); - char buf[N]; - char* pDst = buf; - const char* pSrc = pFormat; while (*pSrc != '\0') { @@ -52,7 +48,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" va_end(args); - return { buf, static_cast(pDst - buf) }; + return pDst; } } } diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index a2175c73040..a18cc7fbf85 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -235,8 +235,10 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); + char buf[6]; + char* end = til::format::format(buf, "\x1b[{}m", vtIndex); // An example string with max length would be "\x1b[100m", which has length = 6. - return _Write(til::format::format<7>("\x1b[{}m", vtIndex)); + return _Write({ buf, static_cast(end - buf) }); } // Method Description: @@ -250,8 +252,10 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { + char buf[11]; // An example string with max length would be "\x1b[38;5;128m", which has length = 11. - return _Write(til::format::format<12>("\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index))); + char* end = til::format::format(buf, "\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); + return _Write({buf, static_cast(end - buf)}); } // Method Description: @@ -269,8 +273,11 @@ using namespace Microsoft::Console::Render; DWORD const g = GetGValue(color); DWORD const b = GetBValue(color); + char buf[19]; + char* end = til::format::format(buf, "\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b); + // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 19. - return _Write(til::format::format<20>("\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b)); + return _Write({ buf, static_cast(end - buf) }); } // Method Description: From f48e59427f5d0bce727df6f1cb340d25ebd52104 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Wed, 16 Jun 2021 12:11:37 +0800 Subject: [PATCH 05/12] Use FMT_COMPILE --- src/inc/til/format.h | 54 --------------------------------- src/renderer/vt/VtSequences.cpp | 15 ++++----- 2 files changed, 6 insertions(+), 63 deletions(-) delete mode 100644 src/inc/til/format.h diff --git a/src/inc/til/format.h b/src/inc/til/format.h deleted file mode 100644 index b1c492fd460..00000000000 --- a/src/inc/til/format.h +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#pragma once - -#include -#include - -#ifdef UNIT_TESTING -#endif - -namespace til // Terminal Implementation Library. Also: "Today I Learned" -{ - namespace format - { - inline char* format_uint8(char* ptr, uint8_t number) - { - if (number >= 100) - { - *ptr++ = static_cast((number / 100) + '0'); - } - if (number >= 10) - { - *ptr++ = static_cast(((number / 10) % 10) + '0'); - } - - *ptr++ = static_cast((number % 10) + '0'); - return ptr; - } - - inline char* format(char* pDst, const char* pFormat, ...) - { - va_list args; - va_start(args, pFormat); - - const char* pSrc = pFormat; - while (*pSrc != '\0') - { - if (*pSrc == '{' && *(pSrc + 1) == '}') - { - pDst = format_uint8(pDst, va_arg(args, uint8_t)); - pSrc += 2; - continue; - } - - *pDst++ = *pSrc++; - } - - va_end(args); - - return pDst; - } - } -} diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index a18cc7fbf85..83ec16bde00 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -4,7 +4,6 @@ #include "precomp.h" #include "vtrenderer.hpp" #include "../../inc/conattrs.hpp" -#include "til/format.h" #pragma hdrstop using namespace Microsoft::Console::Render; @@ -214,8 +213,6 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition16Color(const WORD wAttr, const bool fIsForeground) noexcept { - static const std::string fmt = "\x1b[%dm"; - // Always check using the foreground flags, because the bg flags constants // are a higher byte // Foreground sequences are in [30,37] U [90,97] @@ -235,9 +232,9 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); - char buf[6]; - char* end = til::format::format(buf, "\x1b[{}m", vtIndex); // An example string with max length would be "\x1b[100m", which has length = 6. + char buf[6]; + char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{}m"), vtIndex); return _Write({ buf, static_cast(end - buf) }); } @@ -252,9 +249,9 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { - char buf[11]; // An example string with max length would be "\x1b[38;5;128m", which has length = 11. - char* end = til::format::format(buf, "\x1b[{};5;{}m", fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); + char buf[11]; + char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};5;{}m"), fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); return _Write({buf, static_cast(end - buf)}); } @@ -273,10 +270,10 @@ using namespace Microsoft::Console::Render; DWORD const g = GetGValue(color); DWORD const b = GetBValue(color); + // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 19. char buf[19]; - char* end = til::format::format(buf, "\x1b[{};2;{};{};{}m", fIsForeground ? 38 : 48, r, g, b); + char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); - // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 19. return _Write({ buf, static_cast(end - buf) }); } From 8b899407827e7c9421984c56fea33c18321b54f7 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Wed, 16 Jun 2021 12:14:37 +0800 Subject: [PATCH 06/12] Code health --- src/renderer/vt/VtSequences.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 83ec16bde00..f05bc7fd4f4 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -252,7 +252,7 @@ using namespace Microsoft::Console::Render; // An example string with max length would be "\x1b[38;5;128m", which has length = 11. char buf[11]; char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};5;{}m"), fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); - return _Write({buf, static_cast(end - buf)}); + return _Write({ buf, static_cast(end - buf) }); } // Method Description: From c677fbd79b72ad323d61a537d10ce1db9a4f2587 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Thu, 17 Jun 2021 16:21:51 +0800 Subject: [PATCH 07/12] small_vector --- src/renderer/vt/VtSequences.cpp | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index f05bc7fd4f4..5c54fb66eaa 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -232,10 +232,8 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); - // An example string with max length would be "\x1b[100m", which has length = 6. - char buf[6]; - char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{}m"), vtIndex); - return _Write({ buf, static_cast(end - buf) }); + auto s = fmt::format(FMT_COMPILE("\x1b[{}m"), vtIndex); + return _Write(s); } // Method Description: @@ -249,10 +247,8 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { - // An example string with max length would be "\x1b[38;5;128m", which has length = 11. - char buf[11]; - char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};5;{}m"), fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); - return _Write({ buf, static_cast(end - buf) }); + auto s = fmt::format(FMT_COMPILE("\x1b[{};5;{}m"), fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); + return _Write(s); } // Method Description: @@ -270,11 +266,11 @@ using namespace Microsoft::Console::Render; DWORD const g = GetGValue(color); DWORD const b = GetBValue(color); - // An example string with max length would be "\x1b[38;2;128;128;128m", which has length = 19. - char buf[19]; - char* end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); - - return _Write({ buf, static_cast(end - buf) }); + // Worst case scenario: "\x1b[38;2;128;128;128m", which has length = 19 + \0. + // Use small_vector as MSVC's SSO only buffers up to 15 characters. + boost::container::small_vector buf; + const auto end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); + return _Write({ buf.data(), static_cast(end - buf.begin()) }); } // Method Description: From 1321f05ab5226509a288ee10c5e250ec908f2fa5 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Fri, 18 Jun 2021 14:22:43 +0800 Subject: [PATCH 08/12] Use std::back_insertor --- .github/actions/spelling/allow/microsoft.txt | 1 + src/renderer/vt/VtSequences.cpp | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/actions/spelling/allow/microsoft.txt b/.github/actions/spelling/allow/microsoft.txt index b8187ab507a..56cd73c70e7 100644 --- a/.github/actions/spelling/allow/microsoft.txt +++ b/.github/actions/spelling/allow/microsoft.txt @@ -29,6 +29,7 @@ mfcribbon microsoft microsoftonline msixbundle +MSVC muxc netcore osgvsowi diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 5c54fb66eaa..e9c561c26cd 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -269,8 +269,8 @@ using namespace Microsoft::Console::Render; // Worst case scenario: "\x1b[38;2;128;128;128m", which has length = 19 + \0. // Use small_vector as MSVC's SSO only buffers up to 15 characters. boost::container::small_vector buf; - const auto end = fmt::format_to(std::begin(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); - return _Write({ buf.data(), static_cast(end - buf.begin()) }); + const auto end = fmt::format_to(std::back_inserter(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); + return _Write({ buf.data(), buf.size() }); } // Method Description: From 88e8e0a3811f88de9c96b356acb53d12b6afaf08 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Sun, 20 Jun 2021 21:10:25 +0800 Subject: [PATCH 09/12] Apply patch from Leonard --- src/host/ut_host/VtRendererTests.cpp | 10 ++--- src/renderer/vt/VtSequences.cpp | 59 ++++++++---------------- src/renderer/vt/state.cpp | 67 +--------------------------- src/renderer/vt/vtrenderer.hpp | 12 ++++- src/terminal/parser/stateMachine.cpp | 4 +- 5 files changed, 37 insertions(+), 115 deletions(-) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 08f953bb47c..45e0fa8507d 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -1562,7 +1562,7 @@ void VtRendererTest::FormattedString() TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") END_TEST_METHOD_PROPERTIES(); - static const std::string format("\x1b[%dm"); + static const auto format = FMT_COMPILE("\x1b[{}m"); const auto value = 12; Viewport view = SetUpViewport(); @@ -1573,15 +1573,15 @@ void VtRendererTest::FormattedString() Log::Comment(L"1.) Write it once. It should resize itself."); qExpectedInput.push_back("\x1b[12m"); - VERIFY_SUCCEEDED(engine->_WriteFormattedString(&format, value)); + VERIFY_SUCCEEDED(engine->_WriteFormatted(format, value)); Log::Comment(L"2.) Write the same thing again, should be fine."); qExpectedInput.push_back("\x1b[12m"); - VERIFY_SUCCEEDED(engine->_WriteFormattedString(&format, value)); + VERIFY_SUCCEEDED(engine->_WriteFormatted(&format, value)); Log::Comment(L"3.) Now write something huge. Should resize itself and still be fine."); - static const std::string bigFormat("\x1b[28;3;%d;%d;%dm"); + static const auto bigFormat = FMT_COMPILE("\x1b[28;3;{};{};{}m"); const auto bigValue = 500; qExpectedInput.push_back("\x1b[28;3;500;500;500m"); - VERIFY_SUCCEEDED(engine->_WriteFormattedString(&bigFormat, bigValue, bigValue, bigValue)); + VERIFY_SUCCEEDED(engine->_WriteFormatted(&bigFormat, bigValue, bigValue, bigValue)); } diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index e9c561c26cd..26415e41088 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -81,8 +81,7 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_EraseCharacter(const short chars) noexcept { - static const std::string format = "\x1b[%dX"; - return _WriteFormattedString(&format, chars); + return _WriteFormatted(FMT_COMPILE("\x1b[{}X"), chars); } // Method Description: @@ -93,8 +92,7 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_CursorForward(const short chars) noexcept { - static const std::string format = "\x1b[%dC"; - return _WriteFormattedString(&format, chars); + return _WriteFormatted(FMT_COMPILE("\x1b[{}C"), chars); } // Method Description: @@ -132,9 +130,8 @@ using namespace Microsoft::Console::Render; { return _Write(fInsertLine ? "\x1b[L" : "\x1b[M"); } - const std::string format = fInsertLine ? "\x1b[%dL" : "\x1b[%dM"; - return _WriteFormattedString(&format, sLines); + return _WriteFormatted(FMT_COMPILE("\x1b[{}{}"), sLines, fInsertLine ? 'L' : 'M'); } // Method Description: @@ -171,14 +168,7 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_CursorPosition(const COORD coord) noexcept { - static const std::string cursorFormat = "\x1b[%d;%dH"; - - // VT coords start at 1,1 - COORD coordVt = coord; - coordVt.X++; - coordVt.Y++; - - return _WriteFormattedString(&cursorFormat, coordVt.Y, coordVt.X); + return _WriteFormatted(FMT_COMPILE("\x1b[{};{}H"), coord.Y + 1, coord.X + 1); } // Method Description: @@ -232,8 +222,7 @@ using namespace Microsoft::Console::Render; (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) + (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0); - auto s = fmt::format(FMT_COMPILE("\x1b[{}m"), vtIndex); - return _Write(s); + return _WriteFormatted(FMT_COMPILE("\x1b[{}m"), vtIndex); } // Method Description: @@ -247,8 +236,7 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index, const bool fIsForeground) noexcept { - auto s = fmt::format(FMT_COMPILE("\x1b[{};5;{}m"), fIsForeground ? 38 : 48, ::Xterm256ToWindowsIndex(index)); - return _Write(s); + return _WriteFormatted(FMT_COMPILE("\x1b[{}8;5;{}m"), fIsForeground ? '3' : '4', ::Xterm256ToWindowsIndex(index)); } // Method Description: @@ -262,15 +250,10 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::_SetGraphicsRenditionRGBColor(const COLORREF color, const bool fIsForeground) noexcept { - DWORD const r = GetRValue(color); - DWORD const g = GetGValue(color); - DWORD const b = GetBValue(color); - - // Worst case scenario: "\x1b[38;2;128;128;128m", which has length = 19 + \0. - // Use small_vector as MSVC's SSO only buffers up to 15 characters. - boost::container::small_vector buf; - const auto end = fmt::format_to(std::back_inserter(buf), FMT_COMPILE("\x1b[{};2;{};{};{}m"), fIsForeground ? 38 : 48, r, g, b); - return _Write({ buf.data(), buf.size() }); + const uint8_t r = GetRValue(color); + const uint8_t g = GetGValue(color); + const uint8_t b = GetBValue(color); + return _WriteFormatted(FMT_COMPILE("\x1b[{}8;2;{};{};{}m"), fIsForeground ? '3' : '4', r, g, b); } // Method Description: @@ -282,9 +265,7 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_SetGraphicsRenditionDefaultColor(const bool fIsForeground) noexcept { - const std::string_view fmt = fIsForeground ? ("\x1b[39m") : ("\x1b[49m"); - - return _Write(fmt); + return _Write(fIsForeground ? ("\x1b[39m") : ("\x1b[49m")); } // Method Description: @@ -296,13 +277,12 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_ResizeWindow(const short sWidth, const short sHeight) noexcept { - static const std::string resizeFormat = "\x1b[8;%d;%dt"; if (sWidth < 0 || sHeight < 0) { return E_INVALIDARG; } - return _WriteFormattedString(&resizeFormat, sHeight, sWidth); + return _WriteFormatted(FMT_COMPILE("\x1b[8;{};{}t"), sHeight, sWidth); } // Method Description: @@ -325,8 +305,7 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_ChangeTitle(_In_ const std::string& title) noexcept { - const std::string titleFormat = "\x1b]0;" + title + "\x7"; - return _Write(titleFormat); + return _WriteFormatted(FMT_COMPILE("\x1b]0;{}\x7"), title); } // Method Description: @@ -468,19 +447,17 @@ using namespace Microsoft::Console::Render; // send the auto-assigned ID, prefixed with the PID of this session // (we do this so different conpty sessions do not overwrite each other's hyperlinks) const auto sessionID = GetCurrentProcessId(); - const std::string uri_str{ til::u16u8(uri) }; - auto s = fmt::format(FMT_COMPILE("\x1b]8;id={}-{};{}\x1b\\"), sessionID, numberId, uri_str); - return _Write(s); + const auto uriStr = til::u16u8(uri); + return _WriteFormatted(FMT_COMPILE("\x1b]8;id={}-{};{}\x1b\\"), sessionID, numberId, uriStr); } else { // This is the case of user-defined IDs: // send the user-defined ID, prefixed with a "u" // (we do this so no application can accidentally override a user defined ID) - const std::string uri_str{ til::u16u8(uri) }; - const std::string customId_str{ til::u16u8(customId) }; - auto s = fmt::format(FMT_COMPILE("\x1b]8;id=u-{};{}\x1b\\"), customId_str, uri_str); - return _Write(s); + const auto uriStr = til::u16u8(uri); + const auto customIdStr = til::u16u8(customId); + return _WriteFormatted(FMT_COMPILE("\x1b]8;id=u-{};{}\x1b\\"), customIdStr, uriStr); } } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index a324e988b00..5fc18533891 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -115,7 +115,7 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, if (!_pipeBroken) { - bool fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), static_cast(_buffer.size()), nullptr, nullptr); + bool fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), gsl::narrow_cast(_buffer.size()), nullptr, nullptr); _buffer.clear(); if (!fSuccess) { @@ -178,71 +178,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, return _Write(needed); } -// Method Description: -// - Helper for calling _Write with a string for formatting a sequence. Used -// extensively by VtSequences.cpp -// Arguments: -// - pFormat: pointer to format string to write to the pipe -// - ...: a va_list of args to format the string with. -// Return Value: -// - S_OK, E_INVALIDARG for a invalid format string, or suitable HRESULT error -// from writing pipe. -[[nodiscard]] HRESULT VtEngine::_WriteFormattedString(const std::string* const pFormat, ...) noexcept -try -{ - va_list args; - va_start(args, pFormat); - - // NOTE: pFormat is a pointer because varargs refuses to operate with a ref in that position - // NOTE: We're not using string_view because it doesn't guarantee null (which will be needed - // later in the formatting method). - - HRESULT hr = E_FAIL; - - // We're going to hold onto our format string space across calls because - // the VT renderer will be formatting a LOT of strings and alloc/freeing them - // over and over is going to be way worse for perf than just holding some extra - // memory for formatting purposes. - // See _formatBuffer for its location. - - // First, plow ahead using our pre-reserved string space. - LPSTR destEnd = nullptr; - size_t destRemaining = 0; - if (SUCCEEDED(StringCchVPrintfExA(_formatBuffer.data(), - _formatBuffer.size(), - &destEnd, - &destRemaining, - STRSAFE_NO_TRUNCATION, - pFormat->c_str(), - args))) - { - return _Write({ _formatBuffer.data(), _formatBuffer.size() - destRemaining }); - } - - // If we didn't succeed at filling/using the existing space, then - // we're going to take the long way by counting the space required and resizing up to that - // space and formatting. - - const auto needed = _scprintf(pFormat->c_str(), args); - // -1 is the _scprintf error case https://msdn.microsoft.com/en-us/library/t32cf9tb.aspx - if (needed > -1) - { - _formatBuffer.resize(static_cast(needed) + 1); - - const auto written = _vsnprintf_s(_formatBuffer.data(), _formatBuffer.size(), needed, pFormat->c_str(), args); - hr = _Write({ _formatBuffer.data(), gsl::narrow(written) }); - } - else - { - hr = E_INVALIDARG; - } - - va_end(args); - - return hr; -} -CATCH_RETURN(); - // Method Description: // - This method will update the active font on the current device context // Does nothing for vt, the font is handed by the terminal. diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index a6d8e60f2d7..adb04c7567e 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -154,9 +154,19 @@ namespace Microsoft::Console::Render std::optional _newBottomLineBG{ std::nullopt }; [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept; - [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept; [[nodiscard]] HRESULT _Flush() noexcept; + template + [[nodiscard]] HRESULT _WriteFormatted(S&& format, Args&&... args) + try + { + //auto s = fmt::format(std::forward(format), std::forward(args)...); + //return _Write(s); + fmt::format_to(std::back_inserter(_buffer), std::forward(format), std::forward(args)...); + return S_OK; + } + CATCH_RETURN() + void _OrRect(_Inout_ SMALL_RECT* const pRectExisting, const SMALL_RECT* const pRectToOr) const; bool _AllIsInvalid() const; diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index de34fdaf84f..e552b880944 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -1848,7 +1848,7 @@ void StateMachine::ProcessString(const std::wstring_view string) if (_processingIndividually) { // If we're processing characters individually, send it to the state machine. - ProcessCharacter(string.at(current)); + ProcessCharacter(til::at(string, current)); ++current; if (_state == VTStates::Ground) // Then check if we're back at ground. If we are, the next character (pwchCurr) { // is the start of the next run of characters that might be printable. @@ -1858,7 +1858,7 @@ void StateMachine::ProcessString(const std::wstring_view string) } else { - if (_isActionableFromGround(string.at(current))) // If the current char is the start of an escape sequence, or should be executed in ground state... + if (_isActionableFromGround(til::at(string, current))) // If the current char is the start of an escape sequence, or should be executed in ground state... { if (!_run.empty()) { From 1043b61bf0cdb7ffb55caf5dee0f85a4bc62883f Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Sun, 20 Jun 2021 21:34:21 +0800 Subject: [PATCH 10/12] Yeah this also works --- src/renderer/vt/vtrenderer.hpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index adb04c7567e..f7b0a788b66 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -160,10 +160,9 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _WriteFormatted(S&& format, Args&&... args) try { - //auto s = fmt::format(std::forward(format), std::forward(args)...); - //return _Write(s); - fmt::format_to(std::back_inserter(_buffer), std::forward(format), std::forward(args)...); - return S_OK; + fmt::basic_memory_buffer buf; + fmt::format_to(std::back_inserter(buf), std::forward(format), std::forward(args)...); + return _Write({ buf.data(), buf.size() }); } CATCH_RETURN() From c8b804678b701c6088e8b758c7ad4ca32de4809c Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Sun, 20 Jun 2021 21:37:07 +0800 Subject: [PATCH 11/12] Fix build --- src/host/ut_host/VtRendererTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 45e0fa8507d..b1785380587 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -1577,11 +1577,11 @@ void VtRendererTest::FormattedString() Log::Comment(L"2.) Write the same thing again, should be fine."); qExpectedInput.push_back("\x1b[12m"); - VERIFY_SUCCEEDED(engine->_WriteFormatted(&format, value)); + VERIFY_SUCCEEDED(engine->_WriteFormatted(format, value)); Log::Comment(L"3.) Now write something huge. Should resize itself and still be fine."); static const auto bigFormat = FMT_COMPILE("\x1b[28;3;{};{};{}m"); const auto bigValue = 500; qExpectedInput.push_back("\x1b[28;3;500;500;500m"); - VERIFY_SUCCEEDED(engine->_WriteFormatted(&bigFormat, bigValue, bigValue, bigValue)); + VERIFY_SUCCEEDED(engine->_WriteFormatted(bigFormat, bigValue, bigValue, bigValue)); } From 787900b2c35534103c96dad3ed9a114fbe431cd6 Mon Sep 17 00:00:00 2001 From: Chester Liu Date: Tue, 22 Jun 2021 08:34:39 +0800 Subject: [PATCH 12/12] Restore part of it --- src/renderer/vt/VtSequences.cpp | 7 ++++++- src/terminal/parser/stateMachine.cpp | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 26415e41088..efd16e2cb45 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -168,7 +168,12 @@ using namespace Microsoft::Console::Render; // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::_CursorPosition(const COORD coord) noexcept { - return _WriteFormatted(FMT_COMPILE("\x1b[{};{}H"), coord.Y + 1, coord.X + 1); + // VT coords start at 1,1 + COORD coordVt = coord; + coordVt.X++; + coordVt.Y++; + + return _WriteFormatted(FMT_COMPILE("\x1b[{};{}H"), coordVt.Y, coordVt.X); } // Method Description: diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index e552b880944..de34fdaf84f 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -1848,7 +1848,7 @@ void StateMachine::ProcessString(const std::wstring_view string) if (_processingIndividually) { // If we're processing characters individually, send it to the state machine. - ProcessCharacter(til::at(string, current)); + ProcessCharacter(string.at(current)); ++current; if (_state == VTStates::Ground) // Then check if we're back at ground. If we are, the next character (pwchCurr) { // is the start of the next run of characters that might be printable. @@ -1858,7 +1858,7 @@ void StateMachine::ProcessString(const std::wstring_view string) } else { - if (_isActionableFromGround(til::at(string, current))) // If the current char is the start of an escape sequence, or should be executed in ground state... + if (_isActionableFromGround(string.at(current))) // If the current char is the start of an escape sequence, or should be executed in ground state... { if (!_run.empty()) {