Address feedback, Fix annoyances

src/cascadia/TerminalControl/ControlCore.cpp

@@ -947,12 +947,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // you move its endpoints while it is generating a frame.
         auto lock = _terminal->LockForWriting();
 
-        const short lastVisibleRow = std::max<short>(_terminal->GetViewport().Height() - 1, 0);
-        const short lastVisibleCol = std::max<short>(_terminal->GetViewport().Width() - 1, 0);
-
         til::point terminalPosition{
-            std::clamp<short>(position.narrow_x<short>(), 0, lastVisibleCol),
-            std::clamp<short>(position.narrow_y<short>(), 0, lastVisibleRow)
+            std::clamp(position.x, 0, _terminal->GetViewport().Width() - 1),
+            std::clamp(position.y, 0, _terminal->GetViewport().Height() - 1)
         };
 
         // save location (for rendering) + render

src/cascadia/TerminalControl/TSFInputControl.cpp

@@ -144,7 +144,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // Get the cursor position in text buffer position
         auto cursorArgs = winrt::make_self<CursorPositionEventArgs>();
         _CurrentCursorPositionHandlers(*this, *cursorArgs);
-        const til::point cursorPos{ gsl::narrow_cast<til::CoordType>(cursorArgs->CurrentPosition().X), gsl::narrow_cast<til::CoordType>(cursorArgs->CurrentPosition().Y) };
+        const til::point cursorPos{ til::math::flooring, cursorArgs->CurrentPosition() };
 
         const double actualCanvasWidth{ Canvas().ActualWidth() };
         const double actualTextBlockHeight{ TextBlock().ActualHeight() };

src/cascadia/TerminalControl/TermControl.cpp

@@ -2159,7 +2159,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         // Convert it to pixels
         const auto scale = SwapChainPanel().CompositionScaleX();
-        const til::point relativeToMarginInPixels{ til::math::flooring, relativeToMarginInDIPs.x * scale, relativeToMarginInDIPs.y * scale };
+        const til::point relativeToMarginInPixels{
+            til::math::flooring,
+            relativeToMarginInDIPs.x * scale,
+            relativeToMarginInDIPs.y * scale,
+        };
 
         return relativeToMarginInPixels;
     }
@@ -2597,7 +2601,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 const auto uriText = _core.HoveredUriText();
                 if (!uriText.empty())
                 {
-                    const auto scale = SwapChainPanel().CompositionScaleX();
+                    const auto panel = SwapChainPanel();
+                    const auto scale = panel.CompositionScaleX();
+                    const auto offset = panel.ActualOffset();
 
                     // Update the tooltip with the URI
                     HoveredUri().Text(uriText);
@@ -2618,8 +2624,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     const til::point locationInDIPs{ posInDIPs + marginsInDips };
 
                     // Move the border to the top left corner of the cell
-                    OverlayCanvas().SetLeft(HyperlinkTooltipBorder(), locationInDIPs.x - panel.ActualOffset().x);
-                    OverlayCanvas().SetTop(HyperlinkTooltipBorder(), locationInDIPs.y - panel.ActualOffset().y);
+                    OverlayCanvas().SetLeft(HyperlinkTooltipBorder(), locationInDIPs.x - offset.x);
+                    OverlayCanvas().SetTop(HyperlinkTooltipBorder(), locationInDIPs.y - offset.y);
                 }
             }
         }

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -1085,7 +1085,7 @@ void ConptyRoundtripTests::PassthroughClearAll()
         sm.ProcessString(L"~");
     }
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport, const bool afterClear = false) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport, const bool afterClear = false) {
         const auto width = viewport.narrow_width<short>();
 
         // "~" rows
@@ -1456,7 +1456,7 @@ void ConptyRoundtripTests::ScrollWithChangesInMiddle()
                          L"8"); // Restore
     hostSm.ProcessString(std::wstring(wrappedLineLength, L'A')); // Print 100 'A's
 
