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


    Недавно вышла новая версия 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>?

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

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


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

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

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

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

    Заключение


    Picture 18


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

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

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

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


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Maxim Zvyagintsev. The Little Scrollbar That Could Not.
    PVS-Studio
    Static Code Analysis for C, C++, C# and Java

    Комментарии 3

      +8
      Не хватает ссылки на баг репорт
        0
        Terminal::_AdjustCursorPosition

        Это ведь нарушение стандарта?


        https://en.cppreference.com/w/cpp/language/identifiers


        the identifiers that begin with an underscore followed by an uppercase letter are reserved;
          0
          Да, похоже на то

        Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

        Самое читаемое