Недавно вышла новая версия Windows Terminal. Всё бы ничего, но работоспособность её скроллбара оставляла желать лучшего. Поэтому настало время немного потыкать в него палкой и сыграть на бубне.
Что обычно пользователи делают с новой версией любого приложения? Правильно, именно то, что не делали тестеры. Поэтому после недолгого использования терминала по назначению, я начал делать с ним страшные вещи. Хорошо-хорошо, я просто пролил кофе на клавиатуру и случайно зажал <Enter>, когда протирал ее. Что в итоге получилось?
Да, выглядит не очень впечатляюще, но не спешите бросать в меня камни. Обратите внимание на правую часть. Попробуйте сперва сами догадаться, что с ней не так. Вот вам скриншот для подсказки:
Конечно, название статьи было серьёзным спойлером. :)
Итак, есть проблема со скроллбаром. Переходя на новую строку много раз, после пересечения нижней границы, обычно ожидаешь, что появится скроллбар, и можно будет пролистать наверх. Однако этого не происходит до того момента, как мы не напишем какую-либо команду с выводом чего-либо. Скажем так, поведение странное. Впрочем, это могло быть не так критично, если бы скроллбар работал…
Немного потестировав, я обнаружил, что переход на новую строку не увеличивает буфер. Это делает исключительно вывод команд. Так что вышенаписанная whoami увеличит буфер только на одну строку. Из-за этого со временем мы потеряем много истории, особенно после clear.
Первое, что мне пришло на ум – это воспользоваться нашим анализатором и посмотреть, что он скажет:
Вывод, конечно, внушительных размеров, поэтому я воспользуюсь могуществом фильтрации и обрежу всё, кроме содержащего ScrollBar:
Не могу сказать, что сообщений много… Хорошо, может быть тогда есть что-либо связанное с буфером?
Анализатор не подвел и нашел что-то интересное. Я выделил это предупреждение выше. Давайте посмотрим, что там не так:
V501. There are identical sub-expressions to the left and to the right of the '-' operator: bufferHeight — bufferHeight TermControl.cpp 592
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(bufferHeight - bufferHeight); // <= Ошибка тут
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}Этот код сопровождается комментарием: «Set up the height of the ScrollViewer and the grid we're using to fake our scrolling height».
Имитация высоты прокрутки – это, конечно, хорошо, но почему в максимум мы проставляем 0? Обратившись к документации, стало понятно, что код не сильно подозрителен. Не поймите меня неправильно: вычитание переменной из самой себя, конечно, подозрительно, но мы получаем на выходе ноль, который нам не вредит. В любом случае, я попробовал указать в поле Maximum значение по умолчанию (1):
Скроллбар появился, но он так же не работает:
Если что, то я зажал <Enter> секунд на 30. Видимо проблема не в этом, поэтому оставим как было, разве что, заменив bufferHeight – bufferHeight на 0:
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(0); // <= Замена случилась тут
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}Итак, к решению проблемы мы не особенно приблизились. За неимением лучшего предлагаю отправиться в дебаг. Сперва мы могли бы поставить точку останова (breakpoint) на измененной строке, но сомневаюсь, что это нам как-то поможет. Поэтому нам нужно сперва найти фрагмент, который отвечает за смещение Viewport'а относительно буфера.
Немного о том, как устроен местный (и, скорее всего, любой другой) скроллбар. Есть у нас один большой буфер, который хранит весь вывод. Для взаимодействия с ним используется какая-либо абстракция для отрисовки на экран, в данном случае – viewport.
Используя эти два примитива, можно понять в чем заключается наша проблема. Переход на новую строку не увеличивает буфер, и из-за этого нам просто некуда подниматься. Следовательно, проблема именно в нем.
Вооружившись этими банальными знаниями, продолжим наш героический дебаг. Немного погуляв по функции, я обратил внимание на этот фрагмент:
// This event is explicitly revoked in the destructor: does not need weak_ref
auto onReceiveOutputFn = [this](const hstring str) {
_terminal->Write(str);
};
_connectionOutputEventToken = _connection.TerminalOutput(onReceiveOutputFn);После того, как мы настроили выше ScrollBar, мы настраиваем различные callback-функции и выполняем __connection.Start() для нашего новоиспечённого окна. После чего происходит вызов вышеуказанной лямбды. Так как это первый раз, когда мы пишем что-то в буфер, предлагаю начать наш дебаг именно оттуда.
Ставим точку останова внутри лямбды и заглядываем в _terminal:
Теперь у нас есть две крайне важных для нас переменных – _buffer и _mutableViewport. Поставим на них точки останова и найдём, где они меняются. Правда, с _viewport я немного схитрю и поставлю точку останова не на саму переменную, а на ее поле top (оно нам как раз и нужно).
Теперь нажимаем на <F5>, и ничего не происходит… Хорошо, тогда давайте нажмём пару десятков раз <Enter>. Ничего не произошло. Судя по всему, на _buffer мы поставили точку останова слишком опрометчиво, а _viewport, как и ожидалось, оставался на вершине буфера, который не увеличивался в размере.
В таком случае есть смысл ввести команду, чтобы вызвать обновление вершины _viewport. После этого мы встали на очень любопытном фрагменте кода:
void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
{
....
// Move the viewport down if the cursor moved below the viewport.
if (cursorPosAfter.Y > _mutableViewport.BottomInclusive())
{
const auto newViewTop =
std::max(0, cursorPosAfter.Y - (_mutableViewport.Height() - 1));
if (newViewTop != _mutableViewport.Top())
{
_mutableViewport = Viewport::FromDimensions(....); // <=
notifyScroll = true;
}
}
....
}Я указал комментарием, где мы остановились. Если посмотреть на комментарий к фрагменту, то становится ясно, что мы близки к решению, как никогда. Именно в этом месте видимая часть смещается относительно буфера, и мы получаем возможность скроллить. Немного понаблюдав за поведением, я заметил один интересный момент: при переходе на новую строку значение переменной cursorPosAfter.Y равно значению viewport'а, поэтому мы его не опускаем, и ничего не работает. К тому же есть аналогичная проблема с переменной newViewTop. Поэтому давайте увеличим значение cursorPosAfter.Y на один и посмотрим, что получилось:
void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
{
....
// Move the viewport down if the cursor moved below the viewport.
if (cursorPosAfter.Y + 1 > _mutableViewport.BottomInclusive())
{
const auto newViewTop =
std::max(0, cursorPosAfter.Y + 1 - (_mutableViewport.Height() - 1));
if (newViewTop != _mutableViewport.Top())
{
_mutableViewport = Viewport::FromDimensions(....); // <=
notifyScroll = true;
}
}
....
}И результат запуска:
Чудеса! Я ввёл n-e количество, и скроллбар работает. Правда, до того момента, как мы введем что-либо… Для демонстрации фейла, я приложу гифку:
Судя по всему, мы делаем несколько лишних переходов на новую строку. Давайте тогда попробуем ограничить наши переходы, при помощи координаты X. Будем смещать строку только тогда, когда X равен 0:
void Terminal::_AdjustCursorPosition(const COORD proposedPosition)
{
....
if ( proposedCursorPosition.X == 0
&& proposedCursorPosition.Y == _mutableViewport.BottomInclusive())
{
proposedCursorPosition.Y++;
}
// Update Cursor Position
сursor.SetPosition(proposedCursorPosition);
const COORD cursorPosAfter = cursor.GetPosition();
// Move the viewport down if the cursor moved below the viewport.
if (cursorPosAfter.Y > _mutableViewport.BottomInclusive())
{
const auto newViewTop =
std::max(0, cursorPosAfter.Y - (_mutableViewport.Height() - 1));
if (newViewTop != _mutableViewport.Top())
{
_mutableViewport = Viewport::FromDimensions(....);
notifyScroll = true;
}
}
....
}Фрагмент написанный выше будет смещать координату Y для курсора. После чего мы обновляем позицию курсора. В теории это должно сработать… Что у нас вышло?
Ну, уже, конечно, лучше. Однако есть проблема в том, что мы смещаем точку вывода, но при этом не сдвигаем буфер. Поэтому мы видим два вызова одной и той же команды. Может, конечно, показаться, что я знаю, что делаю, но это не так. :)
На этом этапе я решил проверить содержимое буфера, поэтому вернулся к моменту с которого начал дебаг:
// This event is explicitly revoked in the destructor: does not need weak_ref
auto onReceiveOutputFn = [this](const hstring str) {
_terminal->Write(str);
};
_connectionOutputEventToken = _connection.TerminalOutput(onReceiveOutputFn);Я поставил точку останова в тоже место, что и в прошлый раз, и начал смотреть содержимое переменной str. Начнём с того, что я видел на моём экране:
Как вы думаете, что будет в строке str, когда я нажму <Enter>?
- Строка «LONG DESCRIPTION».
- Весь буфер, который мы сейчас видим.
- Весь буфер, но без первой строки.
Не буду томить – весь буфер, но без первой строки. И это немалая проблема, так как именно из-за этого мы и теряем историю, причем точечно. Вот так будет выглядеть наш фрагмент вывода help после перехода на новую строку:
Стрелочкой я отметил место, где был «LONG DESCRIPTOIN». Может быть тогда перезаписывать буфер со смещением на одну строку? Это бы сработало, если бы этот callback вызывался не на каждый чих.
Я обнаружил как минимум три ситуации, когда он вызывается,
- Когда мы вводим любой символ;
- Когда мы двигаемся по истории;
- Когда мы выполняем команду.
Проблема в том, что смещать буфер нужно только тогда, когда мы выполняем команду, или же вводим <Enter>. В остальных случаях делать это – плохая идея. Значит нам нужно как-то определить внутри, что нужно сместить.
Заключение
Эта статья была попыткой показать, как ловко PVS-Studio смог найти дефектный код, приводящий к замеченной мной ошибке. Сообщение на тему вычитания переменной самой из себя меня сильно мотивировало, и я бодро приступил к написанию текста. Но как видите, моя радость была преждевременной и всё оказалось гораздо сложнее.
Поэтому я решил остановиться. Можно было бы ещё потратить пару вечерков, но чем дольше я этим занимался, тем больше и больше проблем возникало. Всё, что я могу сделать, так это пожелать удачи разработчикам Windows Terminal в исправлении этого бага. :)
Надеюсь, я не сильно разочаровал читателя, что так и не довёл исследования до конца и ему было интересно вместе со мной совершить прогулку по внутренностям проекта. В качестве компенсации, я предлагаю воспользоваться промокодом #WindowsTerminal, благодаря которому вы получите демонстрационную версию PVS-Studio не на неделю, а сразу на месяц. Если вы ещё не пробовали статический анализатор PVS-Studio в деле, это хороший повод как раз это сделать. Просто введите "#WindowsTerminal" в поле «Message» на странице загрузки.
А ещё, пользуясь случаем, хочу напомнить, что скоро появится версия C# анализатора, работающая под Linux и macOS. И уже сейчас можно записаться на предварительное тестирование.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Maxim Zvyagintsev. The Little Scrollbar That Could Not.
Автор: Максим Звягинцев
