Скроллбар, который не смог19.03.2020 16:48

b6xeuj-pps_cnuo5ky22sr0yl5o.png


Недавно вышла новая версия Windows Terminal. Всё бы ничего, но работоспособность её скроллбара оставляла желать лучшего. Поэтому настало время немного потыкать в него палкой и сыграть на бубне.
Что обычно пользователи делают с новой версией любого приложения? Правильно, именно то, что не делали тестеры. Поэтому после недолгого использования терминала по назначению, я начал делать с ним страшные вещи. Хорошо-хорошо, я просто пролил кофе на клавиатуру и случайно зажал , когда протирал ее. Что в итоге получилось?

tqevatebkqh9qhtc7meqb8gd9y8.png

Да, выглядит не очень впечатляюще, но не спешите бросать в меня камни. Обратите внимание на правую часть. Попробуйте сперва сами догадаться, что с ней не так. Вот вам скриншот для подсказки:

33g5nz7ix7fpgot21hpt-flsw0m.png


Конечно, название статьи было серьёзным спойлером. :)

Итак, есть проблема со скроллбаром. Переходя на новую строку много раз, после пересечения нижней границы, обычно ожидаешь, что появится скроллбар, и можно будет пролистать наверх. Однако этого не происходит до того момента, как мы не напишем какую-либо команду с выводом чего-либо. Скажем так, поведение странное. Впрочем, это могло быть не так критично, если бы скроллбар работал…

Немного потестировав, я обнаружил, что переход на новую строку не увеличивает буфер. Это делает исключительно вывод команд. Так что вышенаписанная whoami увеличит буфер только на одну строку. Из-за этого со временем мы потеряем много истории, особенно после clear.

Первое, что мне пришло на ум — это воспользоваться нашим анализатором и посмотреть, что он скажет:

hfdtop2yyl-2hnm0fdzu2bg6jmo.png


Вывод, конечно, внушительных размеров, поэтому я воспользуюсь могуществом фильтрации и обрежу всё, кроме содержащего ScrollBar:

kbk9nhstx9w90yxa5q8sm542fvk.png


Не могу сказать, что сообщений много… Хорошо, может быть тогда есть что-либо связанное с буфером?

jsymu0ehvgbwmldkhi30tkr8duw.png


Анализатор не подвел и нашел что-то интересное. Я выделил это предупреждение выше. Давайте посмотрим, что там не так:

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):

mypz-rza0bhmadv3_q32c5oklkc.png


Скроллбар появился, но он так же не работает:

_niixgvkmgqfoy_fmiyaouu6rog.png

Если что, то я зажал секунд на 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:

0zlzrbnxlr94nbnb2jkgnmmf1sq.png

Теперь у нас есть две крайне важных для нас переменных — _buffer и _mutableViewport. Поставим на них точки останова и найдём, где они меняются. Правда, с _viewport я немного схитрю и поставлю точку останова не на саму переменную, а на ее поле top (оно нам как раз и нужно).

Теперь нажимаем на , и ничего не происходит… Хорошо, тогда давайте нажмём пару десятков раз . Ничего не произошло. Судя по всему, на _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;
    }
  }
  ....
}


И результат запуска:

6g7dxcp_8o_uw-jzmlx6qxy6xjy.png


Чудеса! Я ввёл n-e количество, и скроллбар работает. Правда, до того момента, как мы введем что-либо… Для демонстрации фейла, я приложу гифку:

cled8yrho-rrvtihhjf7uoo7ybq.gif


Судя по всему, мы делаем несколько лишних переходов на новую строку. Давайте тогда попробуем ограничить наши переходы, при помощи координаты 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 для курсора. После чего мы обновляем позицию курсора. В теории это должно сработать… Что у нас вышло?

7ikcxsonvzc4h99f-6obibt1o8u.gif

Ну, уже, конечно, лучше. Однако есть проблема в том, что мы смещаем точку вывода, но при этом не сдвигаем буфер. Поэтому мы видим два вызова одной и той же команды. Может, конечно, показаться, что я знаю, что делаю, но это не так. :)

На этом этапе я решил проверить содержимое буфера, поэтому вернулся к моменту с которого начал дебаг: 

// 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. Начнём с того, что я видел на моём экране:

hall4czp0c9scl5ybzmduvpa2iy.png


Как вы думаете, что будет в строке str, когда я нажму ?

  1. Строка «LONG DESCRIPTION».
  2. Весь буфер, который мы сейчас видим.
  3. Весь буфер, но без первой строки.


Не буду томить — весь буфер, но без первой строки. И это немалая проблема, так как именно из-за этого мы и теряем историю, причем точечно. Вот так будет выглядеть наш фрагмент вывода help после перехода на новую строку:

rcn8d94h6tyf1qtscqyqvfvtn24.png


Стрелочкой я отметил место, где был «LONG DESCRIPTOIN». Может быть тогда перезаписывать буфер со смещением на одну строку? Это бы сработало, если бы этот callback вызывался не на каждый чих.

Я обнаружил как минимум три ситуации, когда он вызывается,

  • Когда мы вводим любой символ;
  • Когда мы двигаемся по истории;
  • Когда мы выполняем команду.


Проблема в том, что смещать буфер нужно только тогда, когда мы выполняем команду, или же вводим. В остальных случаях делать это — плохая идея. Значит нам нужно как-то определить внутри, что нужно сместить.

Заключение


Picture 18

Эта статья была попыткой показать, как ловко PVS-Studio смог найти дефектный код, приводящий к замеченной мной ошибке. Сообщение на тему вычитания переменной самой из себя меня сильно мотивировало, и я бодро приступил к написанию текста. Но как видите, моя радость была преждевременной и всё оказалось гораздо сложнее.

Поэтому я решил остановиться. Можно было бы ещё потратить пару вечерков, но чем дольше я этим занимался, тем больше и больше проблем возникало. Всё, что я могу сделать, так это пожелать удачи разработчикам Windows Terminal в исправлении этого бага. :)

Надеюсь, я не сильно разочаровал читателя, что так и не довёл исследования до конца и ему было интересно вместе со мной совершить прогулку по внутренностям проекта. В качестве компенсации, я предлагаю воспользоваться промокодом #WindowsTerminal, благодаря которому вы получите демонстрационную версию PVS-Studio не на неделю, а сразу на месяц. Если вы ещё не пробовали статический анализатор PVS-Studio в деле, это хороший повод как раз это сделать. Просто введите »#WindowsTerminal» в поле «Message» на странице загрузки.

А ещё, пользуясь случаем, хочу напомнить, что скоро появится версия C# анализатора, работающая под Linux и macOS. И уже сейчас можно записаться на предварительное тестирование.

c7830f70c5577c3d6704f254d7cad6a3.png


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Maxim Zvyagintsev. The Little Scrollbar That Could Not.

© Habrahabr.ru