-    auto verifyBuffer = [](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [](const TextBuffer& tb, const til::rect& viewport) {
         const auto wrappedRow = gsl::narrow<short>(viewport.bottom - 2);
         const short start = viewport.narrow_top<short>();
         for (short i = start; i < wrappedRow; i++)
@@ -2272,7 +2272,7 @@ void ConptyRoundtripTests::OutputWrappedLineWithSpaceAtBottomOfBuffer()
     sm.ProcessString(std::wstring(spacesLength, L' '));
     sm.ProcessString(std::wstring(secondTextLength, L'B'));
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) {
         // Buffer contents should look like the following: (80 wide)
         // (w) means we hard wrapped the line
         // (b) means the line is _not_ wrapped (it's broken, the default state.)
@@ -2587,7 +2587,7 @@ void ConptyRoundtripTests::ResizeRepaintVimExeBuffer()
 
     drawVim();
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) {
         const auto firstRow = viewport.narrow_top<short>();
         const auto width = viewport.narrow_width<short>();
 
@@ -2697,7 +2697,7 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
         sm.ProcessString(L"~");
     }
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport, const bool afterClear = false) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport, const bool afterClear = false) {
         const auto width = viewport.narrow_width<short>();
 
         // "~" rows
@@ -2933,7 +2933,7 @@ void ConptyRoundtripTests::ResizeInitializeBufferWithDefaultAttrs()
         sm.ProcessString(L"#");
     }
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport, const bool isTerminal, const bool afterResize) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport, const bool isTerminal, const bool afterResize) {
         const auto width = viewport.narrow_width<short>();
 
         // Conhost and Terminal attributes are potentially different.
@@ -3071,7 +3071,7 @@ void ConptyRoundtripTests::NewLinesAtBottomWithBackground()
         }
     }
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) {
         const auto width = viewport.narrow_width<short>();
         const auto isTerminal = viewport.top != 0;
 
@@ -3271,7 +3271,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottom()
     // row[4]: |~~~~~~~~~~~~~~~~~~~| <wrap>
     // row[5]: |~~~~~~             | <break>
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) {
         const auto width = viewport.narrow_width<short>();
         const auto isTerminal = viewport.top != 0;
 
@@ -3473,7 +3473,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS()
         }
     }
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) {
         const auto width = viewport.narrow_width<short>();
         const auto isTerminal = viewport.top != 0;
         auto lastRow = gsl::narrow<short>(viewport.bottom - 1);
@@ -3549,7 +3549,7 @@ void ConptyRoundtripTests::DeleteWrappedWord()
     sm.ProcessString(std::wstring(50, L'B'));
     sm.ProcessString(L"\x1b[?25h");
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport, const bool after) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport, const bool after) {
         const auto width = viewport.narrow_width<short>();
 
         auto iter1 = tb.GetCellDataAt({ 0, 0 });
@@ -3711,7 +3711,7 @@ void ConptyRoundtripTests::ClearBufferSignal()
     sm.ProcessString(L"\x1b[?m");
     sm.ProcessString(L"\x1b[?25h");
 
-    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect viewport, const bool before) {
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport, const bool before) {
         const short width = viewport.narrow_width<short>();
         const short numCharsOnSecondLine = 50 - (width - 51);
         auto iter1 = tb.GetCellDataAt({ 0, 0 });

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -554,7 +554,7 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, LaunchMode
     // for the top here - the IslandWindow includes the titlebar in
     // nonClientFrame.top, so adjusting for that would actually place the
     // titlebar _off_ the monitor.
-    til::point origin{ (proposedRect.left + nonClientFrame.narrow_left<LONG>()),
+    til::point origin{ (proposedRect.left + nonClientFrame.left),
                        (proposedRect.top) };
 
     if (_logic.IsQuakeWindow())

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp

@@ -506,8 +506,8 @@ void NonClientIslandWindow::_OnMaximizeChange() noexcept
         const auto isIconified = WI_IsFlagSet(windowStyle, WS_ICONIC);
 
         const auto state = _isMaximized ? winrt::TerminalApp::WindowVisualState::WindowVisualStateMaximized :
-                           isIconified  ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified :
-                                          winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal;
+                                          isIconified ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified :
+                                                        winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal;
 
         try
         {

src/inc/til/operators.h

@@ -9,45 +9,45 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
 
 #pragma region POINT VS SIZE
     // This is a convenience and will take X vs WIDTH and Y vs HEIGHT.
-    constexpr point operator+(const point& lhs, const size& rhs)
+    constexpr point operator+(const point lhs, const size rhs)
     {
         return lhs + til::point{ rhs.width, rhs.height };
     }
 
-    constexpr point operator-(const point& lhs, const size& rhs)
+    constexpr point operator-(const point lhs, const size rhs)
     {
         return lhs - til::point{ rhs.width, rhs.height };
     }
 
-    constexpr point operator*(const point& lhs, const size& rhs)
+    constexpr point operator*(const point lhs, const size rhs)
     {
         return lhs * til::point{ rhs.width, rhs.height };
     }
 
-    constexpr point operator/(const point& lhs, const size& rhs)
+    constexpr point operator/(const point lhs, const size rhs)
     {
         return lhs / til::point{ rhs.width, rhs.height };
     }
 #pragma endregion
 
 #pragma region SIZE VS POINT
     // This is a convenience and will take WIDTH vs X and HEIGHT vs Y.
-    constexpr size operator+(const size& lhs, const point& rhs)
+    constexpr size operator+(const size lhs, const point rhs)
     {
         return lhs + til::size(rhs.x, rhs.y);
     }
 
-    constexpr size operator-(const size& lhs, const point& rhs)
+    constexpr size operator-(const size lhs, const point rhs)
     {
         return lhs - til::size(rhs.x, rhs.y);
     }
 
-    constexpr size operator*(const size& lhs, const point& rhs)
+    constexpr size operator*(const size lhs, const point rhs)
     {
         return lhs * til::size(rhs.x, rhs.y);
     }
 
-    constexpr size operator/(const size& lhs, const point& rhs)
+    constexpr size operator/(const size lhs, const point rhs)
     {
         return lhs / til::size(rhs.x, rhs.y);
     }

src/inc/til/rect.h

@@ -826,7 +826,7 @@ namespace WEX::TestExecution
     class VerifyOutputTraits<til::rect>
     {
     public:
-        static WEX::Common::NoThrowString ToString(const til::rect rect)
+        static WEX::Common::NoThrowString ToString(const til::rect& rect)
         {
             return WEX::Common::NoThrowString(rect.to_string().c_str());
         }
@@ -836,21 +836,21 @@ namespace WEX::TestExecution
     class VerifyCompareTraits<til::rect, til::rect>
     {
     public:
-        static bool AreEqual(const til::rect expected, const til::rect actual) noexcept
+        static bool AreEqual(const til::rect& expected, const til::rect& actual) noexcept
         {
             return expected == actual;
         }
 
-        static bool AreSame(const til::rect expected, const til::rect actual) noexcept
+        static bool AreSame(const til::rect& expected, const til::rect& actual) noexcept
         {
             return &expected == &actual;
         }
 
-        static bool IsLessThan(const til::rect expectedLess, const til::rect expectedGreater) = delete;
+        static bool IsLessThan(const til::rect& expectedLess, const til::rect& expectedGreater) = delete;
 
-        static bool IsGreaterThan(const til::rect expectedGreater, const til::rect expectedLess) = delete;
+        static bool IsGreaterThan(const til::rect& expectedGreater, const til::rect& expectedLess) = delete;
 
-        static bool IsNull(const til::rect object) noexcept
+        static bool IsNull(const til::rect& object) noexcept
         {
             return object == til::rect{};
         }

src/interactivity/onecore/BgfxEngine.cpp

@@ -233,13 +233,8 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid
 
 [[nodiscard]] HRESULT BgfxEngine::GetDirtyArea(gsl::span<const til::rect>& area) noexcept
 {
-    SMALL_RECT r;
-    r.Bottom = _displayHeight > 0 ? (SHORT)(_displayHeight - 1) : 0;
-    r.Top = 0;
-    r.Left = 0;
-    r.Right = _displayWidth > 0 ? (SHORT)(_displayWidth - 1) : 0;
-
-    _dirtyArea = r;
+    _dirtyArea.bottom = std::max(0, _displayHeight);
+    _dirtyArea.right = std::max(0, _displayWidth);
 
     area = { &_dirtyArea,
              1 };

src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp

@@ -21,7 +21,7 @@ using namespace Microsoft::WRL;
 
 using namespace Microsoft::Console::Interactivity::Win32;
 
-static constexpr til::point point_offset_by_char(const til::point start, const til::rect bounds, til::CoordType amt)
+static constexpr til::point point_offset_by_char(const til::point start, const til::rect& bounds, til::CoordType amt)
 {
     auto pos_x = start.x;
     auto pos_y = start.y;
@@ -71,7 +71,7 @@ static constexpr til::point point_offset_by_char(const til::point start, const t
     return { pos_x, pos_y };
 }
 
-static constexpr til::point point_offset_by_line(const til::point start, const til::rect bounds, til::CoordType amt)
+static constexpr til::point point_offset_by_line(const til::point start, const til::rect& bounds, til::CoordType amt)
 {
     // X = left boundary for UIA
     auto pos_x = bounds.left;

src/renderer/atlas/AtlasEngine.cpp

@@ -378,10 +378,10 @@ try
     }
 
     _api.dirtyRect = til::rect{
-        static_cast<ptrdiff_t>(0),
-        static_cast<ptrdiff_t>(_api.invalidatedRows.x),
-        static_cast<ptrdiff_t>(_api.cellCount.x),
-        static_cast<ptrdiff_t>(_api.invalidatedRows.y),
+        0,
+        _api.invalidatedRows.x,
+        _api.cellCount.x,
+        _api.invalidatedRows.y,
     };
 
     return S_OK;

src/renderer/dx/DxRenderer.cpp

@@ -463,8 +463,8 @@ void DxEngine::_ComputePixelShaderSettings() noexcept
             _pixelShaderSettings.Scale = _scale;
 
             // Set the display resolution
-            const float w = 1.0f * _displaySizePixels.narrow_width<UINT>();
-            const float h = 1.0f * _displaySizePixels.narrow_height<UINT>();
+            const float w = static_cast<float>(_displaySizePixels.width);
+            const float h = static_cast<float>(_displaySizePixels.height);
             _pixelShaderSettings.Resolution = XMFLOAT2{ w, h };
 
             // Set the background

src/renderer/gdi/state.cpp

@@ -308,8 +308,7 @@ GdiEngine::~GdiEngine()
 
     // If the font type has changed, select an appropriate font variant or soft font.
     const auto usingItalicFont = textAttributes.IsItalic();
-    const auto fontType = usingSoftFont ? FontType::Soft : usingItalicFont ? FontType::Italic :
-                                                                             FontType::Default;
+    const auto fontType = usingSoftFont ? FontType::Soft : usingItalicFont ? FontType::Italic : FontType::Default;
     if (fontType != _lastFontType)
     {
         switch (fontType)

src/renderer/uia/UiaRenderer.cpp

@@ -444,7 +444,7 @@ void UiaEngine::WaitUntilCanRender() noexcept
 {
     // Magic static is only valid because any instance of this object has the same behavior.
     // Use member variable instead if this ever changes.
-    const static til::rect empty;
+    static constexpr til::rect empty;
     area = { &empty, 1 };
     return S_OK;
 }

src/renderer/vt/tracing.cpp

@@ -82,7 +82,7 @@ void RenderTracing::TraceString(const std::string_view& instr) const
 #endif UNIT_TESTING
 }
 
-void RenderTracing::TraceInvalidate(const til::rect invalidRect) const
+void RenderTracing::TraceInvalidate(const til::rect& invalidRect) const
 {
 #ifndef UNIT_TESTING
     if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, TIL_KEYWORD_TRACE))
@@ -100,7 +100,7 @@ void RenderTracing::TraceInvalidate(const til::rect invalidRect) const
 #endif UNIT_TESTING
 }
 
-void RenderTracing::TraceInvalidateAll(const til::rect viewport) const
+void RenderTracing::TraceInvalidateAll(const til::rect& viewport) const
 {
 #ifndef UNIT_TESTING
     if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, TIL_KEYWORD_TRACE))
@@ -151,7 +151,7 @@ void RenderTracing::TraceInvalidateScroll(const til::point scroll) const
 
 void RenderTracing::TraceStartPaint(const bool quickReturn,
                                     const til::pmr::bitmap& invalidMap,
-                                    const til::rect lastViewport,
+                                    const til::rect& lastViewport,
                                     const til::point scrollDelt,
                                     const bool cursorMoved,
                                     const std::optional<short>& wrappedRow) const

src/renderer/vt/tracing.hpp

@@ -27,20 +27,20 @@ namespace Microsoft::Console::VirtualTerminal
         RenderTracing();
         ~RenderTracing();
         void TraceString(const std::string_view& str) const;
-        void TraceInvalidate(const til::rect view) const;
+        void TraceInvalidate(const til::rect& view) const;
         void TraceLastText(const til::point lastText) const;
         void TraceScrollFrame(const til::point scrollDelta) const;
         void TraceMoveCursor(const til::point lastText, const til::point cursor) const;
         void TraceSetWrapped(const short wrappedRow) const;
         void TraceClearWrapped() const;
         void TraceWrapped() const;
         void TracePaintCursor(const til::point coordCursor) const;
-        void TraceInvalidateAll(const til::rect view) const;
+        void TraceInvalidateAll(const til::rect& view) const;
         void TraceTriggerCircling(const bool newFrame) const;
         void TraceInvalidateScroll(const til::point scroll) const;
         void TraceStartPaint(const bool quickReturn,
                              const til::pmr::bitmap& invalidMap,
-                             const til::rect lastViewport,
+                             const til::rect& lastViewport,
                              const til::point scrollDelta,
                              const bool cursorMoved,
                              const std::optional<short>& wrappedRow) const;