maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Redraw TSFInputControl when Terminal cursor updates (#5135)

Browse source 
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This PR will allow TSFInputControl to redraw its Canvas and TextBlock in response to when the Terminal cursor position updates. This will fix the issue where during Korean composition, the first symbol of the next composition will appear on top of the previous composed character. Since the Terminal Cursor updates a lot, I've added some checks to see if the TSFInputControl really needs to redraw. This will also decrease the number of actual redraws since we receive a bunch of `LayoutRequested` events when there's no difference between them.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #4963
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Startup, teardown, CJK IME gibberish testing, making sure the IME block shows up in the right place.
This commit is contained in:
Leon Liang 2020-03-30 16:21:47 -07:00 committed by GitHub
parent 1ae4252a7b
commit ca6b54e652
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 152 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

128
src/cascadia/TerminalControl/TSFInputControl.cpp
View file

@ -19,7 +19,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
TSFInputControl::TSFInputControl() :
_editContext{ nullptr },
_inComposition{ false },
_activeTextStart{ 0 }
_activeTextStart{ 0 },
_focused{ false },
_currentTerminalCursorPos{ 0, 0 },
_currentCanvasWidth{ 0.0 },
_currentTextBlockHeight{ 0.0 },
_currentTextBounds{ 0, 0, 0, 0 },
_currentControlBounds{ 0, 0, 0, 0 }
{
InitializeComponent();
@ -58,7 +64,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
// Explicitly disconnect the LayoutRequested handler -- it can cause problems during application teardown.
// See GH#4159 for more info.
// Also disconnect compositionCompleted and textUpdating explicitly. It seems to occasionally cause problems if
// a composition is active during application teardown.
_layoutRequestedRevoker.revoke();
_compositionCompletedRevoker.revoke();
_textUpdatingRevoker.revoke();
}
// Method Description:
@ -73,6 +83,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (_editContext != nullptr)
{
_editContext.NotifyFocusEnter();
_focused = true;
}
}
@ -88,6 +99,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (_editContext != nullptr)
{
_editContext.NotifyFocusLeave();
_focused = false;
}
}
@ -114,28 +126,51 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
// Method Description:
// - Handler for LayoutRequested event by CoreEditContext responsible
// for returning the current position the IME should be placed
// in screen coordinates on the screen. TSFInputControls internal
// XAML controls (TextBlock/Canvas) are also positioned and updated.
// NOTE: documentation says application should handle this event
// - Redraw the canvas if certain dimensions have changed since the last
// redraw. This includes the Terminal cursor position, the Canvas width, and the TextBlock height.
// Arguments:
// - sender: CoreTextEditContext sending the request.
// - args: CoreTextLayoutRequestedEventArgs to be updated with position information.
// - <none>
// Return Value:
// - <none>
void TSFInputControl::_layoutRequestedHandler(CoreTextEditContext sender, CoreTextLayoutRequestedEventArgs const& args)
void TSFInputControl::TryRedrawCanvas()
{
auto request = args.Request();
// Get window in screen coordinates, this is the entire window including tabs
const auto windowBounds = CoreWindow::GetForCurrentThread().Bounds();
if (!_focused)
{
return;
}
// Get the cursor position in text buffer position
auto cursorArgs = winrt::make_self<CursorPositionEventArgs>();
_CurrentCursorPositionHandlers(*this, *cursorArgs);
const COORD cursorPos = { ::base::ClampedNumeric<short>(cursorArgs->CurrentPosition().X), ::base::ClampedNumeric<short>(cursorArgs->CurrentPosition().Y) };
const til::point cursorPos{ gsl::narrow_cast<ptrdiff_t>(cursorArgs->CurrentPosition().X), gsl::narrow_cast<ptrdiff_t>(cursorArgs->CurrentPosition().Y) };
const double actualCanvasWidth = Canvas().ActualWidth();
const double actualTextBlockHeight = TextBlock().ActualHeight();
if (_currentTerminalCursorPos == cursorPos &&
_currentCanvasWidth == actualCanvasWidth &&
_currentTextBlockHeight == actualTextBlockHeight)
{
return;
}
_currentTerminalCursorPos = cursorPos;
_currentCanvasWidth = actualCanvasWidth;
_currentTextBlockHeight = actualTextBlockHeight;
_RedrawCanvas();
}
// Method Description:
// - Redraw the Canvas and update the current Text Bounds and Control Bounds for
// the CoreTextEditContext.
// Arguments:
// - <none>
// Return Value:
// - <none>
void TSFInputControl::_RedrawCanvas()
{
// Get Font Info as we use this is the pixel size for characters in the display
auto fontArgs = winrt::make_self<FontInfoEventArgs>();
_CurrentFontInfoHandlers(*this, *fontArgs);
@ -145,8 +180,27 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// Convert text buffer cursor position to client coordinate position within the window
COORD clientCursorPos;
clientCursorPos.X = ::base::ClampMul(cursorPos.X, ::base::ClampedNumeric<short>(fontWidth));
clientCursorPos.Y = ::base::ClampMul(cursorPos.Y, ::base::ClampedNumeric<short>(fontHeight));
clientCursorPos.X = ::base::ClampMul(_currentTerminalCursorPos.x(), ::base::ClampedNumeric<ptrdiff_t>(fontWidth));
clientCursorPos.Y = ::base::ClampMul(_currentTerminalCursorPos.y(), ::base::ClampedNumeric<ptrdiff_t>(fontHeight));
// position textblock to cursor position
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;
TextBlock().FontSize(fontSizePx);
TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace()));
const auto widthToTerminalEnd = _currentCanvasWidth - ::base::ClampedNumeric<double>(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
const auto newMaxWidth = std::max<double>(0.0, widthToTerminalEnd);
TextBlock().MaxWidth(newMaxWidth);
// Get window in screen coordinates, this is the entire window including tabs
const auto windowBounds = CoreWindow::GetForCurrentThread().Bounds();
// Convert from client coordinate to screen coordinate by adding window position
COORD screenCursorPos;
@ -162,33 +216,35 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// Get scale factor for view
const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel();
const auto yOffset = ::base::ClampedNumeric<float>(_currentTextBlockHeight) - fontHeight;
const auto textBottom = ::base::ClampedNumeric<float>(screenCursorPos.Y) + yOffset;
// position textblock to cursor position
Canvas().SetLeft(TextBlock(), clientCursorPos.X);
Canvas().SetTop(TextBlock(), ::base::ClampedNumeric<double>(clientCursorPos.Y));
_currentTextBounds = ScaleRect(Rect(screenCursorPos.X, textBottom, 0, fontHeight), scaleFactor);
_currentControlBounds = ScaleRect(Rect(screenCursorPos.X, screenCursorPos.Y, 0, fontHeight), scaleFactor);
}
// calculate FontSize in pixels from DIPs
const double fontSizePx = (fontHeight * 72) / USER_DEFAULT_SCREEN_DPI;
TextBlock().FontSize(fontSizePx);
TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace()));
// Method Description:
// - Handler for LayoutRequested event by CoreEditContext responsible
// for returning the current position the IME should be placed
// in screen coordinates on the screen. TSFInputControls internal
// XAML controls (TextBlock/Canvas) are also positioned and updated.
// NOTE: documentation says application should handle this event
// Arguments:
// - sender: CoreTextEditContext sending the request.
// - args: CoreTextLayoutRequestedEventArgs to be updated with position information.
// Return Value:
// - <none>
void TSFInputControl::_layoutRequestedHandler(CoreTextEditContext sender, CoreTextLayoutRequestedEventArgs const& args)
{
auto request = args.Request();
const auto canvasActualWidth = Canvas().ActualWidth();
const auto widthToTerminalEnd = canvasActualWidth - ::base::ClampedNumeric<double>(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
const auto newMaxWidth = std::max<double>(0.0, widthToTerminalEnd);
TextBlock().MaxWidth(newMaxWidth);
TryRedrawCanvas();
// Set the text block bounds
const auto yOffset = ::base::ClampedNumeric<float>(TextBlock().ActualHeight()) - fontHeight;
const auto textBottom = ::base::ClampedNumeric<float>(screenCursorPos.Y) + yOffset;
Rect selectionRect = Rect(screenCursorPos.X, textBottom, 0, fontHeight);
request.LayoutBounds().TextBounds(ScaleRect(selectionRect, scaleFactor));
request.LayoutBounds().TextBounds(_currentTextBounds);
// Set the control bounds of the whole control
Rect controlRect = Rect(screenCursorPos.X, screenCursorPos.Y, 0, fontHeight);
request.LayoutBounds().ControlBounds(ScaleRect(controlRect, scaleFactor));
request.LayoutBounds().ControlBounds(_currentControlBounds);
}
// Method Description:

9
src/cascadia/TerminalControl/TSFInputControl.h
View file

@ -37,6 +37,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void NotifyFocusEnter();
void NotifyFocusLeave();
void ClearBuffer();
void TryRedrawCanvas();
void Close();
@ -73,6 +74,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
bool _inComposition;
size_t _activeTextStart;
void _SendAndClearText();
void _RedrawCanvas();
bool _focused;
til::point _currentTerminalCursorPos;
double _currentCanvasWidth;
double _currentTextBlockHeight;
winrt::Windows::Foundation::Rect _currentControlBounds;
winrt::Windows::Foundation::Rect _currentTextBounds;
};
}
namespace winrt::Microsoft::Terminal::TerminalControl::factory_implementation

