From fda80eaf64b6f4bb41f248f5c6b355d672ad64f8 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Mon, 20 Jul 2020 16:13:20 -0700 Subject: [PATCH] wpf: fix a handful of issues with the wpf control (#6983) * send alt/F10 through the control We were not listening for WM_SYSKEY{UP,DOWN} * extract the actual scancode during WM_CHAR, not the bitfield We were accidentally sending some of the additional keypress data in with the character event in Win32 Input Mode * set default fg/bg to campbell The WPF control starts up in PowerShell blue even though it's not typically used in PowerShell blue. * don't rely on the font to determine wideness This is a cross-port of #2928 to the WPF control * deterministic shutdown In testing, I saw a handful of crashes on teardown because we were not shutting down the render thread properly. * don't pass 10 for the font weight ... When Cascadia Code is set, it just looks silly. * trigger render when selection is cleared, do it under lock Fixes #6966. (cherry picked from commit 76de2aedc2d805481ae323bba6f34cb41878a461) --- .../PublicTerminalCore/HwndTerminal.cpp | 94 ++++++++++++++++--- .../PublicTerminalCore/HwndTerminal.hpp | 5 +- .../WpfTerminalControl/NativeMethods.cs | 10 ++ .../WpfTerminalControl/TerminalContainer.cs | 12 ++- 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index cb31e1860f3..669cfbb9be6 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -24,6 +24,25 @@ static constexpr bool _IsMouseMessage(UINT uMsg) uMsg == WM_MOUSEMOVE || uMsg == WM_MOUSEWHEEL; } +// Helper static function to ensure that all ambiguous-width glyphs are reported as narrow. +// See microsoft/terminal#2066 for more info. +static bool _IsGlyphWideForceNarrowFallback(const std::wstring_view /* glyph */) noexcept +{ + return false; // glyph is not wide. +} + +static bool _EnsureStaticInitialization() +{ + // use C++11 magic statics to make sure we only do this once. + static bool initialized = []() { + // *** THIS IS A SINGLETON *** + SetGlyphWidthFallback(_IsGlyphWideForceNarrowFallback); + + return true; + }(); + return initialized; +} + LRESULT CALLBACK HwndTerminal::HwndTerminalWndProc( HWND hwnd, UINT uMsg, @@ -72,7 +91,7 @@ try { const auto bufferData = terminal->_terminal->RetrieveSelectedTextFromBuffer(false); LOG_IF_FAILED(terminal->_CopyTextToSystemClipboard(bufferData, true)); - terminal->_terminal->ClearSelection(); + TerminalClearSelection(terminal); } CATCH_LOG(); } @@ -81,6 +100,11 @@ try terminal->_PasteTextFromClipboard(); } return 0; + case WM_DESTROY: + // Release Terminal's hwnd so Teardown doesn't try to destroy it again + terminal->_hwnd.release(); + terminal->Teardown(); + return 0; } } return DefWindowProc(hwnd, uMsg, wParam, lParam); @@ -114,14 +138,16 @@ static bool RegisterTermClass(HINSTANCE hInstance) noexcept } HwndTerminal::HwndTerminal(HWND parentHwnd) : - _desiredFont{ L"Consolas", 0, 10, { 0, 14 }, CP_UTF8 }, - _actualFont{ L"Consolas", 0, 10, { 0, 14 }, CP_UTF8, false }, + _desiredFont{ L"Consolas", 0, DEFAULT_FONT_WEIGHT, { 0, 14 }, CP_UTF8 }, + _actualFont{ L"Consolas", 0, DEFAULT_FONT_WEIGHT, { 0, 14 }, CP_UTF8, false }, _uiaProvider{ nullptr }, _uiaProviderInitialized{ false }, _currentDpi{ USER_DEFAULT_SCREEN_DPI }, _pfnWriteCallback{ nullptr }, _multiClickTime{ 500 } // this will be overwritten by the windows system double-click time { + _EnsureStaticInitialization(); + HINSTANCE hInstance = wil::GetModuleInstanceHandle(); if (RegisterTermClass(hInstance)) @@ -148,6 +174,11 @@ HwndTerminal::HwndTerminal(HWND parentHwnd) : } } +HwndTerminal::~HwndTerminal() +{ + Teardown(); +} + HRESULT HwndTerminal::Initialize() { _terminal = std::make_unique<::Microsoft::Terminal::Core::Terminal>(); @@ -162,9 +193,6 @@ HRESULT HwndTerminal::Initialize() RETURN_IF_FAILED(dxEngine->Enable()); _renderer->AddRenderEngine(dxEngine.get()); - const auto pfn = std::bind(&::Microsoft::Console::Render::Renderer::IsGlyphWideByFont, _renderer.get(), std::placeholders::_1); - SetGlyphWidthFallback(pfn); - _UpdateFont(USER_DEFAULT_SCREEN_DPI); RECT windowRect; GetWindowRect(_hwnd.get(), &windowRect); @@ -181,8 +209,8 @@ HRESULT HwndTerminal::Initialize() _terminal->SetBackgroundCallback([](auto) {}); _terminal->Create(COORD{ 80, 25 }, 1000, *_renderer); - _terminal->SetDefaultBackground(RGB(5, 27, 80)); - _terminal->SetDefaultForeground(RGB(255, 255, 255)); + _terminal->SetDefaultBackground(RGB(12, 12, 12)); + _terminal->SetDefaultForeground(RGB(204, 204, 204)); _terminal->SetWriteInputCallback([=](std::wstring & input) noexcept { _WriteTextToConnection(input); }); localPointerToThread->EnablePainting(); @@ -191,6 +219,33 @@ HRESULT HwndTerminal::Initialize() return S_OK; } +void HwndTerminal::Teardown() noexcept +try +{ + // As a rule, detach resources from the Terminal before shutting them down. + // This ensures that teardown is reentrant. + + // Shut down the renderer (and therefore the thread) before we implode + if (auto localRenderEngine{ std::exchange(_renderEngine, nullptr) }) + { + if (auto localRenderer{ std::exchange(_renderer, nullptr) }) + { + localRenderer->TriggerTeardown(); + // renderer is destroyed + } + // renderEngine is destroyed + } + + if (auto localHwnd{ _hwnd.release() }) + { + // If we're being called through WM_DESTROY, we won't get here (hwnd is already released) + // If we're not, we may end up in Teardown _again_... but by the time we do, all other + // resources have been released and will not be released again. + DestroyWindow(localHwnd); + } +} +CATCH_LOG(); + void HwndTerminal::RegisterScrollCallback(std::function callback) { _terminal->SetScrollPositionChangedCallback(callback); @@ -467,11 +522,21 @@ try } CATCH_RETURN(); +void HwndTerminal::_ClearSelection() noexcept +try +{ + auto lock{ _terminal->LockForWriting() }; + _terminal->ClearSelection(); + _renderer->TriggerSelection(); +} +CATCH_LOG(); + void _stdcall TerminalClearSelection(void* terminal) { - const auto publicTerminal = static_cast(terminal); - publicTerminal->_terminal->ClearSelection(); + auto publicTerminal = static_cast(terminal); + publicTerminal->_ClearSelection(); } + bool _stdcall TerminalIsSelectionActive(void* terminal) { const auto publicTerminal = static_cast(terminal); @@ -482,9 +547,10 @@ bool _stdcall TerminalIsSelectionActive(void* terminal) // Returns the selected text in the terminal. const wchar_t* _stdcall TerminalGetSelection(void* terminal) { - const auto publicTerminal = static_cast(terminal); + auto publicTerminal = static_cast(terminal); const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false); + publicTerminal->_ClearSelection(); // convert text: vector --> string std::wstring selectedText; @@ -494,8 +560,6 @@ const wchar_t* _stdcall TerminalGetSelection(void* terminal) } auto returnText = wil::make_cotaskmem_string_nothrow(selectedText.c_str()); - TerminalClearSelection(terminal); - return returnText.release(); } @@ -574,7 +638,7 @@ try { if (_terminal->IsSelectionActive()) { - _terminal->ClearSelection(); + _ClearSelection(); if (ch == UNICODE_ESC) { // ESC should clear any selection before it triggers input. @@ -632,7 +696,7 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font publicTerminal->_terminal->SetCursorStyle(theme.CursorStyle); - publicTerminal->_desiredFont = { fontFamily, 0, 10, { 0, fontSize }, CP_UTF8 }; + publicTerminal->_desiredFont = { fontFamily, 0, DEFAULT_FONT_WEIGHT, { 0, fontSize }, CP_UTF8 }; publicTerminal->_UpdateFont(newDpi); // When the font changes the terminal dimensions need to be recalculated since the available row and column diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.hpp b/src/cascadia/PublicTerminalCore/HwndTerminal.hpp index 90b4cc50ac8..f904f526cad 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.hpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.hpp @@ -51,9 +51,10 @@ struct HwndTerminal : ::Microsoft::Console::Types::IControlAccessibilityInfo HwndTerminal(HwndTerminal&&) = default; HwndTerminal& operator=(const HwndTerminal&) = default; HwndTerminal& operator=(HwndTerminal&&) = default; - ~HwndTerminal() = default; + ~HwndTerminal(); HRESULT Initialize(); + void Teardown() noexcept; void SendOutput(std::wstring_view data); HRESULT Refresh(const SIZE windowSize, _Out_ COORD* dimensions); void RegisterScrollCallback(std::function callback); @@ -112,6 +113,8 @@ struct HwndTerminal : ::Microsoft::Console::Types::IControlAccessibilityInfo HRESULT _MoveSelection(LPARAM lParam) noexcept; IRawElementProviderSimple* _GetUiaProvider() noexcept; + void _ClearSelection() noexcept; + bool _CanSendVTMouseInput() const noexcept; bool _SendMouseEvent(UINT uMsg, WPARAM wParam, LPARAM lParam) noexcept; diff --git a/src/cascadia/WpfTerminalControl/NativeMethods.cs b/src/cascadia/WpfTerminalControl/NativeMethods.cs index dd1731ed96e..86ab42742ac 100644 --- a/src/cascadia/WpfTerminalControl/NativeMethods.cs +++ b/src/cascadia/WpfTerminalControl/NativeMethods.cs @@ -59,6 +59,16 @@ public enum WindowMessage : int /// WM_CHAR = 0x0102, + /// + /// The WM_SYSKEYDOWN message is posted to the window with the keyboard focus when a system key is pressed. A system key is F10 or Alt+Something. + /// + WM_SYSKEYDOWN = 0x0104, + + /// + /// The WM_SYSKEYDOWN message is posted to the window with the keyboard focus when a system key is released. A system key is F10 or Alt+Something. + /// + WM_SYSKEYUP = 0x0105, + /// /// The WM_MOUSEMOVE message is posted to a window when the cursor moves. If the mouse is not captured, the message is posted to the window that contains the cursor. Otherwise, the message is posted to the window that has captured the mouse. /// diff --git a/src/cascadia/WpfTerminalControl/TerminalContainer.cs b/src/cascadia/WpfTerminalControl/TerminalContainer.cs index 425378a49ac..8dea6bee30a 100644 --- a/src/cascadia/WpfTerminalControl/TerminalContainer.cs +++ b/src/cascadia/WpfTerminalControl/TerminalContainer.cs @@ -232,6 +232,7 @@ private IntPtr TerminalContainer_MessageHook(IntPtr hwnd, int msg, IntPtr wParam this.Focus(); NativeMethods.SetFocus(this.hwnd); break; + case NativeMethods.WindowMessage.WM_SYSKEYDOWN: // fallthrough case NativeMethods.WindowMessage.WM_KEYDOWN: { // WM_KEYDOWN lParam layout documentation: https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-keydown @@ -243,6 +244,7 @@ private IntPtr TerminalContainer_MessageHook(IntPtr hwnd, int msg, IntPtr wParam break; } + case NativeMethods.WindowMessage.WM_SYSKEYUP: // fallthrough case NativeMethods.WindowMessage.WM_KEYUP: { // WM_KEYUP lParam layout documentation: https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-keyup @@ -252,9 +254,13 @@ private IntPtr TerminalContainer_MessageHook(IntPtr hwnd, int msg, IntPtr wParam } case NativeMethods.WindowMessage.WM_CHAR: - // WM_CHAR lParam layout documentation: https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-char - NativeMethods.TerminalSendCharEvent(this.terminal, (char)wParam, (ushort)((uint)lParam >> 16)); - break; + { + // WM_CHAR lParam layout documentation: https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-char + ulong scanCode = (((ulong)lParam) & 0x00FF0000) >> 16; + NativeMethods.TerminalSendCharEvent(this.terminal, (char)wParam, (ushort)scanCode); + break; + } + case NativeMethods.WindowMessage.WM_WINDOWPOSCHANGED: var windowpos = (NativeMethods.WINDOWPOS)Marshal.PtrToStructure(lParam, typeof(NativeMethods.WINDOWPOS)); if (((NativeMethods.SetWindowPosFlags)windowpos.flags).HasFlag(NativeMethods.SetWindowPosFlags.SWP_NOSIZE))