From ef246caf86cb5ed3d63556f8b51daa82f7116c5b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 11:12:37 -0500 Subject: [PATCH 01/25] This seems to work, but can it avoid resizing the buffer twice on a DPI change? --- src/cascadia/TerminalControl/TermControl.cpp | 64 ++++++++++++++++---- src/cascadia/TerminalControl/TermControl.h | 1 + src/renderer/dx/DxRenderer.cpp | 7 ++- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index c0a83adff63..9e446c9c80f 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -514,8 +514,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return false; } - const auto windowWidth = SwapChainPanel().ActualWidth() * SwapChainPanel().CompositionScaleX(); // Width() and Height() are NaN? - const auto windowHeight = SwapChainPanel().ActualHeight() * SwapChainPanel().CompositionScaleY(); + const auto actualWidth = SwapChainPanel().ActualWidth(); + const auto actualHeight = SwapChainPanel().ActualHeight(); + + const auto windowWidth = actualWidth * SwapChainPanel().CompositionScaleX(); // Width() and Height() are NaN? + const auto windowHeight = actualHeight * SwapChainPanel().CompositionScaleY(); if (windowWidth == 0 || windowHeight == 0) { @@ -1648,12 +1651,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Refresh our font with the renderer _UpdateFont(); - auto lock = _terminal->LockForWriting(); // Resize the terminal's BUFFER to match the new font size. This does // NOT change the size of the window, because that can lead to more // problems (like what happens when you change the font size while the // window is maximized?) - _DoResize(SwapChainPanel().ActualWidth(), SwapChainPanel().ActualHeight()); + _RefreshSize(); } CATCH_LOG(); } @@ -1673,7 +1675,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto lock = _terminal->LockForWriting(); - auto foundationSize = e.NewSize(); + auto newSize = e.NewSize(); + auto currentScaleX = SwapChainPanel().CompositionScaleX(); + currentScaleX; + auto currentScaleY = SwapChainPanel().CompositionScaleY(); + currentScaleY; + auto foundationSize = newSize; foundationSize.Width *= SwapChainPanel().CompositionScaleX(); foundationSize.Height *= SwapChainPanel().CompositionScaleY(); @@ -1685,10 +1692,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (_renderEngine) { - const auto scale = sender.CompositionScaleX(); - const auto dpi = (int)(scale * USER_DEFAULT_SCREEN_DPI); + const auto scaleX = sender.CompositionScaleX(); + const auto scaleY = sender.CompositionScaleY(); + const auto dpi = (int)(scaleX * USER_DEFAULT_SCREEN_DPI); + const auto currentEngineScale = _renderEngine->GetScaling(); + currentEngineScale; + + const auto actualFontOldSize = _actualFont.GetSize(); _renderer->TriggerFontChange(dpi, _desiredFont, _actualFont); + _renderer->TriggerRedrawAll(); + + const auto actualFontNewSize = _actualFont.GetSize(); + if (actualFontNewSize != actualFontOldSize) + { + _RefreshSize(); + } } } @@ -1727,6 +1746,31 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _selectionNeedsToBeCopied = true; } + // Method Description: + // - Perform a resize for the current size of the swapchainpanel. If the + // font size changed, we'll need to resize the buffer to fit the existing + // swapchain size. This helper will call _DoResize with the current size + // of the swapchain, accounting for scaling due to DPI. + // - Note that a DPI change will also trigger a font size change, and will call into here. + // Arguments: + // - + // Return Value: + // - + void TermControl::_RefreshSize() + { + const auto currentScaleX = SwapChainPanel().CompositionScaleX(); + const auto currentScaleY = SwapChainPanel().CompositionScaleY(); + const auto actualWidth = SwapChainPanel().ActualWidth(); + const auto actualHeight = SwapChainPanel().ActualHeight(); + + const auto widthInPixels = actualWidth * currentScaleX; + const auto heightInPixels = actualHeight * currentScaleY; + + // Grab the lock, because we might be changing the buffer size here. + auto lock = _terminal->LockForWriting(); + _DoResize(widthInPixels, heightInPixels); + } + // Method Description: // - Process a resize event that was initiated by the user. This can either // be due to the user resizing the window (causing the swapchain to @@ -1762,11 +1806,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels); // If this function succeeds with S_FALSE, then the terminal didn't - // actually change size. No need to notify the connection of this - // no-op. - // TODO: MSFT:20642295 Resizing the buffer will corrupt it - // I believe we'll need support for CSI 2J, and additionally I think - // we're resetting the viewport to the top + // actually change size. No need to notify the connection of this no-op. const HRESULT hr = _terminal->UserResize({ vp.Width(), vp.Height() }); if (SUCCEEDED(hr) && hr != S_FALSE) { diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index d4f62168ef3..81a8fc85ce6 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -198,6 +198,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void _SwapChainSizeChanged(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::SizeChangedEventArgs const& e); void _SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& args); void _DoResize(const double newWidth, const double newHeight); + void _RefreshSize(); void _TerminalTitleChanged(const std::wstring_view& wstr); winrt::fire_and_forget _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize); winrt::fire_and_forget _TerminalCursorPositionChanged(); diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 0b21fcc747f..2bab149bb7b 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1567,8 +1567,11 @@ CATCH_RETURN(); [[nodiscard]] Viewport DxEngine::GetViewportInCharacters(const Viewport& viewInPixels) noexcept { - const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / _glyphCell.width()); - const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / _glyphCell.height()); + // const auto scaledGlyphSize = _glyphCell.scale(til::math::ceiling, _scale); + // const auto scaledGlyphSize = _glyphCell.scale(til::math::ceiling, (1.0f / _scale)); + const auto scaledGlyphSize = _glyphCell; + const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / scaledGlyphSize.width()); + const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / scaledGlyphSize.height()); return Viewport::FromDimensions(viewInPixels.Origin(), { widthInChars, heightInChars }); } From 87f833c966c14cef0425b4c35c811228ed21a057 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 11:14:06 -0500 Subject: [PATCH 02/25] Add a doc comment that should have been in the previous commit --- src/cascadia/TerminalControl/TermControl.cpp | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 9e446c9c80f..81414023756 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1687,6 +1687,30 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _DoResize(foundationSize.Width, foundationSize.Height); } + // Method Description: + // - Triggered when the swapchain changes DPI. When this happens, we're + // going to recieve 3 events: + // - 1. First, a CompositionScaleChanged _for the original scale_. I don't + // know why this event happens first. + // - 2. Then, a SizeChanged. During that SizeChanged, the scale will still + // be the original DPI. Again, I have no idea why the size & DPI is + // updated in this order. + // - 3. Finally, a CompositionScaleChanged with the _new_ DPI. + // - 4. We'll usually get another SizeChanged some time after this last + // ScaleChanged. This usually seems to happen after something triggeres + // the UI to re-layout, like hovering over the scrollbar. This event + // doesn't reliably happen immediately after a scale change, so we can't + // depend on it (despite the fact that both the scale and size state is + // definitely correct in it) + // - In the 3rd event, we're going to update our font size for te new DPI. + // At that point, we know how big the font should be for the new DPI, and + // how big the SwapChainPanel will be. If these sizes are different, we'll + // need to resize the buffer to fit in the new window. + // - TODO: Right now we'll _also_ resize in response to event 2. Is there a + // way for us to skip that resize, as an optimization? + // Arguments: + // - sender: The SwapChainPanel who's DPI changed. This is our _swapchainPanel. + // - args: This param is unused in the CompositionScaleChanged event. void TermControl::_SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& /*args*/) { From 96c6ba9c491367e8b4962c7434e5aa894a6398cb Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 11:42:33 -0500 Subject: [PATCH 03/25] Remove some dead code I didn't end up needing --- src/cascadia/TerminalControl/TermControl.cpp | 1 - src/renderer/dx/DxRenderer.cpp | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 81414023756..5029b9c9bec 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1725,7 +1725,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto actualFontOldSize = _actualFont.GetSize(); _renderer->TriggerFontChange(dpi, _desiredFont, _actualFont); - _renderer->TriggerRedrawAll(); const auto actualFontNewSize = _actualFont.GetSize(); if (actualFontNewSize != actualFontOldSize) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 2bab149bb7b..0b21fcc747f 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1567,11 +1567,8 @@ CATCH_RETURN(); [[nodiscard]] Viewport DxEngine::GetViewportInCharacters(const Viewport& viewInPixels) noexcept { - // const auto scaledGlyphSize = _glyphCell.scale(til::math::ceiling, _scale); - // const auto scaledGlyphSize = _glyphCell.scale(til::math::ceiling, (1.0f / _scale)); - const auto scaledGlyphSize = _glyphCell; - const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / scaledGlyphSize.width()); - const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / scaledGlyphSize.height()); + const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / _glyphCell.width()); + const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / _glyphCell.height()); return Viewport::FromDimensions(viewInPixels.Origin(), { widthInChars, heightInChars }); } From 727fb065747c70ab0401873910994aba5940c2a8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 12:30:35 -0500 Subject: [PATCH 04/25] convert TSFInputControl::_RedrawCanvas to use til wherever possible This is just here to help me debug --- .../TerminalControl/TSFInputControl.cpp | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index 477a565733a..d9b902570e5 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -175,24 +175,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto fontArgs = winrt::make_self(); _CurrentFontInfoHandlers(*this, *fontArgs); - const auto fontWidth = fontArgs->FontSize().Width; - const auto fontHeight = fontArgs->FontSize().Height; + const til::size fontSize{ til::math::flooring, fontArgs->FontSize() }; // Convert text buffer cursor position to client coordinate position within the window - COORD clientCursorPos; - clientCursorPos.X = ::base::ClampMul(_currentTerminalCursorPos.x(), ::base::ClampedNumeric(fontWidth)); - clientCursorPos.Y = ::base::ClampMul(_currentTerminalCursorPos.y(), ::base::ClampedNumeric(fontHeight)); + const til::point clientCursorPos{ _currentTerminalCursorPos * fontSize }; // position textblock to cursor position - Canvas().SetLeft(TextBlock(), clientCursorPos.X); - Canvas().SetTop(TextBlock(), clientCursorPos.Y); + Canvas().SetLeft(TextBlock(), clientCursorPos.x()); + Canvas().SetTop(TextBlock(), clientCursorPos.y()); // calculate FontSize in pixels from DPIs - const double fontSizePx = (fontHeight * 72) / USER_DEFAULT_SCREEN_DPI; + const double fontSizePx = (fontSize.height() * 72) / USER_DEFAULT_SCREEN_DPI; TextBlock().FontSize(fontSizePx); TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace())); - const auto widthToTerminalEnd = _currentCanvasWidth - ::base::ClampedNumeric(clientCursorPos.X); + const auto widthToTerminalEnd = _currentCanvasWidth - clientCursorPos.x(); // Make sure that we're setting the MaxWidth to a positive number - a // negative number here will crash us in mysterious ways with a useless // stack trace @@ -200,27 +197,35 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TextBlock().MaxWidth(newMaxWidth); // Get window in screen coordinates, this is the entire window including tabs - const auto windowBounds = CoreWindow::GetForCurrentThread().Bounds(); + const auto windowBounds{ CoreWindow::GetForCurrentThread().Bounds() }; + const til::point windowOrigin{ til::math::flooring, windowBounds }; // Convert from client coordinate to screen coordinate by adding window position - COORD screenCursorPos; - screenCursorPos.X = ::base::ClampAdd(clientCursorPos.X, ::base::ClampedNumeric(windowBounds.X)); - screenCursorPos.Y = ::base::ClampAdd(clientCursorPos.Y, ::base::ClampedNumeric(windowBounds.Y)); + til::point screenCursorPos{ clientCursorPos + windowOrigin }; // get any offset (margin + tabs, etc..) of the control within the window - const auto offsetPoint = this->TransformToVisual(nullptr).TransformPoint(winrt::Windows::Foundation::Point(0, 0)); + const til::point controlOrigin{ til::math::flooring, + this->TransformToVisual(nullptr).TransformPoint(Point(0, 0)) }; // add the margin offsets if any - screenCursorPos.X = ::base::ClampAdd(screenCursorPos.X, ::base::ClampedNumeric(offsetPoint.X)); - screenCursorPos.Y = ::base::ClampAdd(screenCursorPos.Y, ::base::ClampedNumeric(offsetPoint.Y)); + screenCursorPos += controlOrigin; // Get scale factor for view const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); - const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontHeight; - const auto textBottom = ::base::ClampedNumeric(screenCursorPos.Y) + yOffset; - - _currentTextBounds = ScaleRect(Rect(screenCursorPos.X, textBottom, 0, fontHeight), scaleFactor); - _currentControlBounds = ScaleRect(Rect(screenCursorPos.X, screenCursorPos.Y, 0, fontHeight), scaleFactor); + const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontSize.height(); + const auto textBottom = ::base::ClampedNumeric(screenCursorPos.y()) + yOffset; + + _currentTextBounds = ScaleRect(Rect(screenCursorPos.x(), + textBottom, + 0, + fontSize.height()), + scaleFactor); + + _currentControlBounds = ScaleRect(Rect(screenCursorPos.x(), + screenCursorPos.y(), + 0, + fontSize.height()), + scaleFactor); } // Method Description: From defba0544c71cdc88caa4efe0ca57526949b3f05 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 14:40:38 -0500 Subject: [PATCH 05/25] I think this works fine on high-dpi for latin, cyrillic, emoji, chinese --- .../TerminalControl/TSFInputControl.cpp | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index d9b902570e5..e286bfb8e05 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -201,25 +201,32 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const til::point windowOrigin{ til::math::flooring, windowBounds }; // Convert from client coordinate to screen coordinate by adding window position - til::point screenCursorPos{ clientCursorPos + windowOrigin }; + // til::point screenCursorPos{ clientCursorPos + windowOrigin }; // get any offset (margin + tabs, etc..) of the control within the window const til::point controlOrigin{ til::math::flooring, this->TransformToVisual(nullptr).TransformPoint(Point(0, 0)) }; - // add the margin offsets if any - screenCursorPos += controlOrigin; + const til::point frameOrigin{ windowOrigin + controlOrigin }; // Get scale factor for view const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); - const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontSize.height(); + const til::point scaledFrameOrigin = frameOrigin * scaleFactor; + + // add the margin offsets if any + til::point screenCursorPos{ scaledFrameOrigin + clientCursorPos }; + + // const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontSize.height(); + // const auto yOffset = fontSize.height(); + const auto yOffset = 0; const auto textBottom = ::base::ClampedNumeric(screenCursorPos.y()) + yOffset; - _currentTextBounds = ScaleRect(Rect(screenCursorPos.x(), - textBottom, - 0, - fontSize.height()), - scaleFactor); + til::rectangle textBounds{ til::point{ screenCursorPos.x() + fontSize.width(), textBottom }, + til::size{ (ptrdiff_t)0, fontSize.height() } }; + + auto r = Rect(textBounds.left(), textBounds.top(), textBounds.width(), textBounds.height()); + // _currentTextBounds = ScaleRect(r, scaleFactor); + _currentTextBounds = ScaleRect(r, 1.0); _currentControlBounds = ScaleRect(Rect(screenCursorPos.x(), screenCursorPos.y(), From 4c411882dbbb8e66166aea62cff2e26a98cbf88d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 16:23:33 -0500 Subject: [PATCH 06/25] This _actaully_ works and displays the composition correctly. Needs comments --- .../TerminalControl/TSFInputControl.cpp | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index e286bfb8e05..4610044dc95 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -180,16 +180,28 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Convert text buffer cursor position to client coordinate position within the window const til::point clientCursorPos{ _currentTerminalCursorPos * fontSize }; + // Get scale factor for view + const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); + const til::point clientCursorInDips{ clientCursorPos / scaleFactor }; + // position textblock to cursor position - Canvas().SetLeft(TextBlock(), clientCursorPos.x()); - Canvas().SetTop(TextBlock(), clientCursorPos.y()); + // Canvas().SetLeft(TextBlock(), clientCursorPos.x()); + // Canvas().SetTop(TextBlock(), clientCursorPos.y()); + + // Canvas().SetLeft(TextBlock(), _currentTerminalCursorPos.x()); + // Canvas().SetTop(TextBlock(), _currentTerminalCursorPos.y()); + + Canvas().SetLeft(TextBlock(), clientCursorInDips.x()); + Canvas().SetTop(TextBlock(), clientCursorInDips.y()); // calculate FontSize in pixels from DPIs const double fontSizePx = (fontSize.height() * 72) / USER_DEFAULT_SCREEN_DPI; - TextBlock().FontSize(fontSizePx); + // const double fontSizePx = fontSize.height(); + // TextBlock().FontSize(fontSizePx); + TextBlock().FontSize(fontSizePx / scaleFactor); TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace())); - const auto widthToTerminalEnd = _currentCanvasWidth - clientCursorPos.x(); + const auto widthToTerminalEnd = _currentCanvasWidth - clientCursorInDips.x(); // Make sure that we're setting the MaxWidth to a positive number - a // negative number here will crash us in mysterious ways with a useless // stack trace @@ -209,8 +221,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const til::point frameOrigin{ windowOrigin + controlOrigin }; - // Get scale factor for view - const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); const til::point scaledFrameOrigin = frameOrigin * scaleFactor; // add the margin offsets if any @@ -232,7 +242,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation screenCursorPos.y(), 0, fontSize.height()), - scaleFactor); + 1.0); + // scaleFactor); } // Method Description: From f8704703af64bf5432508ee05bf80d2e61c7a82a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 16:57:03 -0500 Subject: [PATCH 07/25] Clean up this code significantly --- .../TerminalControl/TSFInputControl.cpp | 61 ++++++++----------- src/inc/til/rectangle.h | 12 ++++ src/inc/til/size.h | 8 +++ 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index 4610044dc95..58755c63615 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -177,27 +177,24 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const til::size fontSize{ til::math::flooring, fontArgs->FontSize() }; - // Convert text buffer cursor position to client coordinate position within the window + // Convert text buffer cursor position to client coordinate position + // within the window. This point is in _pixels_ const til::point clientCursorPos{ _currentTerminalCursorPos * fontSize }; // Get scale factor for view const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); - const til::point clientCursorInDips{ clientCursorPos / scaleFactor }; - - // position textblock to cursor position - // Canvas().SetLeft(TextBlock(), clientCursorPos.x()); - // Canvas().SetTop(TextBlock(), clientCursorPos.y()); - // Canvas().SetLeft(TextBlock(), _currentTerminalCursorPos.x()); - // Canvas().SetTop(TextBlock(), _currentTerminalCursorPos.y()); + const til::point clientCursorInDips{ clientCursorPos / scaleFactor }; + // Position our TextBlock at the cursor position Canvas().SetLeft(TextBlock(), clientCursorInDips.x()); Canvas().SetTop(TextBlock(), clientCursorInDips.y()); - // calculate FontSize in pixels from DPIs + // calculate FontSize in pixels from Points const double fontSizePx = (fontSize.height() * 72) / USER_DEFAULT_SCREEN_DPI; - // const double fontSizePx = fontSize.height(); - // TextBlock().FontSize(fontSizePx); + + // Make sure to unscale the font size to correct for DPI! XAML needs + // things in DIPs, and the fontSize is in pixels. TextBlock().FontSize(fontSizePx / scaleFactor); TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace())); @@ -208,42 +205,36 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto newMaxWidth = std::max(0.0, widthToTerminalEnd); TextBlock().MaxWidth(newMaxWidth); - // Get window in screen coordinates, this is the entire window including tabs + // Get window in screen coordinates, this is the entire window including + // tabs. THIS IS IN DIPs const auto windowBounds{ CoreWindow::GetForCurrentThread().Bounds() }; const til::point windowOrigin{ til::math::flooring, windowBounds }; - // Convert from client coordinate to screen coordinate by adding window position - // til::point screenCursorPos{ clientCursorPos + windowOrigin }; - - // get any offset (margin + tabs, etc..) of the control within the window + // Get the offset (margin + tabs, etc..) of the control within the window + // TODO: Test with a padding on the control. THIS IS IN DIPs const til::point controlOrigin{ til::math::flooring, this->TransformToVisual(nullptr).TransformPoint(Point(0, 0)) }; - const til::point frameOrigin{ windowOrigin + controlOrigin }; + // The controlAbsoluteOrigin is the origin of the control relative to + // the origin of the displays. THIS IS IN DIPs + const til::point controlAbsoluteOrigin{ windowOrigin + controlOrigin }; - const til::point scaledFrameOrigin = frameOrigin * scaleFactor; + // Convert the control origin to pixels + const til::point scaledFrameOrigin = controlAbsoluteOrigin * scaleFactor; - // add the margin offsets if any + // Get the location of the cursor in the display, in pixels. til::point screenCursorPos{ scaledFrameOrigin + clientCursorPos }; - // const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontSize.height(); - // const auto yOffset = fontSize.height(); - const auto yOffset = 0; - const auto textBottom = ::base::ClampedNumeric(screenCursorPos.y()) + yOffset; - - til::rectangle textBounds{ til::point{ screenCursorPos.x() + fontSize.width(), textBottom }, - til::size{ (ptrdiff_t)0, fontSize.height() } }; + // Get the bounds of the composition text, in pixels. + const til::rectangle textBounds{ til::point{ screenCursorPos.x() + fontSize.width(), screenCursorPos.y() }, + til::size{ 0, fontSize.height() } }; - auto r = Rect(textBounds.left(), textBounds.top(), textBounds.width(), textBounds.height()); - // _currentTextBounds = ScaleRect(r, scaleFactor); - _currentTextBounds = ScaleRect(r, 1.0); + _currentTextBounds = textBounds; - _currentControlBounds = ScaleRect(Rect(screenCursorPos.x(), - screenCursorPos.y(), - 0, - fontSize.height()), - 1.0); - // scaleFactor); + _currentControlBounds = Rect(screenCursorPos.x(), + screenCursorPos.y(), + 0, + fontSize.height()); } // Method Description: diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index ba488ea7ea9..83bb0b3f11a 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -863,6 +863,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } #endif +#ifdef WINRT_Windows_Foundation_H + operator winrt::Windows::Foundation::Rect() const + { + winrt::Windows::Foundation::Rect ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(left()).AssignIfValid(&ret.X)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(top()).AssignIfValid(&ret.Y)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(width()).AssignIfValid(&ret.Width)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(height()).AssignIfValid(&ret.Height)); + return ret; + } +#endif + std::wstring to_string() const { return wil::str_printf(L"(L:%td, T:%td, R:%td, B:%td) [W:%td, H:%td]", left(), top(), right(), bottom(), width(), height()); diff --git a/src/inc/til/size.h b/src/inc/til/size.h index b7d71dbd46b..54cd7a4b398 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -25,6 +25,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" size(static_cast(width), static_cast(height)) { } + constexpr size(ptrdiff_t width, int height) noexcept : + size(width, static_cast(height)) + { + } + constexpr size(int width, ptrdiff_t height) noexcept : + size(static_cast(width), height) + { + } #endif size(size_t width, size_t height) From ace7a889205dd12007206426a54585e7252e58b6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 16 Apr 2020 17:01:08 -0500 Subject: [PATCH 08/25] This does in fact work --- src/cascadia/TerminalControl/TSFInputControl.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index 58755c63615..e1c467f5128 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -211,7 +211,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const til::point windowOrigin{ til::math::flooring, windowBounds }; // Get the offset (margin + tabs, etc..) of the control within the window - // TODO: Test with a padding on the control. THIS IS IN DIPs const til::point controlOrigin{ til::math::flooring, this->TransformToVisual(nullptr).TransformPoint(Point(0, 0)) }; From 24eb455d1b64db8990210726980fbef43b950759 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 09:31:33 -0500 Subject: [PATCH 09/25] Skip one of the two resizes during a DPI scale change. --- src/cascadia/TerminalControl/TermControl.cpp | 27 +++++++++++++++++--- src/cascadia/TerminalControl/TermControl.h | 2 ++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 5029b9c9bec..4cd48efecdb 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1684,7 +1684,17 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation foundationSize.Width *= SwapChainPanel().CompositionScaleX(); foundationSize.Height *= SwapChainPanel().CompositionScaleY(); - _DoResize(foundationSize.Width, foundationSize.Height); + // If we're in the middle of a DPI change, we're going to get a + // ScaleChanged, a SizeChanged, then a final ScaleChanged. In that + // scenario, we don't need to resize here. We'll resize in the following + // ScaleChanged. Right now, we don't know what the font size will be at + // the new DPI, so we're going to have to resize again anyways. Might + // was well just skip this one. + if (!_inDpiResize) + { + _DoResize(foundationSize.Width, foundationSize.Height); + } + _inDpiResize = false; } // Method Description: @@ -1718,13 +1728,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { const auto scaleX = sender.CompositionScaleX(); const auto scaleY = sender.CompositionScaleY(); - const auto dpi = (int)(scaleX * USER_DEFAULT_SCREEN_DPI); + const auto dpi = (float)(scaleX * USER_DEFAULT_SCREEN_DPI); const auto currentEngineScale = _renderEngine->GetScaling(); - currentEngineScale; + + // If we're getting a notification to change to the DPI we already + // have, then we're probably just beginning the DPI change. Since + // we'll get _another_ event with the real DPI, do nothing here for + // now. We'll also skip the next resize in _SwapChainSizeChanged. + if (currentEngineScale == dpi) + { + _inDpiResize = true; + return; + } const auto actualFontOldSize = _actualFont.GetSize(); - _renderer->TriggerFontChange(dpi, _desiredFont, _actualFont); + _renderer->TriggerFontChange(::base::saturated_cast(dpi), _desiredFont, _actualFont); const auto actualFontNewSize = _actualFont.GetSize(); if (actualFontNewSize != actualFontOldSize) diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 81a8fc85ce6..05400ec5977 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -172,6 +172,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; + bool _inDpiResize{ false }; + void _ApplyUISettings(); void _InitializeBackgroundBrush(); winrt::fire_and_forget _BackgroundColorChanged(const uint32_t color); From 936735ea4aa783a57a7b08487129e385fd50e427 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 10:38:58 -0500 Subject: [PATCH 10/25] This definitely fixes the gutters, but lets try to be more surgical about it --- src/renderer/dx/DxRenderer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 0b21fcc747f..34c865dc6a4 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -800,6 +800,7 @@ CATCH_RETURN(); try { _invalidMap.set_all(); + _firstFrame = true; return S_OK; } CATCH_RETURN(); From 3535b5a5ecc2ee77f75897ba550d6a2790bbe833 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 10:53:41 -0500 Subject: [PATCH 11/25] Add some comments --- src/renderer/dx/DxRenderer.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 34c865dc6a4..654c71846c0 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -800,6 +800,15 @@ CATCH_RETURN(); try { _invalidMap.set_all(); + + // Since everything is invalidated here, mark this as a "first frame", so + // that we won't use incremental drawing on it. The caller of this intended + // for _everything_ to get redrawn, so setting _firstFrame will force us to + // redraw the entire frame. This will make sure that things like the gutters + // get cleared correctly. + // + // Invalidating everything is supposed to happen with resizes of the + // entire canvas, changes of the font, and other such adjustments. _firstFrame = true; return S_OK; } @@ -1169,9 +1178,6 @@ try D2D1_COLOR_F nothing = { 0 }; // If the entire thing is invalid, just use one big clear operation. - // This will also hit the gutters outside the usual paintable area. - // Invalidating everything is supposed to happen with resizes of the - // entire canvas, changes of the font, and other such adjustments. if (_invalidMap.all()) { _d2dRenderTarget->Clear(nothing); From 436a05c358f6e5a684b3a8f457863c361c747f94 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 12:19:57 -0500 Subject: [PATCH 12/25] This fixes the 'vim open in an inactive tab' bug I was seeing, but I might be able to do it without reverting the double resize --- src/cascadia/TerminalControl/TermControl.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 4cd48efecdb..0c4906997ab 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1680,9 +1680,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation currentScaleX; auto currentScaleY = SwapChainPanel().CompositionScaleY(); currentScaleY; + + const auto currentEngineScale = _renderEngine->GetScaling(); + auto foundationSize = newSize; - foundationSize.Width *= SwapChainPanel().CompositionScaleX(); - foundationSize.Height *= SwapChainPanel().CompositionScaleY(); + + // foundationSize.Width *= SwapChainPanel().CompositionScaleX(); + // foundationSize.Height *= SwapChainPanel().CompositionScaleY(); + foundationSize.Width *= currentEngineScale; + foundationSize.Height *= currentEngineScale; // If we're in the middle of a DPI change, we're going to get a // ScaleChanged, a SizeChanged, then a final ScaleChanged. In that @@ -1690,7 +1696,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // ScaleChanged. Right now, we don't know what the font size will be at // the new DPI, so we're going to have to resize again anyways. Might // was well just skip this one. - if (!_inDpiResize) + // if (!_inDpiResize) { _DoResize(foundationSize.Width, foundationSize.Height); } @@ -1738,7 +1744,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (currentEngineScale == dpi) { _inDpiResize = true; - return; + // return; } const auto actualFontOldSize = _actualFont.GetSize(); From ed81ce79ba86fd22d9e080107b880c01350350aa Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 15:22:19 -0500 Subject: [PATCH 13/25] More good good til helpers --- src/inc/til/point.h | 15 +++++++++++++-- src/inc/til/size.h | 18 +++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/inc/til/point.h b/src/inc/til/point.h index fa9a869a6e9..6d7f40a36d6 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -53,8 +53,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { } + // This template will convert to point from floating-point args; + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" + template + constexpr point(TilMath, const TOther& x, const TOther& y, std::enable_if_t, int> /*sentinel*/ = 0) : + point(TilMath::template cast(x), TilMath::template cast(y)) + { + } + // This template will convert to size from anything that has a X and a Y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" template constexpr point(TilMath, const TOther& other, std::enable_if_t().X)> && std::is_floating_point_v().Y)>, int> /*sentinel*/ = 0) : point(TilMath::template cast(other.X), TilMath::template cast(other.Y)) @@ -62,7 +72,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a x and a y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" template constexpr point(TilMath, const TOther& other, std::enable_if_t().x)> && std::is_floating_point_v().y)>, int> /*sentinel*/ = 0) : point(TilMath::template cast(other.x), TilMath::template cast(other.y)) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 54cd7a4b398..67230b88202 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -62,7 +62,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a X and a Y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().X)> && std::is_floating_point_v().Y)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.X), TilMath::template cast(other.Y)) @@ -70,7 +71,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a cx and a cy field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().cx)> && std::is_floating_point_v().cy)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.cx), TilMath::template cast(other.cy)) @@ -78,13 +80,23 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a Width and a Height field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().Width)> && std::is_floating_point_v().Height)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.Width), TilMath::template cast(other.Height)) { } + // This template will convert to size from floating-point args; + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" + template + constexpr size(TilMath, const TOther& width, const TOther& height, std::enable_if_t, int> /*sentinel*/ = 0) : + size(TilMath::template cast(width), TilMath::template cast(height)) + { + } + constexpr bool operator==(const size& other) const noexcept { return _width == other._width && From 1d11c2300a15ba9abfdf15f3f6384721f00467ce Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 15:26:39 -0500 Subject: [PATCH 14/25] rewrite TermControl::_GetTerminalPosition, this might all be wrong though. --- src/cascadia/TerminalControl/TermControl.cpp | 52 ++++++++++++-------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 0c4906997ab..8a40da17a97 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1059,8 +1059,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point auto& touchdownPoint{ *_singleClickTouchdownPos }; auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) }; - const auto fontSize{ _actualFont.GetSize() }; - if (distance >= (std::min(fontSize.X, fontSize.Y) / 4.f)) + const til::size fontSize{ _actualFont.GetSize() }; + // const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); + // if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f)) + if (distance >= (std::min(fontSize.width(), fontSize.height()) / 4.f)) { _terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint)); // stop tracking the touchdown point @@ -1690,16 +1692,23 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation foundationSize.Width *= currentEngineScale; foundationSize.Height *= currentEngineScale; + const bool dpiHasAlreadyBeenUpdated = currentEngineScale == currentScaleX; // If we're in the middle of a DPI change, we're going to get a // ScaleChanged, a SizeChanged, then a final ScaleChanged. In that // scenario, we don't need to resize here. We'll resize in the following // ScaleChanged. Right now, we don't know what the font size will be at // the new DPI, so we're going to have to resize again anyways. Might // was well just skip this one. - // if (!_inDpiResize) + if (!_inDpiResize && (dpiHasAlreadyBeenUpdated)) { _DoResize(foundationSize.Width, foundationSize.Height); } + else + { + auto a = 0; + a++; + a; + } _inDpiResize = false; } @@ -1741,10 +1750,17 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // have, then we're probably just beginning the DPI change. Since // we'll get _another_ event with the real DPI, do nothing here for // now. We'll also skip the next resize in _SwapChainSizeChanged. - if (currentEngineScale == dpi) + const bool dpiWasUnchanged = currentEngineScale == scaleX; + if (dpiWasUnchanged) { _inDpiResize = true; - // return; + return; + } + else + { + auto b = 0; + b++; + b; } const auto actualFontOldSize = _actualFont.GetSize(); @@ -2390,25 +2406,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - the corresponding viewport terminal position for the given Point parameter const COORD TermControl::_GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition) { - // compensate for DPI scenarios. - cursorPosition.X *= SwapChainPanel().CompositionScaleX(); - cursorPosition.Y *= SwapChainPanel().CompositionScaleY(); + // cursorPosition is DIPs, relative to SwapChainPanel origin + const til::point cursorPosInDIPs{ til::math::rounding, cursorPosition }; + const til::size marginsInDips{ til::math::rounding, SwapChainPanel().Margin().Left, SwapChainPanel().Margin().Top }; - // Exclude padding from cursor position calculation - COORD terminalPosition = { - static_cast(cursorPosition.X - SwapChainPanel().Margin().Left), - static_cast(cursorPosition.Y - SwapChainPanel().Margin().Top) - }; + // This point is the location of the cursor within the actual grid of characters, in DIPs + const til::point relativeToMarginInDIPs = cursorPosInDIPs - marginsInDips; - const auto fontSize = _actualFont.GetSize(); - FAIL_FAST_IF(fontSize.X == 0); - FAIL_FAST_IF(fontSize.Y == 0); + // Convert it to pixels + const til::point relativeToMarginInPixels{ relativeToMarginInDIPs * SwapChainPanel().CompositionScaleX() }; - // Normalize to terminal coordinates by using font size - terminalPosition.X /= fontSize.X; - terminalPosition.Y /= fontSize.Y; + // Get the size of the font, which is in pixels + const til::size fontSize{ _actualFont.GetSize() }; - return terminalPosition; + // Convert the location in pixels to characters within the current viewport. + return til::point{ relativeToMarginInPixels / fontSize }; } // Method Description: From ed74b724f9940df57f0bd6acd7b30840f5553097 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 16:33:50 -0500 Subject: [PATCH 15/25] Do pointer event scaling in DIPs --- src/cascadia/TerminalControl/TermControl.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 8a40da17a97..d078e5f8d41 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1060,9 +1060,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto& touchdownPoint{ *_singleClickTouchdownPos }; auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) }; const til::size fontSize{ _actualFont.GetSize() }; - // const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); - // if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f)) - if (distance >= (std::min(fontSize.width(), fontSize.height()) / 4.f)) + + const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); + if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f)) { _terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint)); // stop tracking the touchdown point @@ -1102,19 +1102,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation winrt::Windows::Foundation::Point newTouchPoint{ contactRect.X, contactRect.Y }; const auto anchor = _touchAnchor.value(); - // Get the difference between the point we've dragged to and the start of the touch. - const float fontHeight = float(_actualFont.GetSize().Y); + // Our _actualFont's size is in pixels, convert to DIPs, which the + // rest of the Points here are in. + const til::size fontSize{ _actualFont.GetSize() }; + const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); + // Get the difference between the point we've dragged to and the start of the touch. const float dy = newTouchPoint.Y - anchor.Y; // Start viewport scroll after we've moved more than a half row of text - if (std::abs(dy) > (fontHeight / 2.0f)) + if (std::abs(dy) > (fontSizeInDips.height() / 2.0f)) { // Multiply by -1, because moving the touch point down will // create a positive delta, but we want the viewport to move up, // so we'll need a negative scroll amount (and the inverse for // panning down) - const float numRows = -1.0f * (dy / fontHeight); + const float numRows = -1.0f * (dy / fontSizeInDips.height()); const auto currentOffset = ::base::ClampedNumeric(ScrollBar().Value()); const auto newValue = numRows + currentOffset; From fb20b13d6325f9348f86151fc486a6bc2c0ba238 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 Apr 2020 16:36:21 -0500 Subject: [PATCH 16/25] Largely cleanup, but also add a TODO as it's almost 5pm here --- src/cascadia/TerminalControl/TermControl.cpp | 59 +++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index d078e5f8d41..38025823894 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1680,38 +1680,44 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto lock = _terminal->LockForWriting(); - auto newSize = e.NewSize(); - auto currentScaleX = SwapChainPanel().CompositionScaleX(); - currentScaleX; - auto currentScaleY = SwapChainPanel().CompositionScaleY(); - currentScaleY; - + const auto newSize = e.NewSize(); + const auto currentScaleX = SwapChainPanel().CompositionScaleX(); const auto currentEngineScale = _renderEngine->GetScaling(); - auto foundationSize = newSize; - // foundationSize.Width *= SwapChainPanel().CompositionScaleX(); - // foundationSize.Height *= SwapChainPanel().CompositionScaleY(); + // A strange thing can happen here. If you have two tabs open, and drag + // across a DPI boundary, then switch to the other tab, that tab will + // receive two events: First, a SizeChanged, then a ScaleChanged. In the + // SizeChanged event handler, the SwapChainPanel's CompositionScale will + // _already_ be the new scaling, but the engine won't have that value + // yet. If we scale by the CompositionScale here, we'll end up in a + // weird torn state. I'm not totally sure why. + // + // Fortunately we will be getting that following ScaleChanged event, and + // we'll end up resizing again, so we don't terribly need to worry about + // this. foundationSize.Width *= currentEngineScale; foundationSize.Height *= currentEngineScale; const bool dpiHasAlreadyBeenUpdated = currentEngineScale == currentScaleX; - // If we're in the middle of a DPI change, we're going to get a + dpiHasAlreadyBeenUpdated; + // If we're in the middle of a DPI change, we MIGHT get a // ScaleChanged, a SizeChanged, then a final ScaleChanged. In that // scenario, we don't need to resize here. We'll resize in the following // ScaleChanged. Right now, we don't know what the font size will be at // the new DPI, so we're going to have to resize again anyways. Might // was well just skip this one. - if (!_inDpiResize && (dpiHasAlreadyBeenUpdated)) + // + // However, we don't always get that first ScaleChanged, and if we + // don't, then we can't optimize this first resize out unfortunately. + // TODO: Can we skip this if !dpiHasAlreadyBeenUpdated? Originally I had + // if (!_inDpiResize && (dpiHasAlreadyBeenUpdated)) + // Which _felt_ right, but I deleted it for some reason + if (!_inDpiResize) { _DoResize(foundationSize.Width, foundationSize.Height); } - else - { - auto a = 0; - a++; - a; - } + _inDpiResize = false; } @@ -1719,10 +1725,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - Triggered when the swapchain changes DPI. When this happens, we're // going to recieve 3 events: // - 1. First, a CompositionScaleChanged _for the original scale_. I don't - // know why this event happens first. - // - 2. Then, a SizeChanged. During that SizeChanged, the scale will still - // be the original DPI. Again, I have no idea why the size & DPI is - // updated in this order. + // know why this event happens first. It also doesn't always happen. + // However, when it does happen, it doesn't give us any useful + // information. + // - 2. Then, a SizeChanged. During that SizeChanged, either: + // - the CompositionScale will still be the original DPI. This happens + // when the control is visible as the DPI changes. + // - The CompositionScale will be the new DPI. This happens when the + // control wasn't focused as the window's DPI changed, so it only got + // these messages after XAML updated it's scaling. // - 3. Finally, a CompositionScaleChanged with the _new_ DPI. // - 4. We'll usually get another SizeChanged some time after this last // ScaleChanged. This usually seems to happen after something triggeres @@ -1759,12 +1770,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _inDpiResize = true; return; } - else - { - auto b = 0; - b++; - b; - } const auto actualFontOldSize = _actualFont.GetSize(); From b067f3c3e7751d179430378c5f57802d84355023 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 07:56:58 -0500 Subject: [PATCH 17/25] Remove unused code, comments about receiving a useless ScaleChanged event --- src/cascadia/TerminalControl/TermControl.cpp | 26 ++------------------ src/cascadia/TerminalControl/TermControl.h | 2 -- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 38025823894..7d0a38a9635 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1699,33 +1699,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation foundationSize.Width *= currentEngineScale; foundationSize.Height *= currentEngineScale; - const bool dpiHasAlreadyBeenUpdated = currentEngineScale == currentScaleX; - dpiHasAlreadyBeenUpdated; - // If we're in the middle of a DPI change, we MIGHT get a - // ScaleChanged, a SizeChanged, then a final ScaleChanged. In that - // scenario, we don't need to resize here. We'll resize in the following - // ScaleChanged. Right now, we don't know what the font size will be at - // the new DPI, so we're going to have to resize again anyways. Might - // was well just skip this one. - // - // However, we don't always get that first ScaleChanged, and if we - // don't, then we can't optimize this first resize out unfortunately. - // TODO: Can we skip this if !dpiHasAlreadyBeenUpdated? Originally I had - // if (!_inDpiResize && (dpiHasAlreadyBeenUpdated)) - // Which _felt_ right, but I deleted it for some reason - if (!_inDpiResize) - { - _DoResize(foundationSize.Width, foundationSize.Height); - } - - _inDpiResize = false; + _DoResize(foundationSize.Width, foundationSize.Height); } // Method Description: // - Triggered when the swapchain changes DPI. When this happens, we're // going to recieve 3 events: // - 1. First, a CompositionScaleChanged _for the original scale_. I don't - // know why this event happens first. It also doesn't always happen. + // know why this event happens first. **It also doesn't always happen.** // However, when it does happen, it doesn't give us any useful // information. // - 2. Then, a SizeChanged. During that SizeChanged, either: @@ -1745,8 +1726,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // At that point, we know how big the font should be for the new DPI, and // how big the SwapChainPanel will be. If these sizes are different, we'll // need to resize the buffer to fit in the new window. - // - TODO: Right now we'll _also_ resize in response to event 2. Is there a - // way for us to skip that resize, as an optimization? // Arguments: // - sender: The SwapChainPanel who's DPI changed. This is our _swapchainPanel. // - args: This param is unused in the CompositionScaleChanged event. @@ -1767,7 +1746,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const bool dpiWasUnchanged = currentEngineScale == scaleX; if (dpiWasUnchanged) { - _inDpiResize = true; return; } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 05400ec5977..81a8fc85ce6 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -172,8 +172,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; - bool _inDpiResize{ false }; - void _ApplyUISettings(); void _InitializeBackgroundBrush(); winrt::fire_and_forget _BackgroundColorChanged(const uint32_t color); From f56a9b5fe7a725210af4526b547b225f6c14986b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 07:57:26 -0500 Subject: [PATCH 18/25] Update the font size in response to a settings reload --- src/cascadia/TerminalControl/TermControl.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 7d0a38a9635..002b3c7f5ea 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -225,17 +225,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _terminal->UpdateSettings(_settings); // Refresh our font with the renderer + const auto actualFontOldSize = _actualFont.GetSize(); _UpdateFont(); - - const auto width = SwapChainPanel().ActualWidth(); - const auto height = SwapChainPanel().ActualHeight(); - if (width != 0 && height != 0) + const auto actualFontNewSize = _actualFont.GetSize(); + if (actualFontNewSize != actualFontOldSize) { - // If the font size changed, or the _swapchainPanel's size changed - // for any reason, we'll need to make sure to also resize the - // buffer. _DoResize will invalidate everything for us. - auto lock = _terminal->LockForWriting(); - _DoResize(width, height); + _RefreshSize(); } } } From 4a32b33ce1558088a8cff8de41f5ce4b7ff26baf Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 09:01:24 -0500 Subject: [PATCH 19/25] Fix blanking vim on a settings update --- src/renderer/dx/DxRenderer.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 654c71846c0..432fd9c2e1c 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1178,7 +1178,12 @@ try D2D1_COLOR_F nothing = { 0 }; // If the entire thing is invalid, just use one big clear operation. - if (_invalidMap.all()) + // Don't use _invalidMap.all() here. That method checks if the union of the + // invalid areas of the map is the entire frame. However, there's a good + // chance that _not_ all the cells of the frame are actually invalid. If we + // used all() here, we'd clear the entire frame, but not actually paint all + // the cells again. + if (_firstFrame) { _d2dRenderTarget->Clear(nothing); } From 831ad65acc73799cee5845e0fe7cc787b11ee237 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 09:17:21 -0500 Subject: [PATCH 20/25] good bot --- src/cascadia/TerminalControl/TermControl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 002b3c7f5ea..1a80e89e11e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1699,7 +1699,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Method Description: // - Triggered when the swapchain changes DPI. When this happens, we're - // going to recieve 3 events: + // going to receive 3 events: // - 1. First, a CompositionScaleChanged _for the original scale_. I don't // know why this event happens first. **It also doesn't always happen.** // However, when it does happen, it doesn't give us any useful @@ -1712,7 +1712,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // these messages after XAML updated it's scaling. // - 3. Finally, a CompositionScaleChanged with the _new_ DPI. // - 4. We'll usually get another SizeChanged some time after this last - // ScaleChanged. This usually seems to happen after something triggeres + // ScaleChanged. This usually seems to happen after something triggers // the UI to re-layout, like hovering over the scrollbar. This event // doesn't reliably happen immediately after a scale change, so we can't // depend on it (despite the fact that both the scale and size state is From 1fb76ecbf9eb2fd70192107f1f399a343f92a9fc Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 10:13:28 -0500 Subject: [PATCH 21/25] Immediately tell the VT Engine about the new viewport size when we resize conpty --- src/host/screenInfo.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 75e16af1ade..ecd49d0c5a7 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1229,6 +1229,20 @@ void SCREEN_INFORMATION::_InternalSetViewportSize(const COORD* const pcoordSize, _viewport = newViewport; UpdateBottom(); Tracing::s_TraceWindowViewport(_viewport); + + // In Conpty mode, call TriggerScroll here without params. By not providing + // params, the renderer will make sure to update the VtEngine with the + // updated viewport size. If we don't do this, the engine can get into a + // torn state on this frame. + // + // Without this statement, the engine won't be told about the new view size + // till the start of the next frame. If any other text gets output before + // that frame starts, there's a very real chance that it'll cause errors as + // the engine tries to invalidate those regions. + if (gci.IsInVtIoMode() && ServiceLocator::LocateGlobals().pRender) + { + ServiceLocator::LocateGlobals().pRender->TriggerScroll(); + } } // Routine Description: From 0a6ad8fc09795533a024bacf65abefca59ee9172 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 15:24:22 -0500 Subject: [PATCH 22/25] fix the bitmap::all() function so we can actually use it (cherry picked from commit ba1a8a34292180cae10c135743ecd94b1da6f286) --- src/inc/til/bitmap.h | 26 ++++---------------------- src/renderer/dx/DxRenderer.cpp | 7 +------ 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index acafbaf1560..e7780c11e2b 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -135,7 +135,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz{}, _rc{}, _bits{}, - _dirty{}, _runs{} { } @@ -149,7 +148,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(_sz.area()), - _dirty(fill ? sz : til::rectangle{}), _runs{} { if (fill) @@ -162,7 +160,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return _sz == other._sz && _rc == other._rc && - _dirty == other._dirty && // dirty is before bits because it's a rough estimate of bits and a faster comparison. _bits == other._bits; // _runs excluded because it's a cache of generated state. } @@ -187,15 +184,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // If we don't have cached runs, rebuild. if (!_runs.has_value()) { - // If there's only one square dirty, quick save it off and be done. - if (one()) - { - _runs.emplace({ _dirty }); - } - else - { - _runs.emplace(begin(), end()); - } + _runs.emplace(begin(), end()); } // Return a reference to the runs. @@ -274,8 +263,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _runs.reset(); // reset cached runs on any non-const method _bits.set(_rc.index_of(pt)); - - _dirty |= til::rectangle{ pt }; } void set(const til::rectangle rc) @@ -287,22 +274,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { _bits.set(_rc.index_of(til::point{ rc.left(), row }), rc.width(), true); } - - _dirty |= rc; } void set_all() noexcept { _runs.reset(); // reset cached runs on any non-const method _bits.set(); - _dirty = _rc; } void reset_all() noexcept { _runs.reset(); // reset cached runs on any non-const method _bits.reset(); - _dirty = {}; } // True if we resized. False if it was the same size as before. @@ -360,7 +343,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" bool one() const { - return _dirty.size() == til::size{ 1, 1 }; + _bits.count() == 1; } constexpr bool any() const noexcept @@ -370,12 +353,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool none() const noexcept { - return _dirty.empty(); + return _bits.none(); } constexpr bool all() const noexcept { - return _dirty == _rc; + return _bits.all(); } constexpr til::size size() const noexcept @@ -399,7 +382,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: - til::rectangle _dirty; til::size _sz; til::rectangle _rc; dynamic_bitset<> _bits; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 432fd9c2e1c..654c71846c0 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1178,12 +1178,7 @@ try D2D1_COLOR_F nothing = { 0 }; // If the entire thing is invalid, just use one big clear operation. - // Don't use _invalidMap.all() here. That method checks if the union of the - // invalid areas of the map is the entire frame. However, there's a good - // chance that _not_ all the cells of the frame are actually invalid. If we - // used all() here, we'd clear the entire frame, but not actually paint all - // the cells again. - if (_firstFrame) + if (_invalidMap.all()) { _d2dRenderTarget->Clear(nothing); } From 2b1baa5289141698dfff111324bf8e0b66c93ae5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 15:30:09 -0500 Subject: [PATCH 23/25] maybe I should let it finish building before I commit (cherry picked from commit 98cdbeca349acb55c5b5aef1d96206a8fa7b62e4) --- src/inc/til/bitmap.h | 2 +- src/winconpty/winconpty.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index e7780c11e2b..ea3c024c8c2 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -343,7 +343,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" bool one() const { - _bits.count() == 1; + return _bits.count() == 1; } constexpr bool any() const noexcept diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp index ed250b55a6e..53952574cc8 100644 --- a/src/winconpty/winconpty.cpp +++ b/src/winconpty/winconpty.cpp @@ -83,7 +83,7 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken, RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files - const wchar_t* pwszFormat = L"\"%s\" --headless %s%s--width %hu --height %hu --signal 0x%x --server 0x%x"; + const wchar_t* pwszFormat = L"\"%s\" %s%s--width %hu --height %hu --signal 0x%x --server 0x%x"; // This is plenty of space to hold the formatted string wchar_t cmd[MAX_PATH]{}; const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR; From a23d3f270690f975d3cba325adca84260a84f601 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 16:12:39 -0500 Subject: [PATCH 24/25] Hey what's this doing here --- src/winconpty/winconpty.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp index 53952574cc8..ed250b55a6e 100644 --- a/src/winconpty/winconpty.cpp +++ b/src/winconpty/winconpty.cpp @@ -83,7 +83,7 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken, RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)); // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files - const wchar_t* pwszFormat = L"\"%s\" %s%s--width %hu --height %hu --signal 0x%x --server 0x%x"; + const wchar_t* pwszFormat = L"\"%s\" --headless %s%s--width %hu --height %hu --signal 0x%x --server 0x%x"; // This is plenty of space to hold the formatted string wchar_t cmd[MAX_PATH]{}; const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR; From c163f17bc6b8e92371020e2062dbc9859fb42539 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 16:12:52 -0500 Subject: [PATCH 25/25] Account for IME wrapping --- src/cascadia/TerminalControl/TSFInputControl.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index e1c467f5128..b025eee4a4e 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -224,9 +224,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Get the location of the cursor in the display, in pixels. til::point screenCursorPos{ scaledFrameOrigin + clientCursorPos }; + // GH #5007 - make sure to account for wrapping the IME composition at + // the right side of the viewport. + const ptrdiff_t textBlockHeight = ::base::ClampMul(_currentTextBlockHeight, scaleFactor); + // Get the bounds of the composition text, in pixels. - const til::rectangle textBounds{ til::point{ screenCursorPos.x() + fontSize.width(), screenCursorPos.y() }, - til::size{ 0, fontSize.height() } }; + const til::rectangle textBounds{ til::point{ screenCursorPos.x(), screenCursorPos.y() }, + til::size{ 0, textBlockHeight } }; _currentTextBounds = textBounds;