1
src/cascadia/TerminalControl/TSFInputControl.idl
View file

@ -28,6 +28,7 @@ namespace Microsoft.Terminal.TerminalControl
void NotifyFocusEnter();
void NotifyFocusLeave();
void ClearBuffer();
void TryRedrawCanvas();
void Close();
}

32
src/cascadia/TerminalControl/TermControl.cpp
View file

@ -86,6 +86,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
auto pfnScrollPositionChanged = std::bind(&TermControl::_TerminalScrollPositionChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3);
_terminal->SetScrollPositionChangedCallback(pfnScrollPositionChanged);
auto pfnTerminalCursorPositionChanged = std::bind(&TermControl::_TerminalCursorPositionChanged, this);
_terminal->SetCursorPositionChangedCallback(pfnTerminalCursorPositionChanged);
// This event is explicitly revoked in the destructor: does not need weak_ref
auto onReceiveOutputFn = [this](const hstring str) {
_terminal->Write(str);
@ -1793,6 +1796,31 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
}
// Method Description:
// - Tells TSFInputControl to redraw the Canvas/TextBlock so it'll update
// to be where the current cursor position is.
// Arguments:
// - N/A
winrt::fire_and_forget TermControl::_TerminalCursorPositionChanged()
{
if (_closing.load())
{
return;
}
auto weakThis{ get_weak() };
co_await winrt::resume_foreground(Dispatcher());
if (auto control{ weakThis.get() })
{
if (!_closing.load())
{
TSFInputControl().TryRedrawCanvas();
}
}
}
hstring TermControl::Title()
{
hstring hstr{ _terminal->GetConsoleTitle() };
@ -2243,8 +2271,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
return;
}
const COORD cursorPos = _terminal->GetCursorPosition();
Windows::Foundation::Point p = { gsl::narrow_cast<float>(cursorPos.X), gsl::narrow_cast<float>(cursorPos.Y) };
const til::point cursorPos = _terminal->GetCursorPosition();
Windows::Foundation::Point p = { ::base::ClampedNumeric<float>(cursorPos.x()), ::base::ClampedNumeric<float>(cursorPos.y()) };
eventArgs.CurrentPosition(p);
}

