Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call UpdatePatternLocations from a background thread #13758

Merged
1 commit merged into from
Aug 16, 2022
Merged

Call UpdatePatternLocations from a background thread #13758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/cascadia/TerminalControl/ControlCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
});

_updatePatternLocations = std::make_shared<ThrottledFuncTrailing<>>(
_dispatcher,
// NOTE: Calling UpdatePatternLocations from a background
// thread is a workaround for us to hit GH#12607 less often.
_updatePatternLocations = std::make_unique<til::throttled_func_trailing<>>(
UpdatePatternLocationsInterval,
[weakThis = get_weak()]() {
if (auto core{ weakThis.get() }; !core->_IsClosing())
if (auto core{ weakThis.get() })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we be sure now that the control isn't tearing down?

Copy link
Member Author

@lhecker lhecker Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We simply can't. After all, _IsClosing isn't a mutex which would prevent the main thread from closing under us while we're running on the background thread.
Thankfully, Terminal::_InvalidatePatternTree doesn't seem to depend on any UI state.

{
core->UpdatePatternLocations();
}
Expand Down Expand Up @@ -509,7 +510,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// itself - it was initiated by the mouse wheel, or the scrollbar.
_terminal->UserScrollViewport(viewTop);

_updatePatternLocations->Run();
(*_updatePatternLocations)();
}

void ControlCore::AdjustOpacity(const double adjustment)
Expand Down Expand Up @@ -1311,7 +1312,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

// Additionally, start the throttled update of where our links are.
_updatePatternLocations->Run();
(*_updatePatternLocations)();
}

void ControlCore::_terminalCursorPositionChanged()
Expand Down Expand Up @@ -1705,7 +1706,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->Write(hstr);

// Start the throttled update of where our hyperlinks are.
_updatePatternLocations->Run();
(*_updatePatternLocations)();
}
catch (...)
{
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
std::shared_ptr<ThrottledFuncTrailing<>> _tsfTryRedrawCanvas;
std::shared_ptr<ThrottledFuncTrailing<>> _updatePatternLocations;
std::unique_ptr<til::throttled_func_trailing<>> _updatePatternLocations;
std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> _updateScrollBar;

winrt::fire_and_forget _asyncCloseConnection();
Expand Down