From 003cbe7c29eb131b9cfa774295aa16b27a3b6e5d Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 17 May 2022 00:16:57 +0200 Subject: [PATCH 1/7] Reduce integer type casts in VtEngine (#13097) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is one of the more difficult rewrites that were necessary as part of #4015, but still simple enough that it can be done as a separate commit. The search for the `lastNonSpace` was replaced with a simpler `std::string_view::find_last_not_of`. ## Validation Steps Performed ConPTY appears to work ✅ --- src/renderer/vt/paint.cpp | 45 +++++++++++++++------------------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 4483c03c775..1e026e356af 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -348,17 +348,17 @@ using namespace Microsoft::Console::Types; _bufferLine.clear(); _bufferLine.reserve(clusters.size()); - short totalWidth = 0; + size_t totalWidth = 0; for (const auto& cluster : clusters) { _bufferLine.append(cluster.GetText()); - RETURN_IF_FAILED(ShortAdd(totalWidth, gsl::narrow(cluster.GetColumns()), &totalWidth)); + totalWidth += cluster.GetColumns(); } RETURN_IF_FAILED(VtEngine::_WriteTerminalAscii(_bufferLine)); // Update our internal tracker of the cursor's position - _lastText.X += totalWidth; + _lastText.X += gsl::narrow(totalWidth); return S_OK; } @@ -384,38 +384,29 @@ using namespace Microsoft::Console::Types; _bufferLine.clear(); _bufferLine.reserve(clusters.size()); - short totalWidth = 0; + size_t totalWidth = 0; for (const auto& cluster : clusters) { _bufferLine.append(cluster.GetText()); - RETURN_IF_FAILED(ShortAdd(totalWidth, static_cast(cluster.GetColumns()), &totalWidth)); + totalWidth += cluster.GetColumns(); } const auto cchLine = _bufferLine.size(); - auto foundNonspace = false; - size_t lastNonSpace = 0; - for (size_t i = 0; i < cchLine; i++) - { - if (_bufferLine.at(i) != L'\x20') - { - lastNonSpace = i; - foundNonspace = true; - } - } + const auto spaceIndex = _bufferLine.find_last_not_of(L' '); + const auto foundNonspace = spaceIndex != decltype(_bufferLine)::npos; + const auto nonSpaceLength = foundNonspace ? spaceIndex + 1 : 0; + // Examples: // - " ": - // cch = 2, lastNonSpace = 0, foundNonSpace = false - // cch-lastNonSpace = 2 -> good - // cch-lastNonSpace-(0) = 2 -> good + // cch = 2, spaceIndex = 0, foundNonSpace = false + // cch-nonSpaceLength = 2 // - "A " - // cch = 2, lastNonSpace = 0, foundNonSpace = true - // cch-lastNonSpace = 2 -> bad - // cch-lastNonSpace-(1) = 1 -> good + // cch = 2, spaceIndex = 0, foundNonSpace = true + // cch-nonSpaceLength = 1 // - "AA" - // cch = 2, lastNonSpace = 1, foundNonSpace = true - // cch-lastNonSpace = 1 -> bad - // cch-lastNonSpace-(1) = 0 -> good - const auto numSpaces = cchLine - lastNonSpace - (foundNonspace ? 1 : 0); + // cch = 2, spaceIndex = 1, foundNonSpace = true + // cch-nonSpaceLength = 0 + const auto numSpaces = cchLine - nonSpaceLength; // Optimizations: // If there are lots of spaces at the end of the line, we can try to Erase @@ -460,9 +451,7 @@ using namespace Microsoft::Console::Types; const auto removeSpaces = !lineWrapped && (useEraseChar || _clearedAllThisFrame || (_newBottomLine && printingBottomLine && bgMatched)); - const auto cchActual = removeSpaces ? - (cchLine - numSpaces) : - cchLine; + const auto cchActual = removeSpaces ? nonSpaceLength : cchLine; const auto columnsActual = removeSpaces ? (totalWidth - numSpaces) : From 1db47ff8dcb6471f21b83376197d513bd603338a Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 17 May 2022 00:18:13 +0200 Subject: [PATCH 2/7] Simplify Utf16Parser (#13096) This trivial commit simplifies `Utf16Parser::IsLeading/TrailingSurrogate` methods, by replacing `` with a simple, equivalent range check. --- src/types/inc/Utf16Parser.hpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/types/inc/Utf16Parser.hpp b/src/types/inc/Utf16Parser.hpp index f48bdc0cda2..5ce8eba4e55 100644 --- a/src/types/inc/Utf16Parser.hpp +++ b/src/types/inc/Utf16Parser.hpp @@ -15,17 +15,9 @@ Author(s): #pragma once #include -#include -#include class Utf16Parser final { -private: - static constexpr unsigned short IndicatorBitCount = 6; - static constexpr unsigned short WcharShiftAmount = sizeof(wchar_t) * 8 - IndicatorBitCount; - static constexpr std::bitset LeadingSurrogateMask = { 54 }; // 110 110 indicates a leading surrogate - static constexpr std::bitset TrailingSurrogateMask = { 55 }; // 110 111 indicates a trailing surrogate - public: static std::vector> Parse(std::wstring_view wstr); static std::wstring_view ParseNext(std::wstring_view wstr) noexcept; @@ -36,11 +28,9 @@ class Utf16Parser final // - wch - the wchar to check // Return Value: // - true if wch is a leading surrogate, false otherwise - static inline bool IsLeadingSurrogate(const wchar_t wch) noexcept + static constexpr bool IsLeadingSurrogate(const wchar_t wch) noexcept { - const wchar_t bits = wch >> WcharShiftAmount; - const std::bitset possBits = { bits }; - return (possBits ^ LeadingSurrogateMask).none(); + return wch >= 0xD800 && wch <= 0xDBFF; } // Routine Description: @@ -49,10 +39,8 @@ class Utf16Parser final // - wch - the wchar to check // Return Value: // - true if wch is a trailing surrogate, false otherwise - static inline bool IsTrailingSurrogate(const wchar_t wch) noexcept + static constexpr bool IsTrailingSurrogate(const wchar_t wch) noexcept { - const wchar_t bits = wch >> WcharShiftAmount; - const std::bitset possBits = { bits }; - return (possBits ^ TrailingSurrogateMask).none(); + return wch >= 0xDC00 && wch <= 0xDFFF; } }; From df627c26f1419f0dd7805288c87f917cc33a0515 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 17 May 2022 01:28:32 +0200 Subject: [PATCH 3/7] Various improvements for til::hash/point/size/rect (#13093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit includes various minor improvements to til::hash/point/size/rect which accumulated while working on #4015. * Allow xvalue containers and non-`size_t` indices in `til::at`. * `til::as_unsigned` can be used to reinterpret a potentially signed integer as a unsigned one. This can potentially enable some optimizations as no sign extension is needed anymore. `til::hash` can make use of this to drop about 20% of the hashing of signed integers <= 32 bit. On x86 this translates to a `mov` (virtually no latency) or no instructions at all, instead of requiring a `movsx` (some latency) for sign extension. * `til::point` operators that prefer mutability. This is a opinionated change, but it follows the STL style beter and generates less assembly. * Simpler `rect` scale_up/down and `size` divide_ceil. `scale_up` will not depend on the operator header anymore. `scale_down` / `divide_ceil` can be implemented without checked numerics, so I did. It also follows the related GdiEngine code better now, which makes me confident that we can replace GdiEngine's code with this. * Removal of rect-size-shift operators. They were only used in DxEngine and confusing as they weren't commutative. Adding and then subtracting a size from a rect (and vice versa) didn't do what you'd intuitively think it'd do. The code was replaced with addition and clamps in DxEngine. * Various unsafe `as_` casts for point/size/rect. This will aid the migration in #4015. ## Validation Steps Performed * Vertical scrolling works in `DxEngine` ✅ --- src/inc/til/at.h | 28 +-- src/inc/til/bit.h | 10 + src/inc/til/hash.h | 8 +- src/inc/til/point.h | 64 +++--- src/inc/til/rect.h | 314 ++++++++++------------------ src/inc/til/size.h | 82 ++++---- src/renderer/dx/DxRenderer.cpp | 5 +- src/terminal/adapter/FontBuffer.cpp | 2 +- src/terminal/adapter/FontBuffer.hpp | 2 +- src/til/ut_til/PointTests.cpp | 7 + src/til/ut_til/RectangleTests.cpp | 173 ++------------- src/til/ut_til/SizeTests.cpp | 35 +--- 12 files changed, 264 insertions(+), 466 deletions(-) diff --git a/src/inc/til/at.h b/src/inc/til/at.h index 592129634ae..28a715be86b 100644 --- a/src/inc/til/at.h +++ b/src/inc/til/at.h @@ -34,24 +34,24 @@ namespace til // gsl::at will do the check again. As will .at(). And using [] will have a warning in audit. // This template is explicitly disabled if T is of type gsl::span, as it would interfere with // the overload below. - template::value, int> = 0> - constexpr auto at(T& cont, const size_t i) -> decltype(cont[cont.size()]) + template + constexpr auto at(T&& cont, const I i) noexcept -> decltype(auto) { -#pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions -#pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. -#pragma warning(suppress : 26445) // Suppress lifetime check for a reference to gsl::span or std::string_view - return cont[i]; - } - #ifdef GSL_SPAN_H - // This is an overload of til::at for span that access its backing buffer directly (UNCHECKED) - template - constexpr auto at(gsl::span span, const std::ptrdiff_t i) -> decltype(span[span.size()]) - { + if constexpr (details::is_span::value) + { #pragma warning(suppress : 26481) // Suppress bounds.1 check for doing pointer arithmetic #pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions #pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. - return span.data()[i]; - } + return cont.data()[i]; + } + else #endif + { +#pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions +#pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. +#pragma warning(suppress : 26445) // Suppress lifetime check for a reference to gsl::span or std::string_view + return cont[i]; + } + } } diff --git a/src/inc/til/bit.h b/src/inc/til/bit.h index dc15d4bce44..a3ce7775a3b 100644 --- a/src/inc/til/bit.h +++ b/src/inc/til/bit.h @@ -5,10 +5,20 @@ namespace til { + // bit_cast is a backport of the STL's std::bit_cast to C++17. template, std::is_trivially_copyable, std::is_trivially_copyable>, int> = 0> [[nodiscard]] constexpr To bit_cast(const From& _Val) noexcept { // TODO: Replace til::bit_cast and __builtin_bit_cast with std::bit_cast return __builtin_bit_cast(To, _Val); } + + // When you cast a signed integer to an unsigned one, the compiler will use "sign extension" + // so that -1 translates to all bits being set, no matter the size of the target type. + // Sometimes you don't need or want that, which is when you can use this function. + template + [[nodiscard]] constexpr auto as_unsigned(const T& v) noexcept + { + return bit_cast>(v); + } } diff --git a/src/inc/til/hash.h b/src/inc/til/hash.h index c30a30e4ade..a96f26dcf1b 100644 --- a/src/inc/til/hash.h +++ b/src/inc/til/hash.h @@ -3,6 +3,8 @@ #pragma once +#include "bit.h" + namespace til { template @@ -129,7 +131,11 @@ namespace til { // This runs murmurhash3's finalizer (fmix32/fmix64) on a single integer. // It's fast, public domain and produces good results. - auto h = static_cast(v); + // + // Using til::as_unsigned here allows the compiler to drop the first + // `>> 33` mix for all Ts which are >= 32 bits. + // The existence of sign extension shouldn't change hash quality. + size_t h = til::as_unsigned(v); if constexpr (sizeof(size_t) == 4) { h ^= h >> 16; diff --git a/src/inc/til/point.h b/src/inc/til/point.h index f8eea99ccef..ea0d9a4416d 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -65,6 +65,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return __builtin_memcmp(this, &rhs, sizeof(rhs)) != 0; } + constexpr explicit operator bool() const noexcept + { + return (x > 0) & (y > 0); + } + constexpr bool operator<(const point other) const noexcept { return y < other.y || (y == other.y && x < other.x); @@ -87,62 +92,61 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr point operator+(const point other) const { - return point{ - details::extract(::base::CheckAdd(x, other.x)), - details::extract(::base::CheckAdd(y, other.y)), - }; + auto copy = *this; + copy += other; + return copy; } constexpr point& operator+=(const point other) { - *this = *this + other; + x = details::extract(::base::CheckAdd(x, other.x)); + y = details::extract(::base::CheckAdd(y, other.y)); return *this; } constexpr point operator-(const point other) const { - return point{ - details::extract(::base::CheckSub(x, other.x)), - details::extract(::base::CheckSub(y, other.y)), - }; + auto copy = *this; + copy -= other; + return copy; } constexpr point& operator-=(const point other) { - *this = *this - other; + x = details::extract(::base::CheckSub(x, other.x)); + y = details::extract(::base::CheckSub(y, other.y)); return *this; } constexpr point operator*(const point other) const { - return point{ - details::extract(::base::CheckMul(x, other.x)), - details::extract(::base::CheckMul(y, other.y)), - }; + auto copy = *this; + copy *= other; + return copy; } constexpr point& operator*=(const point other) { - *this = *this * other; + x = details::extract(::base::CheckMul(x, other.x)); + y = details::extract(::base::CheckMul(y, other.y)); return *this; } constexpr point operator/(const point other) const { - return point{ - details::extract(::base::CheckDiv(x, other.x)), - details::extract(::base::CheckDiv(y, other.y)), - }; + auto copy = *this; + copy /= other; + return copy; } constexpr point& operator/=(const point other) { - *this = *this / other; + x = details::extract(::base::CheckDiv(x, other.x)); + y = details::extract(::base::CheckDiv(y, other.y)); return *this; } - template>> - constexpr point operator*(const T scale) const + constexpr point operator*(const til::CoordType scale) const { return point{ details::extract(::base::CheckMul(x, scale)), @@ -150,8 +154,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" }; } - template>> - constexpr point operator/(const T scale) const + constexpr point operator/(const til::CoordType scale) const { return point{ details::extract(::base::CheckDiv(x, scale)), @@ -193,6 +196,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return { x, y }; } + + // til::point and POINT have the exact same layout at the time of writing, + // so this function lets you unsafely "view" this point as a POINT + // if you need to pass it to a Win32 function. + // + // Use as_win32_point() as sparingly as possible because it'll be a pain to hack + // it out of this code base once til::point and POINT aren't the same anymore. + // Prefer casting to POINT and back to til::point instead if possible. + POINT* as_win32_point() noexcept + { +#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1). + return std::launder(reinterpret_cast(this)); + } #endif #ifdef DCOMMON_H_INCLUDED diff --git a/src/inc/til/rect.h b/src/inc/til/rect.h index fb51d1877f7..4d1fca122e9 100644 --- a/src/inc/til/rect.h +++ b/src/inc/til/rect.h @@ -47,6 +47,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return __builtin_memcmp(this, &rhs, sizeof(rhs)) != 0; } + + explicit constexpr operator bool() const noexcept + { + return (left >= 0) & (top >= 0) & + (right >= left) & (bottom >= top); + } }; constexpr inclusive_rect wrap_small_rect(const SMALL_RECT& rect) noexcept @@ -416,22 +422,22 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" const rect l{ left, intersect.top, intersect.left, intersect.bottom }; const rect r{ intersect.right, intersect.top, right, intersect.bottom }; - if (!t.empty()) + if (t) { result.push_back(t); } - if (!b.empty()) + if (b) { result.push_back(b); } - if (!l.empty()) + if (l) { result.push_back(l); } - if (!r.empty()) + if (r) { result.push_back(r); } @@ -477,210 +483,43 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" #pragma endregion #pragma region RECTANGLE VS SIZE - // ADD will grow the total area of the rect. The sign is the direction to grow. - constexpr rect operator+(const size size) const - { - // Fetch the pieces of the rect. - auto l = left; - auto r = right; - auto t = top; - auto b = bottom; - - // Fetch the scale factors we're using. - const auto width = size.width; - const auto height = size.height; - - // Since this is the add operation versus a size, the result - // should grow the total rect area. - // The sign determines which edge of the rect moves. - // We use the magnitude as how far to move. - if (width > 0) - { - // Adding the positive makes the rect "grow" - // because right stretches outward (to the right). - // - // Example with adding width 3... - // |-- x = origin - // V - // x---------| x------------| - // | | | | - // | | | | - // |---------| |------------| - // BEFORE AFTER - r = details::extract(::base::CheckAdd(r, width)); - } - else - { - // Adding the negative makes the rect "grow" - // because left stretches outward (to the left). - // - // Example with adding width -3... - // |-- x = origin - // V - // x---------| |--x---------| - // | | | | - // | | | | - // |---------| |------------| - // BEFORE AFTER - l = details::extract(::base::CheckAdd(l, width)); - } - - if (height > 0) - { - // Adding the positive makes the rect "grow" - // because bottom stretches outward (to the down). - // - // Example with adding height 2... - // |-- x = origin - // V - // x---------| x---------| - // | | | | - // | | | | - // |---------| | | - // | | - // |---------| - // BEFORE AFTER - b = details::extract(::base::CheckAdd(b, height)); - } - else - { - // Adding the negative makes the rect "grow" - // because top stretches outward (to the up). - // - // Example with adding height -2... - // |-- x = origin - // | - // | |---------| - // V | | - // x---------| x | - // | | | | - // | | | | - // |---------| |---------| - // BEFORE AFTER - t = details::extract(::base::CheckAdd(t, height)); - } - - return rect{ point{ l, t }, point{ r, b } }; - } - - constexpr rect& operator+=(const size size) - { - *this = *this + size; - return *this; - } - - // SUB will shrink the total area of the rect. The sign is the direction to shrink. - constexpr rect operator-(const size size) const - { - // Fetch the pieces of the rect. - auto l = left; - auto r = right; - auto t = top; - auto b = bottom; - - // Fetch the scale factors we're using. - const auto width = size.width; - const auto height = size.height; - - // Since this is the subtract operation versus a size, the result - // should shrink the total rect area. - // The sign determines which edge of the rect moves. - // We use the magnitude as how far to move. - if (width > 0) - { - // Subtracting the positive makes the rect "shrink" - // because right pulls inward (to the left). - // - // Example with subtracting width 3... - // |-- x = origin - // V - // x---------| x------| - // | | | | - // | | | | - // |---------| |------| - // BEFORE AFTER - r = details::extract(::base::CheckSub(r, width)); - } - else - { - // Subtracting the negative makes the rect "shrink" - // because left pulls inward (to the right). - // - // Example with subtracting width -3... - // |-- x = origin - // V - // x---------| x |------| - // | | | | - // | | | | - // |---------| |------| - // BEFORE AFTER - l = details::extract(::base::CheckSub(l, width)); - } - - if (height > 0) - { - // Subtracting the positive makes the rect "shrink" - // because bottom pulls inward (to the up). - // - // Example with subtracting height 2... - // |-- x = origin - // V - // x---------| x---------| - // | | |---------| - // | | - // |---------| - // BEFORE AFTER - b = details::extract(::base::CheckSub(b, height)); - } - else - { - // Subtracting the positive makes the rect "shrink" - // because top pulls inward (to the down). - // - // Example with subtracting height -2... - // |-- x = origin - // V - // x---------| x - // | | - // | | |---------| - // |---------| |---------| - // BEFORE AFTER - t = details::extract(::base::CheckSub(t, height)); - } - - return rect{ point{ l, t }, point{ r, b } }; - } - - constexpr rect& operator-=(const size size) - { - *this = *this - size; - return *this; - } // scale_up will scale the entire rect up by the size factor - // This includes moving the origin. constexpr rect scale_up(const size size) const { - const auto topLeft = point{ left, top } * size; - const auto bottomRight = point{ right, bottom } * size; - return rect{ topLeft, bottomRight }; + return rect{ + details::extract(::base::CheckMul(left, size.width)), + details::extract(::base::CheckMul(top, size.height)), + details::extract(::base::CheckMul(right, size.width)), + details::extract(::base::CheckMul(bottom, size.height)), + }; } - // scale_down will scale the entire rect down by the size factor, - // but rounds the bottom-right corner out. - // This includes moving the origin. + // scale_down will scale the entire rect down by the size factor. + // The top/left corner is rounded down (floor) and + // the bottom/right corner is rounded up (ceil). constexpr rect scale_down(const size size) const { - auto topLeft = point{ left, top }; - auto bottomRight = point{ right, bottom }; - topLeft = topLeft / size; - - // Move bottom right point into a size - // Use size specialization of divide_ceil to round up against the size given. - // Add leading addition to point to convert it back into a point. - bottomRight = point{} + til::size{ right, bottom }.divide_ceil(size); + // The integer ceil division `((a - 1) / b) + 1` only works for numbers >0. + // Support for negative numbers wasn't deemed useful at this point. + if ((left < 0) | (top < 0) | (right < 0) | (bottom < 0) | (size.width <= 0) | (size.height <= 0)) + { + throw std::invalid_argument{ "invalid til::rect::scale_down" }; + } - return rect{ topLeft, bottomRight }; + // Imagine a terminal of 120x30 "cells" with each cell being + // 5x10 pixels large. The terminal is therefore 600x300 pixels. + // Given a rectangle in pixel coordinates, what's the rectangle in cell coordinates? + // Clearly this requires us to floor() top/left and ceil() bottom/right to cover all pixels. + // And thus: + // {17, 24, 31, 38}.scale_down({5, 10}) == {3, 2, 7, 4} + // {3, 2, 7, 4}.scale_up({5, 10}) == {15, 20, 35, 40} + return rect{ + left / size.width, + top / size.height, + right != 0 ? (right - 1) / size.width + 1 : 0, + bottom != 0 ? (bottom - 1) / size.height + 1 : 0, + }; } #pragma endregion @@ -738,7 +577,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr size size() const noexcept { - return til::size{ width(), height() }; + return { width(), height() }; } constexpr bool empty() const noexcept @@ -841,6 +680,32 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return { left, top, right, bottom }; } + + // til::rect and RECT have the exact same layout at the time of writing, + // so this function lets you unsafely "view" this rect as a RECT + // if you need to pass it to a Win32 function. + // + // Use as_win32_rect() as sparingly as possible because it'll be a pain to hack + // it out of this code base once til::rect and RECT aren't the same anymore. + // Prefer casting to RECT and back to til::rect instead if possible. + RECT* as_win32_rect() noexcept + { +#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1). + return std::launder(reinterpret_cast(this)); + } + + // til::rect and POINT[2] have the exact same layout at the time of writing, + // so this function lets you unsafely "view" this rect as a POINT[2] array + // if you need to pass it to a Win32 function. + // + // Use as_win32_points() as sparingly as possible because it'll be a pain to hack + // it out of this code base once til::rect and POINT[2] aren't the same anymore. + // Prefer casting to POINT and back to til::rect instead if possible. + POINT* as_win32_points() noexcept + { +#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1). + return std::launder(reinterpret_cast(this)); + } #endif #ifdef DCOMMON_H_INCLUDED @@ -902,11 +767,60 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return wil::str_printf(L"(L:%d, T:%d, R:%d, B:%d) [W:%d, H:%d]", left, top, right, bottom, width(), height()); } }; + + constexpr rect wrap_exclusive_small_rect(const SMALL_RECT& rect) noexcept + { + return { rect.Left, rect.Top, rect.Right, rect.Bottom }; + } + + constexpr SMALL_RECT unwrap_exclusive_small_rect(const rect& rect) + { + return { + gsl::narrow(rect.left), + gsl::narrow(rect.top), + gsl::narrow(rect.right), + gsl::narrow(rect.bottom), + }; + } } #ifdef __WEX_COMMON_H__ namespace WEX::TestExecution { + template<> + class VerifyOutputTraits + { + public: + static WEX::Common::NoThrowString ToString(const til::inclusive_rect& rect) + { + return WEX::Common::NoThrowString().Format(L"(L:%d, T:%d, R:%d, B:%d) [W:%d, H:%d]", rect.left, rect.top, rect.right, rect.bottom, rect.right - rect.left, rect.bottom - rect.top); + } + }; + + template<> + class VerifyCompareTraits + { + public: + static bool AreEqual(const til::inclusive_rect& expected, const til::inclusive_rect& actual) noexcept + { + return expected == actual; + } + + static bool AreSame(const til::inclusive_rect& expected, const til::inclusive_rect& actual) noexcept + { + return &expected == &actual; + } + + static bool IsLessThan(const til::inclusive_rect& expectedLess, const til::inclusive_rect& expectedGreater) = delete; + + static bool IsGreaterThan(const til::inclusive_rect& expectedGreater, const til::inclusive_rect& expectedLess) = delete; + + static bool IsNull(const til::inclusive_rect& object) noexcept + { + return object == til::inclusive_rect{}; + } + }; + template<> class VerifyOutputTraits { diff --git a/src/inc/til/size.h b/src/inc/til/size.h index df0b893067e..2b21abd41de 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -9,8 +9,21 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { struct size { - CoordType width = 0; - CoordType height = 0; + // **** TRANSITIONAL **** + // The old COORD type uses uppercase X/Y member names. + // We'll migrate to lowercase width/height in the future. + union + { + CoordType width = 0; + CoordType X; + CoordType cx; + }; + union + { + CoordType height = 0; + CoordType Y; + CoordType cy; + }; constexpr size() noexcept = default; @@ -42,7 +55,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr explicit operator bool() const noexcept { - return width > 0 && height > 0; + return (width > 0) & (height > 0); } constexpr size operator+(const size other) const @@ -80,7 +93,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" template>> constexpr size scale(TilMath math, const T scale) const { - return til::size{ + return { math, width * scale, height * scale, @@ -89,49 +102,17 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr size divide_ceil(const size other) const { - // Divide normally to get the floor. - const size floor = *this / other; - - CoordType adjWidth = 0; - CoordType adjHeight = 0; - - // Check for width remainder, anything not 0. - // If we multiply the floored number with the other, it will equal - // the old width if there was no remainder. - if (other.width * floor.width != width) + // The integer ceil division `((a - 1) / b) + 1` only works for numbers >0. + // Support for negative numbers wasn't deemed useful at this point. + if ((width < 0) | (height < 0) | (other.width <= 0) | (other.height <= 0)) { - // If there was any remainder, - // Grow the magnitude by 1 in the - // direction of the sign. - if (floor.width >= 0) - { - ++adjWidth; - } - else - { - --adjWidth; - } + throw std::invalid_argument{ "invalid til::size::divide_ceil" }; } - // Check for height remainder, anything not 0. - // If we multiply the floored number with the other, it will equal - // the old width if there was no remainder. - if (other.height * floor.height != height) - { - // If there was any remainder, - // Grow the magnitude by 1 in the - // direction of the sign. - if (height >= 0) - { - ++adjHeight; - } - else - { - --adjHeight; - } - } - - return floor + size{ adjWidth, adjHeight }; + return { + width != 0 ? (width - 1) / other.width + 1 : 0, + height != 0 ? (height - 1) / other.height + 1 : 0, + }; } template @@ -174,6 +155,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return { width, height }; } + + // til::size and SIZE have the exact same layout at the time of writing, + // so this function lets you unsafely "view" this size as a SIZE + // if you need to pass it to a Win32 function. + // + // Use as_win32_size() as sparingly as possible because it'll be a pain to hack + // it out of this code base once til::size and SIZE aren't the same anymore. + // Prefer casting to SIZE and back to til::size instead if possible. + SIZE* as_win32_size() noexcept + { +#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1). + return std::launder(reinterpret_cast(this)); + } #endif #ifdef DCOMMON_H_INCLUDED diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 37febe8dba5..dec159cbe5e 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1395,7 +1395,10 @@ try til::rect scrollArea{ _invalidMap.size() * _fontRenderData->GlyphCell() }; // Reduce the size of the rectangle by the scroll. - scrollArea -= til::size{} - scrollPixels; + scrollArea.left = std::clamp(scrollArea.left + scrollPixels.x, scrollArea.left, scrollArea.right); + scrollArea.top = std::clamp(scrollArea.top + scrollPixels.y, scrollArea.top, scrollArea.bottom); + scrollArea.right = std::clamp(scrollArea.right + scrollPixels.x, scrollArea.left, scrollArea.right); + scrollArea.bottom = std::clamp(scrollArea.bottom + scrollPixels.y, scrollArea.top, scrollArea.bottom); // Assign the area to the present storage _presentScroll = scrollArea.to_win32_rect(); diff --git a/src/terminal/adapter/FontBuffer.cpp b/src/terminal/adapter/FontBuffer.cpp index 02a115a1a4e..98d13d78319 100644 --- a/src/terminal/adapter/FontBuffer.cpp +++ b/src/terminal/adapter/FontBuffer.cpp @@ -478,7 +478,7 @@ std::tuple FontBuffer::_calculateDimensions() const } } -void FontBuffer::_packAndCenterBitPatterns() +void FontBuffer::_packAndCenterBitPatterns() noexcept { // If this is a text font, we'll clip the bits up to the text width and // center them within the full cell width. For a full cell font we'll just diff --git a/src/terminal/adapter/FontBuffer.hpp b/src/terminal/adapter/FontBuffer.hpp index 275dab0ad04..4b5ddbef220 100644 --- a/src/terminal/adapter/FontBuffer.hpp +++ b/src/terminal/adapter/FontBuffer.hpp @@ -48,7 +48,7 @@ namespace Microsoft::Console::VirtualTerminal void _endOfCharacter(); std::tuple _calculateDimensions() const; - void _packAndCenterBitPatterns(); + void _packAndCenterBitPatterns() noexcept; void _fillUnusedCharacters(); std::array _generateErrorGlyph(); diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index 5d4cd2cee00..fca594abf13 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -9,6 +9,13 @@ using namespace WEX::Common; using namespace WEX::Logging; using namespace WEX::TestExecution; +// Ensure the "safety" of til::point::as_win32_point +static_assert( + sizeof(til::point) == sizeof(POINT) && + alignof(til::point) == alignof(POINT) && + offsetof(til::point, x) == offsetof(POINT, x) && + offsetof(til::point, y) == offsetof(POINT, y)); + class PointTests { TEST_CLASS(PointTests); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 5611dc8a4cb..b59862db5a6 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -7,6 +7,23 @@ using namespace WEX::Common; using namespace WEX::Logging; using namespace WEX::TestExecution; +// Ensure the "safety" of til::rect::as_win32_rect +static_assert( + sizeof(til::rect) == sizeof(RECT) && + alignof(til::rect) == alignof(RECT) && + offsetof(til::rect, left) == offsetof(RECT, left) && + offsetof(til::rect, top) == offsetof(RECT, top) && + offsetof(til::rect, right) == offsetof(RECT, right) && + offsetof(til::rect, bottom) == offsetof(RECT, bottom)); +// Ensure the "safety" of til::rect::as_win32_points +static_assert( + sizeof(til::rect) == 2 * sizeof(POINT) && + alignof(til::rect) == alignof(POINT) && + offsetof(til::rect, left) == offsetof(POINT, x) && + offsetof(til::rect, top) == offsetof(POINT, y) && + offsetof(til::rect, right) == offsetof(POINT, x) + sizeof(POINT) && + offsetof(til::rect, bottom) == offsetof(POINT, y) + sizeof(POINT)); + class RectangleTests { TEST_CLASS(RectangleTests); @@ -512,162 +529,6 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, start); } - TEST_METHOD(AdditionSize) - { - const til::rect start{ 10, 20, 30, 40 }; - - Log::Comment(L"Add size to bottom and right"); - { - const til::size scale{ 3, 7 }; - const til::rect expected{ 10, 20, 33, 47 }; - const auto actual = start + scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to top and left"); - { - const til::size scale{ -3, -7 }; - const til::rect expected{ 7, 13, 30, 40 }; - const auto actual = start + scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to bottom and left"); - { - const til::size scale{ -3, 7 }; - const til::rect expected{ 7, 20, 30, 47 }; - const auto actual = start + scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to top and right"); - { - const til::size scale{ 3, -7 }; - const til::rect expected{ 10, 13, 33, 40 }; - const auto actual = start + scale; - VERIFY_ARE_EQUAL(expected, actual); - } - } - - TEST_METHOD(AdditionSizeInplace) - { - const til::rect start{ 10, 20, 30, 40 }; - - Log::Comment(L"Add size to bottom and right"); - { - auto actual = start; - const til::size scale{ 3, 7 }; - const til::rect expected{ 10, 20, 33, 47 }; - actual += scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to top and left"); - { - auto actual = start; - const til::size scale{ -3, -7 }; - const til::rect expected{ 7, 13, 30, 40 }; - actual += scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to bottom and left"); - { - auto actual = start; - const til::size scale{ -3, 7 }; - const til::rect expected{ 7, 20, 30, 47 }; - actual += scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Add size to top and right"); - { - auto actual = start; - const til::size scale{ 3, -7 }; - const til::rect expected{ 10, 13, 33, 40 }; - actual += scale; - VERIFY_ARE_EQUAL(expected, actual); - } - } - - TEST_METHOD(SubtractionSize) - { - const til::rect start{ 10, 20, 30, 40 }; - - Log::Comment(L"Subtract size from bottom and right"); - { - const til::size scale{ 3, 7 }; - const til::rect expected{ 10, 20, 27, 33 }; - const auto actual = start - scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from top and left"); - { - const til::size scale{ -3, -7 }; - const til::rect expected{ 13, 27, 30, 40 }; - const auto actual = start - scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from bottom and left"); - { - const til::size scale{ -3, 7 }; - const til::rect expected{ 13, 20, 30, 33 }; - const auto actual = start - scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from top and right"); - { - const til::size scale{ 3, -6 }; - const til::rect expected{ 10, 26, 27, 40 }; - const auto actual = start - scale; - VERIFY_ARE_EQUAL(expected, actual); - } - } - - TEST_METHOD(SubtractionSizeInplace) - { - const til::rect start{ 10, 20, 30, 40 }; - - Log::Comment(L"Subtract size from bottom and right"); - { - auto actual = start; - const til::size scale{ 3, 7 }; - const til::rect expected{ 10, 20, 27, 33 }; - actual -= scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from top and left"); - { - auto actual = start; - const til::size scale{ -3, -7 }; - const til::rect expected{ 13, 27, 30, 40 }; - actual -= scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from bottom and left"); - { - auto actual = start; - const til::size scale{ -3, 7 }; - const til::rect expected{ 13, 20, 30, 33 }; - actual -= scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"Subtract size from top and right"); - { - auto actual = start; - const til::size scale{ 3, -6 }; - const til::rect expected{ 10, 26, 27, 40 }; - actual -= scale; - VERIFY_ARE_EQUAL(expected, actual); - } - } - TEST_METHOD(ScaleUpSize) { const til::rect start{ 10, 20, 30, 40 }; diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index a5b18d2e2c0..34122c69bc9 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -9,6 +9,13 @@ using namespace WEX::Common; using namespace WEX::Logging; using namespace WEX::TestExecution; +// Ensure the "safety" of til::point::as_win32_size +static_assert( + sizeof(til::size) == sizeof(SIZE) && + alignof(til::size) == alignof(SIZE) && + offsetof(til::size, width) == offsetof(SIZE, cx) && + offsetof(til::size, height) == offsetof(SIZE, cy)); + class SizeTests { TEST_CLASS(SizeTests); @@ -44,26 +51,6 @@ class SizeTests VERIFY_ARE_EQUAL(height, sz.height); } - TEST_METHOD(MixedRawTypeConstruct) - { - const auto a = -5; - const auto b = -10; - - Log::Comment(L"Case 1: til::CoordType/int"); - { - const til::size sz{ a, b }; - VERIFY_ARE_EQUAL(a, sz.width); - VERIFY_ARE_EQUAL(b, sz.height); - } - - Log::Comment(L"Case 2: int/til::CoordType"); - { - const til::size sz{ b, a }; - VERIFY_ARE_EQUAL(b, sz.width); - VERIFY_ARE_EQUAL(a, sz.height); - } - } - TEST_METHOD(CoordConstruct) { COORD coord{ -5, 10 }; @@ -371,11 +358,11 @@ class SizeTests const til::size sz{ -10, -5 }; const til::size divisor{ 3, 2 }; - // -10 / 3 is -3.333, rounded up is -4. - // -5 / 2 is -2.5, rounded up is -3. - const til::size expected{ -4, -3 }; + auto fn = [&]() { + sz.divide_ceil(divisor); + }; - VERIFY_ARE_EQUAL(expected, sz.divide_ceil(divisor)); + VERIFY_THROWS(fn(), std::invalid_argument); } } From b851b0d3f45d885484f4e0fe8a102abd022bc79e Mon Sep 17 00:00:00 2001 From: Sergey Semushin Date: Tue, 17 May 2022 02:30:05 +0300 Subject: [PATCH 4/7] Add find item to tab menu (#13055) ## Summary of the Pull Request Add `Find` to tab context menu as describe in issue #5633. ## PR Checklist * [x] Closes #5633 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA ## Detailed Description of the Pull Request / Additional comments Just wanted to solve `Easy Starter` issue, so any corrections/suggestions welcome. There's a couple of points I'm not sure of * Placement of item within menu, currently it's at the end before close tab block. * Should it be named longer, something like `Find in Tab` of just `Find` is fine? * The workaround for focus similar to tab rename is a bit annoying, especially because it required adding a method to `TermControl` but without it find window obviously opens without focus which is bad. ## Validation Steps Performed Open menu, press menu item try to find things via opened find dialog. --- .../Resources/en-US/Resources.resw | 3 +++ src/cascadia/TerminalApp/TabManagement.cpp | 10 ++++++++++ src/cascadia/TerminalApp/TerminalTab.cpp | 20 ++++++++++++++++++- src/cascadia/TerminalApp/TerminalTab.h | 1 + src/cascadia/TerminalControl/TermControl.cpp | 14 +++++++++++++ src/cascadia/TerminalControl/TermControl.h | 2 ++ src/cascadia/TerminalControl/TermControl.idl | 1 + 7 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index b2133af1816..a0a653927e3 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -709,6 +709,9 @@ Successfully exported terminal content + + Find + Plain Text diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index d744cb09fd2..47cfa70d7b2 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -209,6 +209,16 @@ namespace winrt::TerminalApp::implementation } }); + newTabImpl->FindRequested([weakTab, weakThis{ get_weak() }]() { + auto page{ weakThis.get() }; + auto tab{ weakTab.get() }; + + if (page && tab) + { + page->_Find(); + } + }); + auto tabViewItem = newTabImpl->TabViewItem(); _tabView.TabItems().Append(tabViewItem); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index c5c6cd2f5b2..64cbd16dc4f 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -1273,6 +1273,23 @@ namespace winrt::TerminalApp::implementation exportTabMenuItem.Icon(exportTabSymbol); } + Controls::MenuFlyoutItem findMenuItem; + { + // "Split Tab" + Controls::FontIcon findSymbol; + findSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); + findSymbol.Glyph(L"\xF78B"); // SearchMedium + + findMenuItem.Click([weakThis](auto&&, auto&&) { + if (auto tab{ weakThis.get() }) + { + tab->_FindRequestedHandlers(); + } + }); + findMenuItem.Text(RS_(L"FindText")); + findMenuItem.Icon(findSymbol); + } + // Build the menu Controls::MenuFlyout contextMenuFlyout; Controls::MenuFlyoutSeparator menuSeparator; @@ -1281,6 +1298,7 @@ namespace winrt::TerminalApp::implementation contextMenuFlyout.Items().Append(duplicateTabMenuItem); contextMenuFlyout.Items().Append(splitTabMenuItem); contextMenuFlyout.Items().Append(exportTabMenuItem); + contextMenuFlyout.Items().Append(findMenuItem); contextMenuFlyout.Items().Append(menuSeparator); // GH#5750 - When the context menu is dismissed with ESC, toss the focus @@ -1291,7 +1309,7 @@ namespace winrt::TerminalApp::implementation // GH#10112 - if we're opening the tab renamer, don't // immediately toss focus to the control. We don't want to steal // focus from the tab renamer. - if (!tab->_headerControl.InRename()) + if (!tab->_headerControl.InRename() && !tab->GetActiveTerminalControl().SearchBoxEditInFocus()) { tab->_RequestFocusActiveControlHandlers(); } diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index 09922f2f1ad..f5ea3b0cd41 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -102,6 +102,7 @@ namespace winrt::TerminalApp::implementation WINRT_CALLBACK(TabRaiseVisualBell, winrt::delegate<>); WINRT_CALLBACK(DuplicateRequested, winrt::delegate<>); WINRT_CALLBACK(SplitTabRequested, winrt::delegate<>); + WINRT_CALLBACK(FindRequested, winrt::delegate<>); WINRT_CALLBACK(ExportTabRequested, winrt::delegate<>); TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 3a0f6cba84d..fdb19c31acf 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -197,6 +197,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation } } + // Method Description: + // Find if search box text edit currently is in focus + // Return Value: + // - true, if search box text edit is in focus + bool TermControl::SearchBoxEditInFocus() const + { + if (!_searchBox) + { + return false; + } + + return _searchBox->TextBox().FocusState() == FocusState::Keyboard; + } + // Method Description: // - Search text in text buffer. This is triggered if the user click // search button or press enter. diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index c76c4b58703..656e1f68451 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -90,6 +90,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SearchMatch(const bool goForward); + bool SearchBoxEditInFocus() const; + bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down); bool OnMouseWheel(const Windows::Foundation::Point location, const int32_t delta, const bool leftButtonDown, const bool midButtonDown, const bool rightButtonDown); diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index b235962a73f..8633931c7df 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -62,6 +62,7 @@ namespace Microsoft.Terminal.Control void ScrollViewport(Int32 viewTop); void CreateSearchBoxControl(); + Boolean SearchBoxEditInFocus(); void SearchMatch(Boolean goForward); From b699f9275f9857382ae90ed5a3ab9f0001c9d3da Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 16 May 2022 17:06:05 -0700 Subject: [PATCH 5/7] Add `experimental.useBackgroundImageForWindow` to schema (#13114) Adds the `experimental.useBackgroundImageForWindow` global setting introduced in #12893 to the schema. --- doc/cascadia/profiles.schema.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 9de676a965a..95ce2ff9990 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -1708,6 +1708,11 @@ "description": "Force the terminal to use the legacy input encoding. Certain keys in some applications may stop working when enabling this setting.", "type": "boolean" }, + "experimental.useBackgroundImageForWindow": { + "default": false, + "description": "When set to true, the background image for the currently focused profile is expanded to encompass the entire window, beneath other panes.", + "type": "boolean" + }, "initialCols": { "default": 120, "description": "The number of columns displayed in the window upon first load. If \"launchMode\" is set to \"maximized\" (or \"maximizedFocus\"), this property is ignored.", From fa6b06674734ca822e6c4cc90644d9b3d8c5c95c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 17 May 2022 02:07:48 +0200 Subject: [PATCH 6/7] AtlasEngine: Stop resizing buffers on scroll (#13100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This regressed in ad2358d. We're interested in the size of the viewport only, but it can shift up/down during scrolling. In these situations we shouldn't resize our buffers of course. ## Validation Steps Performed * Scroll * Not setting `ApiInvalidations::Size` ✅ --- src/renderer/atlas/AtlasEngine.api.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 3cfc35c363a..78ba30aaf74 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -174,9 +174,15 @@ constexpr HRESULT vec2_narrow(U x, U y, AtlasEngine::vec2& out) noexcept [[nodiscard]] HRESULT AtlasEngine::UpdateViewport(const SMALL_RECT srNewViewport) noexcept { - _api.cellCount.x = gsl::narrow_cast(srNewViewport.Right - srNewViewport.Left + 1); - _api.cellCount.y = gsl::narrow_cast(srNewViewport.Bottom - srNewViewport.Top + 1); - WI_SetFlag(_api.invalidations, ApiInvalidations::Size); + const u16x2 cellCount{ + gsl::narrow_cast(srNewViewport.Right - srNewViewport.Left + 1), + gsl::narrow_cast(srNewViewport.Bottom - srNewViewport.Top + 1), + }; + if (_api.cellCount != cellCount) + { + _api.cellCount = cellCount; + WI_SetFlag(_api.invalidations, ApiInvalidations::Size); + } return S_OK; } From e1086de5126e84e3fab43cb47f6b07b3cbb521f6 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 17 May 2022 15:56:04 +0100 Subject: [PATCH 7/7] Fix handling of cursor position reports in passthrough mode (#13109) ## Summary of the Pull Request When the conpty passthrough mode is enabled, it often needs to send `DSR-CPR` queries (cursor position reports) to the client terminal to obtain the current cursor position. However, the code that originally handled the responses to these queries got broken by the refactoring of the `ConGetSet` API. This PR is an attempt to correct that regression. ## References The conpty passthrough mode was introduced in PR #11264. The refactoring that broke the cursor position handling was in PR #12703. ## PR Checklist * [x] Closes #13106 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments Prior to the `ConGetSet` refactoring, the code that handled `DSR-CPR` responses (`InteractDispatch::MoveCursor`) would pass the cursor position to `ConGetSet::SetCursorPosition`, which in turn would forward it to the `SetConsoleCursorPositionImpl` API, and from there to the `VtIo` class. After the refactor, all of those intermediate steps were removed - the cursor was simply updated directly in `InteractDispatch::MoveCursor`, and the `VtIo` call was moved from `SetConsoleCursorPositionImpl` to `InteractDispatch` (since that was the only place it was actually required). However, when the conpty passthrough mode was introduced - which happened in parallel - it relied on the `SetConsoleCursorPositionImpl` API being called from `InteractDispatch` in order to handle its own `DSR-CPR` responses, and that's why things stopped working when the two PRs merged. So what I've done now is made `InteractDispatch::MoveCursor` method call `SetConsoleCursorPositionImpl` again (although without the intermediate `ConGetSet` overhead), and moved the `VtIo::SetCursorPosition` call back into `SetConsoleCursorPositionImpl`. This is not ideal, and there are still a bunch of problems with the `DSR-CPR` handling in passthrough mode, but it's at least as good as it was before. ## Validation Steps Performed I've just manually tested various shells with passthrough mode enabled, and confirmed that they're working better now. There are still issues, but nothing that wasn't already a problem in the initial implementation, at least as far as I can tell. --- src/host/getset.cpp | 4 ++++ src/terminal/adapter/InteractDispatch.cpp | 11 +++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/host/getset.cpp b/src/host/getset.cpp index e9057d12693..0b3356242ca 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -719,6 +719,10 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont position.Y < 0)); // clang-format on + // MSFT: 15813316 - Try to use this SetCursorPosition call to inherit the cursor position. + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + RETURN_IF_FAILED(gci.GetVtIo()->SetCursorPosition(position)); + RETURN_IF_NTSTATUS_FAILED(buffer.SetCursorPosition(position, true)); LOG_IF_FAILED(ConsoleImeResizeCompStrView()); diff --git a/src/terminal/adapter/InteractDispatch.cpp b/src/terminal/adapter/InteractDispatch.cpp index 6c205d3508b..8da0f121a54 100644 --- a/src/terminal/adapter/InteractDispatch.cpp +++ b/src/terminal/adapter/InteractDispatch.cpp @@ -150,15 +150,10 @@ bool InteractDispatch::MoveCursor(const VTInt row, const VTInt col) const auto coordCursorShort = til::unwrap_coord(coordCursor); - // MSFT: 15813316 - Try to use this MoveCursor call to inherit the cursor position. - auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - RETURN_IF_FAILED(gci.GetVtIo()->SetCursorPosition(coordCursorShort)); - // Finally, attempt to set the adjusted cursor position back into the console. - auto& cursor = _api.GetTextBuffer().GetCursor(); - cursor.SetPosition(coordCursorShort); - cursor.SetHasMoved(true); - return true; + const auto api = gsl::not_null{ ServiceLocator::LocateGlobals().api }; + auto& info = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer(); + return SUCCEEDED(api->SetConsoleCursorPositionImpl(info, coordCursorShort)); } // Routine Description: