Skip to content

Commit

Permalink
Debounce window state changes caused by the PTY (#13147)
Browse files Browse the repository at this point in the history
Use a throttled update to update our window state. Throttling should prevent scenarios where the Terminal window state and PTY window state get de-sync'd, and cause the window to minimize/restore constantly in a loop.  "Should" is doing a lot of work in this sentence.

A 200ms delay was chosen because it's the typical animation timeout in Windows. This does result in a delay between the PTY requesting a change to the window state and the Terminal realizing it, but should mitigate issues where the Terminal and PTY get desync'd.

I think we're overall not super confident that this fixes the root causes of the issue. Rather, we're hopeful that a small amount of throttling here should leave time for the Terminal and pty to sync back up. We're comfortable enough with that as a bandaid for 1.14 preview, to see how this behaves in the wild.

(cherry picked from commit c5fad74)
Service-Card-Id: 82315892
Service-Version: 1.14
  • Loading branch information
zadjii-msft authored and DHowett committed May 23, 2022
1 parent c0e6c25 commit 39c71d6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
24 changes: 23 additions & 1 deletion src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,19 @@ AppHost::AppHost() noexcept :
{
_BecomeMonarch(nullptr, nullptr);
}

// Create a throttled function for updating the window state, to match the
// one requested by the pty. A 200ms delay was chosen because it's the
// typical animation timeout in Windows. This does result in a delay between
// the PTY requesting a change to the window state and the Terminal
// realizing it, but should mitigate issues where the Terminal and PTY get
// de-sync'd.
_showHideWindowThrottler = std::make_shared<ThrottledFuncTrailing<bool>>(
winrt::Windows::System::DispatcherQueue::GetForCurrentThread(),
std::chrono::milliseconds(200),
[this](const bool show) {
_window->ShowWindowChanged(show);
});
}

AppHost::~AppHost()
Expand All @@ -129,6 +142,8 @@ AppHost::~AppHost()
// out the window, then close the app.
_revokers = {};

_showHideWindowThrottler.reset();

_window = nullptr;
_app.Close();
_app = nullptr;
Expand Down Expand Up @@ -1397,7 +1412,14 @@ void AppHost::_QuitAllRequested(const winrt::Windows::Foundation::IInspectable&,
void AppHost::_ShowWindowChanged(const winrt::Windows::Foundation::IInspectable&,
const winrt::Microsoft::Terminal::Control::ShowWindowArgs& args)
{
_window->ShowWindowChanged(args.ShowOrHide());
// GH#13147: Enqueue a throttled update to our window state. Throttling
// should prevent scenarios where the Terminal window state and PTY window
// state get de-sync'd, and cause the window to minimize/restore constantly
// in a loop.
if (_showHideWindowThrottler)
{
_showHideWindowThrottler->Run(args.ShowOrHide());
}
}

void AppHost::_SummonWindowRequested(const winrt::Windows::Foundation::IInspectable& sender,
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "pch.h"
#include "NonClientIslandWindow.h"
#include "NotificationIcon.h"
#include <til/throttled_func.h>
#include <ThrottledFunc.h>

class AppHost
{
Expand Down Expand Up @@ -33,6 +33,7 @@ class AppHost
bool _useNonClientArea{ false };

std::optional<til::throttled_func_trailing<>> _getWindowLayoutThrottler;
std::shared_ptr<ThrottledFuncTrailing<bool>> _showHideWindowThrottler;
winrt::Windows::Foundation::IAsyncAction _SaveWindowLayouts();
winrt::fire_and_forget _SaveWindowLayoutsRepeat();

Expand Down

0 comments on commit 39c71d6

Please sign in to comment.