Some ScrollConsoleScreenBuffer directions have stopped working correctly

[j4james]

Windows Terminal version

Commit f93347ed4bbeea0d9e663f2069994d9e2d069f10

Windows build number

10.0.19045.4780

Other Software

No response

Steps to reproduce

Compile and run the following code in a recent build of Windows Terminal:

#include <windows.h>
#include <stdio.h>

void scroll(HANDLE h, int left, int top, int right, int bottom, int dx, int dy)
{
  SHORT x1 = min(left+dx, left);
  SHORT x2 = max(right+dx, right);
  SHORT y1 = min(top+dy, top);
  SHORT y2 = max(bottom+dy, bottom);
  SMALL_RECT rect = {x1-1, y1-1, x2-1, y2-1};
  SHORT ox = x1+dx-1;
  SHORT oy = y1+dy-1;
  CHAR_INFO fill = {'+', 0x56};
  ScrollConsoleScreenBuffer(h, &rect, &rect, {ox,oy}, &fill);
}

int main(int argc, char* argv[])
{
  DWORD mode;
  HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
  GetConsoleMode(h, &mode);
  mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
  SetConsoleMode(h, mode);

  printf("\033#8");

  scroll(h,39,8,42,10,0,-2);
  scroll(h,39,15,42,17,0,2);
  scroll(h,31,12,36,13,-4,0);
  scroll(h,45,12,50,13,4,0);

  SetConsoleMode(h, mode);
  return 0;
}

Expected Behavior

It should fill the screen with a DECALN pattern of E characters, and then scroll slices of that buffer up, down, left, and right, leaving four areas of purple with yellow + characters in the "erased" space revealed by the scrolling.

You should end up with a pattern something like this:

Actual Behavior

It looks like some of the scrolling directions aren't working correctly, so you only end up with two of the purple areas showing.

I'm guessing this might be caused by the new VT implementation of ScrollConsoleScreenBuffer from PR #17747.

[j4james]

I can't claim to fully understand this code, but my first impression is that this line looks wrong:

https://github.com/microsoft/terminal/blob/1f71568c2a60fa7ec3549e72bc1c8c47c7033b29/src/host/getset.cpp#L1062

If we're applying an offset to the target origin, then shouldn't we also be shrinking the dimensions by the same amount?

[j4james]

I've just tried now, and at least my test case can be fixed by subtracting the offset from the dimensions, i.e. changing the line quoted above to:

const auto copyTargetViewport = Viewport::FromDimensions(target + offset, sourceViewport.Dimensions() - offset).Clamp(clipViewport);

But I don't know if there are more complicated edge cases that I'm missing.

[j4james]

FYI, I've done a bit more testing with rectangles that are partially off screen, and there are definitely more issues than the ones I've described above. That offset change by itself is not going to be enough to fix everything.

[lhecker]

I kind of expected that I didn't get the math right but wasn't able to figure in what way before the release had to be ready (last Friday). Thank you for investigating this!

[j4james]

In case it helps, I've included my extended test case below. It's intended to be run in an 80x24 window and performs a series of moves in 8 different directions, using rects in different areas of the screen (first in the center, and then the four corners, with the latter tests extending partly outside the window boundaries).

Each test starts with a background fill designed to make the expected block movement somewhat easier to track. When you press a key, the scroll will be performed, and when you press another key it'll proceed with the next test. If you run the tests in an old preview build alongside the current code you can easily see where it's getting things wrong.

I'm not sure this covers all edge case, but it's at least something to start with.

#include <windows.h>
#include <stdio.h>
#include <conio.h>
#include <array>
#include <tuple>

void scroll(HANDLE h, int left, int top, int right, int bottom, int dx, int dy)
{
  SMALL_RECT rect = {SHORT(left-1), SHORT(top-1), SHORT(right-1), SHORT(bottom-1)};
  SHORT ox = left+dx-1;
  SHORT oy = top+dy-1;
  CHAR_INFO fill = {'+', 0x56};
  ScrollConsoleScreenBuffer(h, &rect, &rect, {ox,oy}, &fill);
}

void background(HANDLE h, int cx, int cy, int dx, int dy)
{
  printf("\033[H");
  for (auto y = 0; y < 24; y++) {
    for (auto x = 0; x < 80; x++) {
      auto ybased = false;
      if (dx == 0)
        ybased = true;
      else if (dy == 0)
        ybased = false;
      else {
        auto ey = (x - cx) * dy / dx + cy;
        ybased = ey >= y;
      }
      if (ybased)
        printf("%c",char(y%10 + 48));
      else
        printf("%c",char(x%26 + 65));
    }
  }
  printf("\033[1;30H");
  printf(" %d;%d move %+d,%+d ", cx, cy, dx, dy);
}

int main(int argc, char* argv[])
{
  DWORD mode;
  HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
  GetConsoleMode(h, &mode);
  mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
  SetConsoleMode(h, mode);

  const std::array<std::pair<int,int>,5> tests = {{
    {40,12}, {3,2}, {78,2}, {3,23}, {78,23}
  }};
  for (auto [cx,cy] : tests) {
    for (auto dy = -2; dy <= 2; dy += 2) {
      for (auto dx = -4; dx <= 4; dx += 4) {
        if (!dx && !dy) continue;
        background(h,cx,cy,dx,dy);
        getch();
        scroll(h,cx-4,cy-2,cx+4,cy+2,dx,dy);
        getch();
      }
    }
  }

  printf("\033[2J");
  SetConsoleMode(h, mode);
  return 0;
}

[lhecker]

Your example code is super helpful for my testing btw!

It also made me notice today that the Windows CRT does not support line buffering. (??) That certainly surprised me a lot. It may also be a hint towards why people think that console IO is so slow. Your snippet for instance becomes dramatically faster if you add setvbuf(stdout, NULL, _IOFBF, 4096); and fflush(). It shudders me to think about how much software would be break if the CRT suddenly added _IOLBF support and used it by default like other platforms. Oh well...