From 30956d7f6629d0b9635fbb1b7c475fcefe2e062b Mon Sep 17 00:00:00 2001 From: skyline75489 Date: Sat, 12 Oct 2019 16:10:26 +0800 Subject: [PATCH 1/2] Added read lock and fix various cursor pos issue --- src/cascadia/TerminalCore/Terminal.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 2d7dd5d424d..93e62947f41 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -178,6 +178,21 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting proposedTop -= (proposedBottom - bufferSize.Y); } + Cursor& cursor = _buffer->GetCursor(); + auto cursorPosition = cursor.GetPosition(); + if (cursorPosition.X > viewportSize.X) + { + cursorPosition.X = viewportSize.X - 1; + } + if (cursorPosition.Y > viewportSize.Y) + { + cursorPosition.Y = viewportSize.Y; + } + + cursor.StartDeferDrawing(); + cursor.SetPosition(cursorPosition); + cursor.EndDeferDrawing(); + _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); _scrollOffset = 0; _NotifyScrollEvent(); @@ -425,6 +440,11 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView) } } + if (proposedCursorPosition.X > bufferSize.RightInclusive()) + { + proposedCursorPosition.X = 0; + } + // If we're about to scroll past the bottom of the buffer, instead cycle the buffer. const auto newRows = proposedCursorPosition.Y - bufferSize.Height() + 1; if (newRows > 0) @@ -533,6 +553,8 @@ void Terminal::_InitializeColorTable() // - isVisible: whether the cursor should be visible void Terminal::SetCursorVisible(const bool isVisible) noexcept { + auto lock = LockForReading(); + auto& cursor = _buffer->GetCursor(); cursor.SetIsVisible(isVisible); } From 941b377e063139694f87cd593c6d7bcdb49b3a6f Mon Sep 17 00:00:00 2001 From: skyline75489 Date: Fri, 18 Oct 2019 10:35:47 +0800 Subject: [PATCH 2/2] Changes for review --- src/cascadia/TerminalCore/Terminal.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 93e62947f41..d58ca2fb032 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -180,14 +180,11 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting Cursor& cursor = _buffer->GetCursor(); auto cursorPosition = cursor.GetPosition(); - if (cursorPosition.X > viewportSize.X) - { - cursorPosition.X = viewportSize.X - 1; - } - if (cursorPosition.Y > viewportSize.Y) - { - cursorPosition.Y = viewportSize.Y; - } + + // If the cursor is positioned beyond the range of the viewport, then + // set the cursor position to a legal value. + cursorPosition.X = std::min(cursorPosition.X, static_cast(viewportSize.X - 1)); + cursorPosition.Y = std::min(cursorPosition.Y, static_cast(viewportSize.Y - 1)); cursor.StartDeferDrawing(); cursor.SetPosition(cursorPosition); @@ -553,7 +550,7 @@ void Terminal::_InitializeColorTable() // - isVisible: whether the cursor should be visible void Terminal::SetCursorVisible(const bool isVisible) noexcept { - auto lock = LockForReading(); + auto lock = LockForWriting(); auto& cursor = _buffer->GetCursor(); cursor.SetIsVisible(isVisible);