Bug: ScreenInteractive::FixedSize screen stomps on the history terminal output

[zozowell]

Bug: ScreenInteractive::FixedSize screen stomps on the preceding terminal output

if I use the "ScreenInteractive::FixedSize()" screen and it's Loop function to draw the screen, the entire screen is showed above the latest cursor line, and covers all history terminal output. This bug doesn't exist in other Dimension type like TerminalOuput.

Reproduce

Reproduced on the lates main, using the following sample code:

#include "ftxui/component/component.hpp"       // for Button, Renderer, Vertical
#include "ftxui/component/screen_interactive.hpp"  // for ScreenInteractive
#include "ftxui/dom/elements.hpp"  // for operator|, text, Element, hbox, separator, size, vbox, border, frame, vscroll_indicator, HEIGHT, LESS_THAN
 
using namespace ftxui;

int main() {
  auto screen = ScreenInteractive::FixedSize(10, 12);
  auto slogon = Renderer([] {
      return vbox({
          text("0000000000"),
          text("1111111111"),
          text("2222222222"),
          text("3333333333"),
          text("4444444444"),
          text("5555555555"),
          text("6666666666"),
          text("7777777777"),
          text("8888888888"),  
          text("9999999999"),
          }) | border;
      });
  screen.Loop(slogon);

  return 0;
}

Root Cause

The problem comes from ScreenInteractive::FixedSize(int dimx, int dimy) factory function, it initialized the parent Screen dimensions to the fixed size at the very beginning, before the first draw.

When ScreenInteractive::Draw() did the first render later, it did an redundant ResetPosition, because the screen's dimy is set to the expected fixed size initially, the ResetPostiion would move up the cursor by dimy lines, although actually in the first render, the cursor should not rewind, because no print happened before it.

Fix

• Always initialize the parent screen to (0,0).
• Store the fixedSize at the ScreenInteractive subclass.
• Use the locally stored fixedSize to initialize the screen dimension at the first render.

Test

[ArthurSonzogni]

Many thanks!

I suspect the CI will output:

[==========] 8 tests from 1 test suite ran. (47 ms total)
[  PASSED  ] 3 tests.
[  FAILED  ] 5 tests, listed below:
[  FAILED  ] SelectionTest.StyleSelection
[  FAILED  ] SelectionTest.VBoxSelection
[  FAILED  ] SelectionTest.VBoxSaturatedSelection
[  FAILED  ] SelectionTest.HBoxSelection
[  FAILED  ] SelectionTest.HBoxSaturatedSelection

I suspect this is because we are now drawing the screen twice and those test aren't expecting this.

I am going to tweak the patch, refactor a bit FTXUI and add a CHANGELOG.

Expect this patch to be merged during the week!

[zozowell]

@ArthurSonzogni ha, you are so fast, I saw the CI error, and working on an update to fix the reorder warning, but just found the upstream has changed... thanks.

[ArthurSonzogni]

Thanks @zozowell