Repository: microsoft/terminal (language: cpp)

Issue:
Some ScrollConsoleScreenBuffer directions have stopped working correctly
### 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:
```cpp
#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:

![image](https://github.com/user-attachments/assets/4f1a6fbd-1827-4fb0-beaf-e7470a88131d)





### 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.

![image](https://github.com/user-attachments/assets/0c9915f0-7a9b-4b6b-80d1-d10d19cbd227)

I'm guessing this might be caused by the new VT implementation of `ScrollConsoleScreenBuffer` from PR #17747. 


Discussion / hints:
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?
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:

```cpp
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.
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.
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!
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.

```cpp
#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;
}
```
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...

Fix patch:
diff --git a/src/host/getset.cpp b/src/host/getset.cpp
index cb3c80d7bbb..96edda199d7 100644
--- a/src/host/getset.cpp
+++ b/src/host/getset.cpp
@@ -995,7 +995,9 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
     try
     {
         // Just in case if the client application didn't check if this request is useless.
-        if (source.left == target.x && source.top == target.y)
+        // Checking if the source is empty also prevents bugs where we use the size of calculations.
+        if ((source.left == target.x && source.top == target.y) ||
+            source.left > source.right || source.top > source.bottom)
         {
             return S_OK;
         }
@@ -1044,23 +1046,23 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
 
             if (gci.GetVtIo()->GetDeviceAttributes().test(Microsoft::Console::VirtualTerminal::DeviceAttribute::RectangularAreaOperations))
             {
-                // This calculates just the positive offsets caused by out-of-bounds (OOB) source and target coordinates.
-                //
-                // If the source rectangle is OOB to the bottom-right, then the size of the rectangle that can
-                // be copied shrinks, but its origin stays the same. However, if the rectangle is OOB to the
-                // top-left then the origin of the to-be-copied rectangle will be offset by an inverse amount.
-                // Similarly, if the *target* rectangle is OOB to the bottom-right, its size shrinks while
-                // the origin stays the same, and if it's OOB to the top-left, then the origin is offset.
-                //
-                // In other words, this calculates the total offset that needs to be applied to the to-be-copied rectangle.
-                // Later down below we'll then clamp that rectangle which will cause its size to shrink as needed.
-                const til::point offset{
-                    std::max(0, -source.left) + std::max(0, clipViewport.Left() - target.x),
-                    std::max(0, -source.top) + std::max(0, clipViewport.Top() - target.y),
-                };
-
-                const auto copyTargetViewport = Viewport::FromDimensions(target + offset, sourceViewport.Dimensions()).Clamp(clipViewport);
-                const auto copySourceViewport = Viewport::FromDimensions(sourceViewport.Origin() + offset, copyTargetViewport.Dimensions()).Clamp(bufferSize);
+                const til::point targetSourceDistance{ target - sourceViewport.Origin() };
+                const til::point sourceTargetDistance{ -targetSourceDistance.x, -targetSourceDistance.y };
+
+                // To figure out what part of "source" we can copy to "target" without
+                // * reading outside the bufferSize
+                // * writing outside the clipViewport
+                // we move the clipViewport into a coordinate system relative to the source rectangle (= clipAtSource).
+                // Then we can intersect the source rectangle with both the valid bufferSize and clipAtSource at once.
+                const auto clipAtSource = Viewport::Offset(clipViewport, sourceTargetDistance);
+                auto copySourceViewport = sourceViewport.Clamp(bufferSize).Clamp(clipAtSource);
+                if (!copySourceViewport.IsValid())
+                {
+                    copySourceViewport = Viewport::Empty();
+                }
+
+                // Afterward we can undo the translation of clipAtSource to get the target rectangle.
+                const auto copyTargetViewport = Viewport::Offset(copySourceViewport, targetSourceDistance);
                 const auto fills = Viewport::Subtract(fillViewport, copyTargetViewport);
                 std::wstring buf;
 
@@ -1069,7 +1071,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
                     Microsoft::Console::VirtualTerminal::VtIo::FormatAttributes(buf, TextAttribute{ fillAttribute });
                 }
 
-                if (copySourceViewport.IsValid() && copyTargetViewport.IsValid())
+                if (copyTargetViewport.IsValid())
                 {
                     // DECCRA: Copy Rectangular Area
                     fmt::format_to(


Test patch: