Throttle scrollbar updates in TermControl to ~one per 8ms (#4608)
In addition to the below (original) description, this commit introduces a ThrottledFunc template that can throttle _any_ function. It applies that type to muffle updates to the scrollbar. --- Redo #3531 but without the bug that it caused (#3622) which is why it was reverted. I'm sorry if I explain this badly. If you don't understand a part, make sure to let me know and I will explain it better. ### Explanation How it worked before: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it dispatches work for later to be ran the UI thread to updates the scrollbar's values Why it's bad: * If we have many viewport changes, it will create a long stack of operations to run. Instead, we should just update the scroll bar with the most recent information that we know. * Imagine if the rate that the work gets pushed on the UI thread is greater than the rate that it can handle: it might freeze? * No need to be real time, we can wait just a little bit (8ms) to accumulate viewport changes before we actually change the scroll bar's value because it appears to be expensive (see perf below). Now: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it tells the `ScrollBarUpdater` about a new update -> the `ScrollBarUpdater` only runs one job (I don't know if that's the right term) on the UI thread at a time. If a job is already running but hasn't updated the scroll bar yet, it changes the setting in the already existing job to update the scroll bar with the new values. A job "waits" some time before doing the update to throttle updates because we don't need real time scroll bar updates. -> eventually, it updates the scroll bar If the user scrolls when a scroll bar update is pending, we keep the scroll bar's Maximum and Minimum but let the user choose its new Value with the `CancelPendingValueChange` method. ### Note Also I changed a little bit the code from the Terminal to notify the TermControl less often when possible. I tried to scroll with the scroll bar, with the mouse wheel. I tried to scroll while content is being outputted. I tried to reproduce the crash from #2248 without success (good). Co-authored-by: Leonard Hecker <leonard@hecker.io> Closes #3622
This commit is contained in:
greg904
GitHub
parent
e8ece1645c
commit
25df527743
|
|
@ -7,6 +7,7 @@ EXPCMDFLAGS
|
|||
EXPCMDSTATE
|
||||
fullkbd
|
||||
href
|
||||
IAsync
|
||||
IBox
|
||||
IBind
|
||||
IClass
|
||||
|
|
|
|||
|
|
@ -27,6 +27,10 @@ using namespace winrt::Windows::System;
|
|||
using namespace winrt::Microsoft::Terminal::Settings;
|
||||
using namespace winrt::Windows::ApplicationModel::DataTransfer;
|
||||
|
||||
// The minimum delay between updates to the scroll bar's values.
|
||||
// The updates are throttled to limit power usage.
|
||||
constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
|
||||
|
||||
namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
||||
{
|
||||
// Helper static function to ensure that all ambiguous-width glyphs are reported as narrow.
|
||||
|
|
@ -58,7 +62,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
_initializedTerminal{ false },
|
||||
_settings{ settings },
|
||||
_closing{ false },
|
||||
_isTerminalInitiatedScroll{ false },
|
||||
_isInternalScrollBarUpdate{ false },
|
||||
_autoScrollVelocity{ 0 },
|
||||
_autoScrollingPointerPoint{ std::nullopt },
|
||||
_autoScrollTimer{},
|
||||
|
|
@ -120,6 +124,32 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
}
|
||||
});
|
||||
|
||||
_updateScrollBar = std::make_shared<ThrottledFunc<ScrollBarUpdate>>(
|
||||
[weakThis = get_weak()](const auto& update) {
|
||||
if (auto control{ weakThis.get() })
|
||||
{
|
||||
control->Dispatcher()
|
||||
.RunAsync(CoreDispatcherPriority::Normal, [=]() {
|
||||
if (auto control2{ weakThis.get() })
|
||||
{
|
||||
control2->_isInternalScrollBarUpdate = true;
|
||||
|
||||
auto scrollBar = control2->ScrollBar();
|
||||
if (update.newValue.has_value())
|
||||
{
|
||||
scrollBar.Value(update.newValue.value());
|
||||
}
|
||||
scrollBar.Maximum(update.newMaximum);
|
||||
scrollBar.Minimum(update.newMinimum);
|
||||
scrollBar.ViewportSize(update.newViewportSize);
|
||||
|
||||
control2->_isInternalScrollBarUpdate = false;
|
||||
}
|
||||
});
|
||||
}
|
||||
},
|
||||
ScrollBarUpdateInterval);
|
||||
|
||||
static constexpr auto AutoScrollUpdateInterval = std::chrono::microseconds(static_cast<int>(1.0 / 30.0 * 1000000));
|
||||
_autoScrollTimer.Interval(AutoScrollUpdateInterval);
|
||||
_autoScrollTimer.Tick({ this, &TermControl::_UpdateAutoScroll });
|
||||
|
|
@ -214,7 +244,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
{
|
||||
if (_closing)
|
||||
{
|
||||
return;
|
||||
co_return;
|
||||
}
|
||||
|
||||
// Update our control settings
|
||||
|
|
@ -1437,8 +1467,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
void TermControl::_ScrollbarChangeHandler(Windows::Foundation::IInspectable const& /*sender*/,
|
||||
Controls::Primitives::RangeBaseValueChangedEventArgs const& args)
|
||||
{
|
||||
if (_isTerminalInitiatedScroll || _closing)
|
||||
if (_isInternalScrollBarUpdate || _closing)
|
||||
{
|
||||
// The update comes from ourselves, more specifically from the
|
||||
// terminal. So we don't have to update the terminal because it
|
||||
// already knows.
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -1448,9 +1481,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
// itself - it was initiated by the mouse wheel, or the scrollbar.
|
||||
_terminal->UserScrollViewport(newValue);
|
||||
|
||||
// We've just told the terminal to update its viewport to reflect the
|
||||
// new scroll value so the scroll bar matches the viewport now.
|
||||
_willUpdateScrollBarToMatchViewport.store(false);
|
||||
// User input takes priority over terminal events so cancel
|
||||
// any pending scroll bar update if the user scrolls.
|
||||
_updateScrollBar->ModifyPending([](auto& update) {
|
||||
update.newValue.reset();
|
||||
});
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
|
|
@ -1949,35 +1984,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
_titleChangedHandlers(winrt::hstring{ wstr });
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Update the position and size of the scrollbar to match the given
|
||||
// viewport top, viewport height, and buffer size.
|
||||
// The change will be actually handled in _ScrollbarChangeHandler.
|
||||
// This should be done on the UI thread. Make sure the caller is calling
|
||||
// us in a RunAsync block.
|
||||
// Arguments:
|
||||
// - viewTop: the top of the visible viewport, in rows. 0 indicates the top
|
||||
// of the buffer.
|
||||
// - viewHeight: the height of the viewport in rows.
|
||||
// - bufferSize: the length of the buffer, in rows
|
||||
void TermControl::_ScrollbarUpdater(Controls::Primitives::ScrollBar scrollBar,
|
||||
const int viewTop,
|
||||
const int viewHeight,
|
||||
const int bufferSize)
|
||||
{
|
||||
// The terminal is already in the scroll position it wants, so no need
|
||||
// to tell it to scroll.
|
||||
_isTerminalInitiatedScroll = true;
|
||||
|
||||
const auto hiddenContent = bufferSize - viewHeight;
|
||||
scrollBar.Maximum(hiddenContent);
|
||||
scrollBar.Minimum(0);
|
||||
scrollBar.ViewportSize(viewHeight);
|
||||
scrollBar.Value(viewTop);
|
||||
|
||||
_isTerminalInitiatedScroll = false;
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Update the position and size of the scrollbar to match the given
|
||||
// viewport top, viewport height, and buffer size.
|
||||
|
|
@ -1988,9 +1994,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
// of the buffer.
|
||||
// - viewHeight: the height of the viewport in rows.
|
||||
// - bufferSize: the length of the buffer, in rows
|
||||
winrt::fire_and_forget TermControl::_TerminalScrollPositionChanged(const int viewTop,
|
||||
const int viewHeight,
|
||||
const int bufferSize)
|
||||
void TermControl::_TerminalScrollPositionChanged(const int viewTop,
|
||||
const int viewHeight,
|
||||
const int bufferSize)
|
||||
{
|
||||
// Since this callback fires from non-UI thread, we might be already
|
||||
// closed/closing.
|
||||
|
|
@ -2001,21 +2007,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
|
||||
_scrollPositionChangedHandlers(viewTop, viewHeight, bufferSize);
|
||||
|
||||
auto weakThis{ get_weak() };
|
||||
ScrollBarUpdate update;
|
||||
const auto hiddenContent = bufferSize - viewHeight;
|
||||
update.newMaximum = hiddenContent;
|
||||
update.newMinimum = 0;
|
||||
update.newViewportSize = viewHeight;
|
||||
update.newValue = viewTop;
|
||||
|
||||
co_await winrt::resume_foreground(Dispatcher());
|
||||
|
||||
// Even if we weren't closed/closing few lines above, we might be
|
||||
// while waiting for this block of code to be dispatched.
|
||||
// If 'weakThis' is locked, then we can safely work with 'this'
|
||||
if (auto control{ weakThis.get() })
|
||||
{
|
||||
if (!_closing.load())
|
||||
{
|
||||
// Update our scrollbar
|
||||
_ScrollbarUpdater(ScrollBar(), viewTop, viewHeight, bufferSize);
|
||||
}
|
||||
}
|
||||
_updateScrollBar->Run(update);
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@
|
|||
#include "../buffer/out/search.h"
|
||||
#include "cppwinrt_utils.h"
|
||||
#include "SearchBoxControl.h"
|
||||
#include "ThrottledFunc.h"
|
||||
|
||||
namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
||||
{
|
||||
|
|
@ -135,8 +136,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
FontInfoDesired _desiredFont;
|
||||
FontInfo _actualFont;
|
||||
|
||||
bool _isTerminalInitiatedScroll;
|
||||
std::atomic<bool> _willUpdateScrollBarToMatchViewport;
|
||||
struct ScrollBarUpdate
|
||||
{
|
||||
std::optional<double> newValue;
|
||||
double newMaximum;
|
||||
double newMinimum;
|
||||
double newViewportSize;
|
||||
};
|
||||
std::shared_ptr<ThrottledFunc<ScrollBarUpdate>> _updateScrollBar;
|
||||
bool _isInternalScrollBarUpdate;
|
||||
|
||||
int _rowsToScroll;
|
||||
|
||||
|
|
@ -201,7 +209,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
void _DoResizeUnderLock(const double newWidth, const double newHeight);
|
||||
void _RefreshSizeUnderLock();
|
||||
void _TerminalTitleChanged(const std::wstring_view& wstr);
|
||||
winrt::fire_and_forget _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize);
|
||||
void _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize);
|
||||
winrt::fire_and_forget _TerminalCursorPositionChanged();
|
||||
|
||||
void _MouseScrollHandler(const double mouseDelta, const Windows::Foundation::Point point, const bool isLeftButtonPressed);
|
||||
|
|
@ -216,7 +224,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
|
|||
void _TryStopAutoScroll(const uint32_t pointerId);
|
||||
void _UpdateAutoScroll(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
|
||||
|
||||
void _ScrollbarUpdater(Windows::UI::Xaml::Controls::Primitives::ScrollBar scrollbar, const int viewTop, const int viewHeight, const int bufferSize);
|
||||
static Windows::UI::Xaml::Thickness _ParseThicknessFromPadding(const hstring padding);
|
||||
|
||||
void _KeyHandler(Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e, const bool keyDown);
|
||||
|
|
|
|||
|
|
@ -43,6 +43,8 @@
|
|||
<ClInclude Include="TermControlAutomationPeer.h">
|
||||
<DependentUpon>TermControlAutomationPeer.idl</DependentUpon>
|
||||
</ClInclude>
|
||||
<ClInclude Include="ThreadSafeOptional.h" />
|
||||
<ClInclude Include="ThrottledFunc.h" />
|
||||
<ClInclude Include="TSFInputControl.h">
|
||||
<DependentUpon>TSFInputControl.xaml</DependentUpon>
|
||||
</ClInclude>
|
||||
|
|
|
|||
|
|
@ -26,6 +26,8 @@
|
|||
<ClInclude Include="TermControlAutomationPeer.h" />
|
||||
<ClInclude Include="XamlUiaTextRange.h" />
|
||||
<ClInclude Include="TermControlUiaProvider.hpp" />
|
||||
<ClInclude Include="ThreadSafeOptional.h" />
|
||||
<ClInclude Include="ThrottledFunc.h" />
|
||||
<ClInclude Include="UiaTextRange.hpp" />
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
|
|
|
|||
54
src/cascadia/TerminalControl/ThreadSafeOptional.h
Normal file
54
src/cascadia/TerminalControl/ThreadSafeOptional.h
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
// Copyright (c) Microsoft Corporation.
|
||||
// Licensed under the MIT license.
|
||||
|
||||
#pragma once
|
||||
#include "pch.h"
|
||||
|
||||
template<typename T>
|
||||
class ThreadSafeOptional
|
||||
{
|
||||
public:
|
||||
template<class... Args>
|
||||
bool Emplace(Args&&... args)
|
||||
{
|
||||
std::lock_guard guard{ _lock };
|
||||
|
||||
bool hadValue = _inner.has_value();
|
||||
_inner.emplace(std::forward<Args>(args)...);
|
||||
return !hadValue;
|
||||
}
|
||||
|
||||
std::optional<T> Take()
|
||||
{
|
||||
std::lock_guard guard{ _lock };
|
||||
|
||||
std::optional<T> value;
|
||||
_inner.swap(value);
|
||||
|
||||
return value;
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - If the optional has a value, then call the specified function with a
|
||||
// reference to the value.
|
||||
// - This method is always thread-safe. It can be called multiple times on
|
||||
// different threads.
|
||||
// Arguments:
|
||||
// - f: the function to call with a reference to the value
|
||||
// Return Value:
|
||||
// - <none>
|
||||
template<typename F>
|
||||
void ModifyValue(F f)
|
||||
{
|
||||
std::lock_guard guard{ _lock };
|
||||
|
||||
if (_inner.has_value())
|
||||
{
|
||||
f(_inner.value());
|
||||
}
|
||||
}
|
||||
|
||||
private:
|
||||
std::mutex _lock;
|
||||
std::optional<T> _inner;
|
||||
};
|
||||
97
src/cascadia/TerminalControl/ThrottledFunc.h
Normal file
97
src/cascadia/TerminalControl/ThrottledFunc.h
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
/*++
|
||||
Copyright (c) Microsoft Corporation
|
||||
Licensed under the MIT license.
|
||||
|
||||
Module Name:
|
||||
- ThrottledFunc.h
|
||||
|
||||
Abstract:
|
||||
- This module defines a class to throttle function calls.
|
||||
- You create an instance of a `ThrottledFunc` with a function and the delay
|
||||
between two function calls.
|
||||
- The function takes an argument of type `T`, the template argument of
|
||||
`ThrottledFunc`.
|
||||
- Use the `Run` method to wait and then call the function.
|
||||
--*/
|
||||
|
||||
#pragma once
|
||||
#include "pch.h"
|
||||
|
||||
#include "ThreadSafeOptional.h"
|
||||
|
||||
template<typename T>
|
||||
class ThrottledFunc : public std::enable_shared_from_this<ThrottledFunc<T>>
|
||||
{
|
||||
public:
|
||||
using Func = std::function<void(T arg)>;
|
||||
|
||||
ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay) :
|
||||
_func{ func },
|
||||
_delay{ delay }
|
||||
{
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Runs the function later with the specified argument, except if `Run`
|
||||
// is called again before with a new argument, in which case the new
|
||||
// argument will be instead.
|
||||
// - For more information, read the "Abstract" section in the header file.
|
||||
// Arguments:
|
||||
// - arg: the argument to pass to the function
|
||||
// Return Value:
|
||||
// - <none>
|
||||
winrt::fire_and_forget Run(T arg)
|
||||
{
|
||||
if (!_pendingCallArg.Emplace(arg))
|
||||
{
|
||||
// already pending
|
||||
return;
|
||||
}
|
||||
|
||||
auto weakThis = this->weak_from_this();
|
||||
|
||||
co_await winrt::resume_after(_delay);
|
||||
|
||||
if (auto self{ weakThis.lock() })
|
||||
{
|
||||
auto arg = self->_pendingCallArg.Take();
|
||||
if (arg.has_value())
|
||||
{
|
||||
self->_func(arg.value());
|
||||
}
|
||||
else
|
||||
{
|
||||
// should not happen
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
// - Modifies the pending argument for the next function invocation, if
|
||||
// there is one pending currently.
|
||||
// - Let's say that you just called the `Run` method with argument A.
|
||||
// After the delay specified in the constructor, the function R
|
||||
// specified in the constructor will be called with argument A.
|
||||
// - By using this method, you can modify argument A before the function R
|
||||
// is called with argument A.
|
||||
// - You pass a function to this method which will take a reference to
|
||||
// argument A and will modify it.
|
||||
// - When there is no pending invocation of function R, this method will
|
||||
// not do anything.
|
||||
// - This method is always thread-safe. It can be called multiple times on
|
||||
// different threads.
|
||||
// Arguments:
|
||||
// - f: the function to call with a reference to the argument
|
||||
// Return Value:
|
||||
// - <none>
|
||||
template<typename F>
|
||||
void ModifyPending(F f)
|
||||
{
|
||||
_pendingCallArg.ModifyValue(f);
|
||||
}
|
||||
|
||||
private:
|
||||
Func _func;
|
||||
winrt::Windows::Foundation::TimeSpan _delay;
|
||||
ThreadSafeOptional<T> _pendingCallArg;
|
||||
};
|
||||
|
|
@ -781,48 +781,49 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
|
|||
auto proposedCursorPosition = proposedPosition;
|
||||
auto& cursor = _buffer->GetCursor();
|
||||
const Viewport bufferSize = _buffer->GetSize();
|
||||
bool notifyScroll = false;
|
||||
|
||||
// If we're about to scroll past the bottom of the buffer, instead cycle the
|
||||
// buffer.
|
||||
// GH#5540 - Make sure this is a positive number. We can't create a
|
||||
// negative number of new rows.
|
||||
const auto newRows = std::max(0, proposedCursorPosition.Y - bufferSize.Height() + 1);
|
||||
if (newRows > 0)
|
||||
SHORT rowsPushedOffTopOfBuffer = 0;
|
||||
if (proposedCursorPosition.Y >= bufferSize.Height())
|
||||
{
|
||||
const auto newRows = proposedCursorPosition.Y - bufferSize.Height() + 1;
|
||||
for (auto dy = 0; dy < newRows; dy++)
|
||||
{
|
||||
_buffer->IncrementCircularBuffer();
|
||||
proposedCursorPosition.Y--;
|
||||
rowsPushedOffTopOfBuffer++;
|
||||
}
|
||||
notifyScroll = true;
|
||||
}
|
||||
|
||||
// Update Cursor Position
|
||||
cursor.SetPosition(proposedCursorPosition);
|
||||
|
||||
const COORD cursorPosAfter = cursor.GetPosition();
|
||||
|
||||
// Move the viewport down if the cursor moved below the viewport.
|
||||
if (cursorPosAfter.Y > _mutableViewport.BottomInclusive())
|
||||
bool updatedViewport = false;
|
||||
if (proposedCursorPosition.Y > _mutableViewport.BottomInclusive())
|
||||
{
|
||||
const auto newViewTop = std::max(0, cursorPosAfter.Y - (_mutableViewport.Height() - 1));
|
||||
const auto newViewTop = std::max(0, proposedCursorPosition.Y - (_mutableViewport.Height() - 1));
|
||||
if (newViewTop != _mutableViewport.Top())
|
||||
{
|
||||
_mutableViewport = Viewport::FromDimensions({ 0, gsl::narrow<short>(newViewTop) },
|
||||
_mutableViewport.Dimensions());
|
||||
notifyScroll = true;
|
||||
updatedViewport = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (notifyScroll)
|
||||
if (updatedViewport)
|
||||
{
|
||||
_NotifyScrollEvent();
|
||||
}
|
||||
|
||||
if (rowsPushedOffTopOfBuffer != 0)
|
||||
{
|
||||
// We have to report the delta here because we might have circled the text buffer.
|
||||
// That didn't change the viewport and therefore the TriggerScroll(void)
|
||||
// method can't detect the delta on its own.
|
||||
COORD delta{ 0, -gsl::narrow<SHORT>(newRows) };
|
||||
COORD delta{ 0, -rowsPushedOffTopOfBuffer };
|
||||
_buffer->GetRenderTarget().TriggerScroll(&delta);
|
||||
_NotifyScrollEvent();
|
||||
}
|
||||
|
||||
_NotifyTerminalCursorPositionChanged();
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ namespace TerminalCoreUnitTests
|
|||
class TerminalBufferTests;
|
||||
class TerminalApiTest;
|
||||
class ConptyRoundtripTests;
|
||||
class TerminalAndRendererTests;
|
||||
class ScrollTest;
|
||||
};
|
||||
#endif
|
||||
|
||||
|
|
@ -301,6 +301,6 @@ private:
|
|||
friend class TerminalCoreUnitTests::TerminalBufferTests;
|
||||
friend class TerminalCoreUnitTests::TerminalApiTest;
|
||||
friend class TerminalCoreUnitTests::ConptyRoundtripTests;
|
||||
friend class TerminalCoreUnitTests::TerminalAndRendererTests;
|
||||
friend class TerminalCoreUnitTests::ScrollTest;
|
||||
#endif
|
||||
};
|
||||
|
|
|
|||
217
src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp
Normal file
217
src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp
Normal file
|
|
@ -0,0 +1,217 @@
|
|||
// Copyright (c) Microsoft Corporation.
|
||||
// Licensed under the MIT license.
|
||||
|
||||
#include "precomp.h"
|
||||
#include <WexTestClass.h>
|
||||
|
||||
#include <argb.h>
|
||||
#include <DefaultSettings.h>
|
||||
|
||||
#include "../renderer/inc/DummyRenderTarget.hpp"
|
||||
#include "../renderer/base/Renderer.hpp"
|
||||
#include "../renderer/dx/DxRenderer.hpp"
|
||||
|
||||
#include "../cascadia/TerminalCore/Terminal.hpp"
|
||||
#include "MockTermSettings.h"
|
||||
#include "consoletaeftemplates.hpp"
|
||||
#include "TestUtils.h"
|
||||
|
||||
using namespace winrt::Microsoft::Terminal::Settings;
|
||||
using namespace Microsoft::Terminal::Core;
|
||||
using namespace ::Microsoft::Console::Types;
|
||||
|
||||
using namespace WEX::Common;
|
||||
using namespace WEX::Logging;
|
||||
using namespace WEX::TestExecution;
|
||||
|
||||
namespace
|
||||
{
|
||||
class MockScrollRenderTarget final : public ::Microsoft::Console::Render::IRenderTarget
|
||||
{
|
||||
public:
|
||||
~MockScrollRenderTarget() override{};
|
||||
|
||||
std::optional<COORD> TriggerScrollDelta() const
|
||||
{
|
||||
return _triggerScrollDelta;
|
||||
}
|
||||
|
||||
void Reset()
|
||||
{
|
||||
_triggerScrollDelta.reset();
|
||||
}
|
||||
|
||||
virtual void TriggerRedraw(const Microsoft::Console::Types::Viewport&){};
|
||||
virtual void TriggerRedraw(const COORD* const){};
|
||||
virtual void TriggerRedrawCursor(const COORD* const){};
|
||||
virtual void TriggerRedrawAll(){};
|
||||
virtual void TriggerTeardown(){};
|
||||
virtual void TriggerSelection(){};
|
||||
virtual void TriggerScroll(){};
|
||||
virtual void TriggerScroll(const COORD* const delta)
|
||||
{
|
||||
_triggerScrollDelta = { *delta };
|
||||
};
|
||||
virtual void TriggerCircling(){};
|
||||
void TriggerTitleChange(){};
|
||||
|
||||
private:
|
||||
std::optional<COORD> _triggerScrollDelta;
|
||||
};
|
||||
|
||||
struct ScrollBarNotification
|
||||
{
|
||||
int ViewportTop;
|
||||
int ViewportHeight;
|
||||
int BufferHeight;
|
||||
};
|
||||
}
|
||||
|
||||
namespace TerminalCoreUnitTests
|
||||
{
|
||||
class ScrollTest;
|
||||
};
|
||||
using namespace TerminalCoreUnitTests;
|
||||
|
||||
class TerminalCoreUnitTests::ScrollTest final
|
||||
{
|
||||
// !!! DANGER: Many tests in this class expect the Terminal buffer
|
||||
// to be 80x32. If you change these, you'll probably inadvertently break a
|
||||
// bunch of tests !!!
|
||||
static const SHORT TerminalViewWidth = 80;
|
||||
static const SHORT TerminalViewHeight = 32;
|
||||
// For TestNotifyScrolling, it's important that this value is ~=9000.
|
||||
// Something smaller like 100 won't cause the test to fail.
|
||||
static const SHORT TerminalHistoryLength = 9001;
|
||||
|
||||
TEST_CLASS(ScrollTest);
|
||||
|
||||
TEST_METHOD(TestNotifyScrolling);
|
||||
|
||||
TEST_METHOD_SETUP(MethodSetup)
|
||||
{
|
||||
_term = std::make_unique<::Microsoft::Terminal::Core::Terminal>();
|
||||
|
||||
_scrollBarNotification = std::make_shared<std::optional<ScrollBarNotification>>();
|
||||
_term->SetScrollPositionChangedCallback([scrollBarNotification = _scrollBarNotification](const int top, const int height, const int bottom) {
|
||||
ScrollBarNotification tmp;
|
||||
tmp.ViewportTop = top;
|
||||
tmp.ViewportHeight = height;
|
||||
tmp.BufferHeight = bottom;
|
||||
*scrollBarNotification = { tmp };
|
||||
});
|
||||
|
||||
_renderTarget = std::make_unique<MockScrollRenderTarget>();
|
||||
_term->Create({ TerminalViewWidth, TerminalViewHeight }, TerminalHistoryLength, *_renderTarget);
|
||||
return true;
|
||||
}
|
||||
|
||||
TEST_METHOD_CLEANUP(MethodCleanup)
|
||||
{
|
||||
_term = nullptr;
|
||||
return true;
|
||||
}
|
||||
|
||||
private:
|
||||
std::unique_ptr<Terminal> _term;
|
||||
std::unique_ptr<MockScrollRenderTarget> _renderTarget;
|
||||
std::shared_ptr<std::optional<ScrollBarNotification>> _scrollBarNotification;
|
||||
};
|
||||
|
||||
void ScrollTest::TestNotifyScrolling()
|
||||
{
|
||||
// See https://github.com/microsoft/terminal/pull/5630
|
||||
//
|
||||
// This is a test for GH#5540, in the most bizarre way. The origin of that
|
||||
// bug was that as newlines were emitted, we'd accumulate an enormous scroll
|
||||
// delta into a selection region, to the point of overflowing a SHORT. When
|
||||
// the overflow occurred, the Terminal would fail to send a NotifyScroll() to
|
||||
// the TermControl hosting it.
|
||||
//
|
||||
// For this bug to repro, we need to:
|
||||
// - Have a sufficiently large buffer, because each newline we'll accumulate
|
||||
// a delta of (0, ~bufferHeight), so (bufferHeight^2 + bufferHeight) >
|
||||
// SHRT_MAX
|
||||
// - Have a selection
|
||||
|
||||
Log::Comment(L"Watch out - this test takes a while to run, and won't "
|
||||
L"output anything unless in encounters an error. This is expected.");
|
||||
|
||||
auto& termTb = *_term->_buffer;
|
||||
auto& termSm = *_term->_stateMachine;
|
||||
|
||||
const auto totalBufferSize = termTb.GetSize().Height();
|
||||
|
||||
auto currentRow = 0;
|
||||
|
||||
// We're outputting like 18000 lines of text here, so emitting 18000*4 lines
|
||||
// of output to the console is actually quite unnecessary
|
||||
WEX::TestExecution::SetVerifyOutput settings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);
|
||||
|
||||
// Emit a bunch of newlines to test scrolling.
|
||||
for (; currentRow < totalBufferSize * 2; currentRow++)
|
||||
{
|
||||
*_scrollBarNotification = std::nullopt;
|
||||
_renderTarget->Reset();
|
||||
|
||||
termSm.ProcessString(L"X\r\n");
|
||||
|
||||
// When we're on TerminalViewHeight-1, we'll emit the newline that
|
||||
// causes the first scroll event
|
||||
bool scrolled = currentRow >= TerminalViewHeight - 1;
|
||||
|
||||
// When we circle the buffer, the scroll bar's position does not
|
||||
// change.
|
||||
bool circledBuffer = currentRow >= totalBufferSize - 1;
|
||||
bool expectScrollBarNotification = scrolled && !circledBuffer;
|
||||
|
||||
if (expectScrollBarNotification)
|
||||
{
|
||||
VERIFY_IS_TRUE(_scrollBarNotification->has_value(),
|
||||
fmt::format(L"Expected a 'scroll bar position changed' notification for row {}", currentRow).c_str());
|
||||
}
|
||||
else
|
||||
{
|
||||
VERIFY_IS_FALSE(_scrollBarNotification->has_value(),
|
||||
fmt::format(L"Expected to not see a 'scroll bar position changed' notification for row {}", currentRow).c_str());
|
||||
}
|
||||
|
||||
// If we scrolled but it circled the buffer, then the terminal will
|
||||
// call `TriggerScroll` with a delta to tell the renderer about it.
|
||||
if (scrolled && circledBuffer)
|
||||
{
|
||||
VERIFY_IS_TRUE(_renderTarget->TriggerScrollDelta().has_value(),
|
||||
fmt::format(L"Expected a 'trigger scroll' notification in RenderTarget for row {}", currentRow).c_str());
|
||||
|
||||
COORD expectedDelta;
|
||||
expectedDelta.X = 0;
|
||||
expectedDelta.Y = -1;
|
||||
VERIFY_ARE_EQUAL(expectedDelta, _renderTarget->TriggerScrollDelta().value(), fmt::format(L"Wrong value in 'trigger scroll' notification in RenderTarget for row {}", currentRow).c_str());
|
||||
}
|
||||
else
|
||||
{
|
||||
VERIFY_IS_FALSE(_renderTarget->TriggerScrollDelta().has_value(),
|
||||
fmt::format(L"Expected to not see a 'trigger scroll' notification in RenderTarget for row {}", currentRow).c_str());
|
||||
}
|
||||
|
||||
if (_scrollBarNotification->has_value())
|
||||
{
|
||||
const auto tmp = _scrollBarNotification->value();
|
||||
|
||||
const int expectedTop = std::clamp<int>(currentRow - TerminalViewHeight + 2,
|
||||
0,
|
||||
TerminalHistoryLength);
|
||||
const int expectedHeight = TerminalViewHeight;
|
||||
const int expectedBottom = expectedTop + TerminalViewHeight;
|
||||
if ((tmp.ViewportTop != expectedTop) ||
|
||||
(tmp.ViewportHeight != expectedHeight) ||
|
||||
(tmp.BufferHeight != expectedBottom))
|
||||
{
|
||||
Log::Comment(NoThrowString().Format(L"Expected viewport values did not match on line %d", currentRow));
|
||||
}
|
||||
VERIFY_ARE_EQUAL(tmp.ViewportTop, expectedTop);
|
||||
VERIFY_ARE_EQUAL(tmp.ViewportHeight, expectedHeight);
|
||||
VERIFY_ARE_EQUAL(tmp.BufferHeight, expectedBottom);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,177 +0,0 @@
|
|||
// Copyright (c) Microsoft Corporation.
|
||||
// Licensed under the MIT license.
|
||||
//
|
||||
// This test class is useful for cases where we need a Terminal, a Renderer, and
|
||||
// a DxEngine. Some bugs won't repro without all three actually being hooked up.
|
||||
// Note however, that the DxEngine is not wired up to actually be PaintFrame'd
|
||||
// in this test - it pretty heavily depends on being able to actually get a
|
||||
// render target, and as we're running in a unit test, we don't have one of
|
||||
// those. However, this class is good for testing how invalidation works across
|
||||
// all three.
|
||||
|
||||
#include "precomp.h"
|
||||
#include <WexTestClass.h>
|
||||
|
||||
#include <argb.h>
|
||||
#include <DefaultSettings.h>
|
||||
|
||||
#include "../renderer/inc/DummyRenderTarget.hpp"
|
||||
#include "../renderer/base/Renderer.hpp"
|
||||
#include "../renderer/dx/DxRenderer.hpp"
|
||||
|
||||
#include "../cascadia/TerminalCore/Terminal.hpp"
|
||||
#include "MockTermSettings.h"
|
||||
#include "consoletaeftemplates.hpp"
|
||||
#include "TestUtils.h"
|
||||
|
||||
using namespace winrt::Microsoft::Terminal::Settings;
|
||||
using namespace Microsoft::Terminal::Core;
|
||||
using namespace ::Microsoft::Console::Types;
|
||||
|
||||
using namespace WEX::Common;
|
||||
using namespace WEX::Logging;
|
||||
using namespace WEX::TestExecution;
|
||||
|
||||
namespace TerminalCoreUnitTests
|
||||
{
|
||||
class TerminalAndRendererTests;
|
||||
};
|
||||
using namespace TerminalCoreUnitTests;
|
||||
|
||||
class TerminalCoreUnitTests::TerminalAndRendererTests final
|
||||
{
|
||||
// !!! DANGER: Many tests in this class expect the Terminal buffer
|
||||
// to be 80x32. If you change these, you'll probably inadvertently break a
|
||||
// bunch of tests !!!
|
||||
static const SHORT TerminalViewWidth = 80;
|
||||
static const SHORT TerminalViewHeight = 32;
|
||||
// For TestNotifyScrolling, it's important that this value is ~=9000.
|
||||
// Something smaller like 100 won't cause the test to fail.
|
||||
static const SHORT TerminalHistoryLength = 9001;
|
||||
|
||||
TEST_CLASS(TerminalAndRendererTests);
|
||||
|
||||
TEST_METHOD(TestNotifyScrolling);
|
||||
|
||||
TEST_METHOD_SETUP(MethodSetup)
|
||||
{
|
||||
_term = std::make_unique<::Microsoft::Terminal::Core::Terminal>();
|
||||
|
||||
// Create the renderer
|
||||
_renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(_term.get(), nullptr, 0, nullptr);
|
||||
::Microsoft::Console::Render::IRenderTarget& renderTarget = *_renderer;
|
||||
|
||||
// Create the engine
|
||||
_dxEngine = std::make_unique<::Microsoft::Console::Render::DxEngine>();
|
||||
_renderer->AddRenderEngine(_dxEngine.get());
|
||||
|
||||
// Initialize the renderer, engine for a default font size
|
||||
_renderer->TriggerFontChange(USER_DEFAULT_SCREEN_DPI, _desiredFont, _actualFont);
|
||||
const til::size viewportSize{ TerminalViewWidth, TerminalViewHeight };
|
||||
const til::size fontSize = _actualFont.GetSize();
|
||||
const til::size windowSize = viewportSize * fontSize;
|
||||
VERIFY_SUCCEEDED(_dxEngine->SetWindowSize(windowSize));
|
||||
const auto vp = _dxEngine->GetViewportInCharacters(Viewport::FromDimensions({ 0, 0 }, windowSize));
|
||||
VERIFY_ARE_EQUAL(viewportSize, til::size{ vp.Dimensions() });
|
||||
|
||||
// Set up the Terminal, using the Renderer (which has the engine in it)
|
||||
_term->Create({ TerminalViewWidth, TerminalViewHeight }, TerminalHistoryLength, renderTarget);
|
||||
return true;
|
||||
}
|
||||
|
||||
TEST_METHOD_CLEANUP(MethodCleanup)
|
||||
{
|
||||
_term = nullptr;
|
||||
return true;
|
||||
}
|
||||
|
||||
private:
|
||||
std::unique_ptr<Terminal> _term;
|
||||
std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer;
|
||||
std::unique_ptr<::Microsoft::Console::Render::DxEngine> _dxEngine;
|
||||
|
||||
FontInfoDesired _desiredFont{ DEFAULT_FONT_FACE, 0, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8 };
|
||||
FontInfo _actualFont{ DEFAULT_FONT_FACE, 0, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false };
|
||||
};
|
||||
|
||||
void TerminalAndRendererTests::TestNotifyScrolling()
|
||||
{
|
||||
// See https://github.com/microsoft/terminal/pull/5630
|
||||
//
|
||||
// This is a test for GH#5540, in the most bizarre way. The origin of that
|
||||
// bug was that as newlines were emitted, we'd accumulate an enormous scroll
|
||||
// delta into a selection region, to the point of overflowing a SHORT. When
|
||||
// the overflow occurred, the Terminal would fail to send a NotifyScroll() to
|
||||
// the TermControl hosting it.
|
||||
//
|
||||
// For this bug to repro, we need to:
|
||||
// - Have a sufficiently large buffer, because each newline we'll accumulate
|
||||
// a delta of (0, ~bufferHeight), so (bufferHeight^2 + bufferHeight) >
|
||||
// SHRT_MAX
|
||||
// - Have a selection
|
||||
|
||||
Log::Comment(L"Watch out - this test takes a while to run, and won't "
|
||||
L"output anything unless in encounters an error. This is expected.");
|
||||
|
||||
auto& termTb = *_term->_buffer;
|
||||
auto& termSm = *_term->_stateMachine;
|
||||
|
||||
const auto totalBufferSize = termTb.GetSize().Height();
|
||||
|
||||
auto currentRow = 0;
|
||||
bool gotScrollingNotification = false;
|
||||
|
||||
// We're outputting like 18000 lines of text here, so emitting 18000*4 lines
|
||||
// of output to the console is actually quite unnecessary
|
||||
WEX::TestExecution::SetVerifyOutput settings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);
|
||||
|
||||
auto verifyScrolling = [&](const int top, const int height, const int bottom) {
|
||||
const int expectedTop = std::clamp<int>(currentRow - TerminalViewHeight + 2,
|
||||
0,
|
||||
TerminalHistoryLength);
|
||||
|
||||
const int expectedHeight = TerminalViewHeight;
|
||||
const int expectedBottom = expectedTop + TerminalViewHeight;
|
||||
if ((expectedTop != top) ||
|
||||
(expectedHeight != height) ||
|
||||
(expectedBottom != bottom))
|
||||
{
|
||||
Log::Comment(NoThrowString().Format(L"Expected values did not match on line %d", currentRow));
|
||||
}
|
||||
VERIFY_ARE_EQUAL(expectedTop, top);
|
||||
VERIFY_ARE_EQUAL(expectedHeight, height);
|
||||
VERIFY_ARE_EQUAL(expectedBottom, bottom);
|
||||
|
||||
gotScrollingNotification = true;
|
||||
};
|
||||
|
||||
// Hook up the scrolling callback
|
||||
_term->SetScrollPositionChangedCallback(verifyScrolling);
|
||||
|
||||
// Create a selection - the actual bounds don't matter, we just need to have one.
|
||||
_term->SetSelectionAnchor(COORD{ 0, 0 });
|
||||
_term->SetSelectionEnd(COORD{ TerminalViewWidth - 1, 0 });
|
||||
_renderer->TriggerSelection();
|
||||
|
||||
// Emit a bunch of newlines. Eventually, the accumulated scroll delta will
|
||||
// cause an overflow, and cause us to miss a NotifyScroll.
|
||||
for (; currentRow < totalBufferSize * 2; currentRow++)
|
||||
{
|
||||
gotScrollingNotification = false;
|
||||
|
||||
termSm.ProcessString(L"X\r\n");
|
||||
|
||||
// When we're on TerminalViewHeight-1, we'll emit the newline that
|
||||
// causes the first scroll event
|
||||
if (currentRow >= TerminalViewHeight - 1)
|
||||
{
|
||||
VERIFY_IS_TRUE(gotScrollingNotification,
|
||||
fmt::format(L"Expected a scrolling notification for row {}", currentRow).c_str());
|
||||
}
|
||||
else
|
||||
{
|
||||
VERIFY_IS_FALSE(gotScrollingNotification,
|
||||
fmt::format(L"Expected to not see scrolling notification for row {}", currentRow).c_str());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -20,7 +20,7 @@
|
|||
<ClCompile Include="TerminalApiTest.cpp" />
|
||||
<ClCompile Include="ConptyRoundtripTests.cpp" />
|
||||
<ClCompile Include="TerminalBufferTests.cpp" />
|
||||
<ClCompile Include="TerminalAndRendererTests.cpp" />
|
||||
<ClCompile Include="ScrollTest.cpp" />
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\..\buffer\out\lib\bufferout.vcxproj">
|
||||
|
|
|
|||
Loading…
Reference in a new issue