Skip to content

Commit

Permalink
wpf: fix a handful of issues with the wpf control (#6983)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@DHowett
DHowett authored Jul 20, 2020
1 parent c390b61 commit 76de2ae
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 19 deletions.
94 changes: 79 additions & 15 deletions src/cascadia/PublicTerminalCore/HwndTerminal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}
Expand All @@ -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);
Expand Down Expand Up @@ -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))
Expand All @@ -148,6 +174,11 @@ HwndTerminal::HwndTerminal(HWND parentHwnd) :
}
}

HwndTerminal::~HwndTerminal()
{
Teardown();
}

HRESULT HwndTerminal::Initialize()
{
_terminal = std::make_unique<::Microsoft::Terminal::Core::Terminal>();
Expand All @@ -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);
Expand All @@ -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();

Expand All @@ -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<void(int, int, int)> callback)
{
_terminal->SetScrollPositionChangedCallback(callback);
Expand Down Expand Up @@ -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<const HwndTerminal*>(terminal);
publicTerminal->_terminal->ClearSelection();
auto publicTerminal = static_cast<HwndTerminal*>(terminal);
publicTerminal->_ClearSelection();
}

bool _stdcall TerminalIsSelectionActive(void* terminal)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
Expand All @@ -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<const HwndTerminal*>(terminal);
auto publicTerminal = static_cast<HwndTerminal*>(terminal);

const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false);
publicTerminal->_ClearSelection();

// convert text: vector<string> --> string
std::wstring selectedText;
Expand All @@ -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();
}

Expand Down Expand Up @@ -574,7 +638,7 @@ try
{
if (_terminal->IsSelectionActive())
{
_terminal->ClearSelection();
_ClearSelection();
if (ch == UNICODE_ESC)
{
// ESC should clear any selection before it triggers input.
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/cascadia/PublicTerminalCore/HwndTerminal.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<void(int, int, int)> callback);
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/WpfTerminalControl/NativeMethods.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ public enum WindowMessage : int
/// </summary>
WM_CHAR = 0x0102,

/// <summary>
/// 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.
/// </summary>
WM_SYSKEYDOWN = 0x0104,

/// <summary>
/// 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.
/// </summary>
WM_SYSKEYUP = 0x0105,

/// <summary>
/// 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.
/// </summary>
Expand Down
12 changes: 9 additions & 3 deletions src/cascadia/WpfTerminalControl/TerminalContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,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
Expand All @@ -246,6 +247,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
Expand All @@ -255,9 +257,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))
Expand Down

0 comments on commit 76de2ae

Please sign in to comment.