1
src/cascadia/TerminalControl/TermControl.h
View file

@ -196,6 +196,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void _DoResize(const double newWidth, const double newHeight);
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();
void _MouseScrollHandler(const double delta, Windows::UI::Input::PointerPoint const& pointerPoint);
void _MouseZoomHandler(const double delta);

15
src/cascadia/TerminalCore/Terminal.cpp
View file

@ -704,6 +704,8 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
_buffer->GetRenderTarget().TriggerRedrawAll();
_NotifyScrollEvent();
}
_NotifyTerminalCursorPositionChanged();
}
void Terminal::UserScrollViewport(const int viewTop)
@ -736,6 +738,14 @@ try
}
CATCH_LOG()
void Terminal::_NotifyTerminalCursorPositionChanged() noexcept
{
if (_pfnCursorPositionChanged)
{
_pfnCursorPositionChanged();
}
}
void Terminal::SetWriteInputCallback(std::function<void(std::wstring&)> pfn) noexcept
{
_pfnWriteInput.swap(pfn);
@ -751,6 +761,11 @@ void Terminal::SetScrollPositionChangedCallback(std::function<void(const int, co
_pfnScrollPositionChanged.swap(pfn);
}
void Terminal::SetCursorPositionChangedCallback(std::function<void()> pfn) noexcept
{
_pfnCursorPositionChanged.swap(pfn);
}
// Method Description:
// - Allows setting a callback for when the background color is changed
// Arguments:

4
src/cascadia/TerminalCore/Terminal.hpp
View file

@ -168,6 +168,7 @@ public:
void SetWriteInputCallback(std::function<void(std::wstring&)> pfn) noexcept;
void SetTitleChangedCallback(std::function<void(const std::wstring_view&)> pfn) noexcept;
void SetScrollPositionChangedCallback(std::function<void(const int, const int, const int)> pfn) noexcept;
void SetCursorPositionChangedCallback(std::function<void()> pfn) noexcept;
void SetBackgroundCallback(std::function<void(const uint32_t)> pfn) noexcept;
void SetCursorOn(const bool isOn) noexcept;
@ -194,6 +195,7 @@ private:
std::function<void(const std::wstring_view&)> _pfnTitleChanged;
std::function<void(const int, const int, const int)> _pfnScrollPositionChanged;
std::function<void(const uint32_t)> _pfnBackgroundColorChanged;
std::function<void()> _pfnCursorPositionChanged;
std::unique_ptr<::Microsoft::Console::VirtualTerminal::StateMachine> _stateMachine;
std::unique_ptr<::Microsoft::Console::VirtualTerminal::TerminalInput> _terminalInput;
@ -266,6 +268,8 @@ private:
void _NotifyScrollEvent() noexcept;
void _NotifyTerminalCursorPositionChanged() noexcept;
#pragma region TextSelection
// These methods are defined in TerminalSelection.cpp
std::vector<SMALL_RECT> _GetSelectionRects() const noexcept;