Repository: microsoft/terminal
File: src/host/screenInfo.cpp (C++)

Code before:
        // ...so slide both left/right back into the buffer. This will prevent us
        // from having a negative width later.
        srNewViewport.Right -= offRightDelta;
        srNewViewport.Left = std::max<SHORT>(0, srNewViewport.Left - offRightDelta);
    }
    const SHORT offBottomDelta = srNewViewport.Bottom - (coordScreenBufferSize.Y - 1);
    if (offBottomDelta > 0) // the viewport was off the right of the buffer...
    {
        // ...so slide both top/bottom back into the buffer. This will prevent us
        // from having a negative width later.
        srNewViewport.Bottom -= offBottomDelta;
        srNewViewport.Top = std::max<SHORT>(0, srNewViewport.Top - offBottomDelta);
    }

    // See MSFT:19917443
    // If we're in terminal scrolling mode, and we've changed the height of the
    //      viewport, the new viewport's bottom to the _virtualBottom.
    // GH#1206 - Only do this if the viewport is _growing_ in height. This can
    // cause unexpected behavior if we try to anchor the _virtualBottom to a
    // position that will be greater than the height of the buffer.
    const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
    auto newViewport = Viewport::FromInclusive(srNewViewport);
    if (gci.IsTerminalScrolling() && newViewport.Height() >= _viewport.Height())
    {
        const auto newTop = static_cast<short>(std::max(0, _virtualBottom - (newViewport.Height() - 1)));

        newViewport = Viewport::FromDimensions(COORD({ newViewport.Left(), newTop }), newViewport.Dimensions());
    }

    _viewport = newViewport;
    UpdateBottom();
    Tracing::s_TraceWindowViewport(_viewport);

    // In Conpty mode, call TriggerScroll here without params. By not providing
    // params, the renderer will make sure to update the VtEngine with the
    // updated viewport size. If we don't do this, the engine can get into a
    // torn state on this frame.
    //
    // Without this statement, the engine won't be told about the new view size
    // till the start of the next frame. If any other text gets output before
    // that frame starts, there's a very real chance that it'll cause errors as
    // the engine tries to invalidate those regions.
    if (gci.IsInVtIoMode() && ServiceLocator::LocateGlobals().pRender)
    {
        ServiceLocator::LocateGlobals().pRender->TriggerScroll();
    }
}

// Routine Description:
// - Modifies the size of the current viewport to match the width/height of the request given.
// - Uses the old and new client areas to determine which side the window was resized from.

Review comment (suggestion):
I had a bit trouble understanding this visually until I realized that it basically moves the virtual bottom if the viewport changed the side it is relative to the virtual bottom. In other words: If the viewport used to be above the virtual bottom and is now below, then the virtual bottom is moved and vice versa.

In order to understand this I expressed the boolean condition in an equivalent "numeric" expression. These two lines are equivalent to:
```cpp
const auto delta1 = _virtualBottom - _viewport.BottomInclusive();
const auto delta2 = _virtualBottom - newViewport.BottomInclusive();
// Notes:
// * `delta2 != 0` is required because _virtualBottom and newViewport.BottomInclusive()
//   are only ever compared via < and >
// * `delta1 == 0` is required because _virtualBottom and _viewport.BottomInclusive()
//    are also implicitly compared for == (due to >= / <=)
// * (a ^ b) < 0 is true if a/b have opposing sign bits
delta2 != 0 && (delta1 == 0 || (delta1 ^ delta2) < 0);
```

It seems it doesn't matter whether we check for `delta2 == 0`, because if it is 0, then `_virtualBottom == srNewViewport.Bottom` and the line below won't change anything. This is why we can also write:
```cpp
const auto delta1 = _virtualBottom - _viewport.BottomInclusive();
const auto delta2 = _virtualBottom - newViewport.BottomInclusive();
delta1 == 0 || (delta1 ^ delta2) < 0;
```

With this expression in place I found it easier to understand that the virtual bottom moves if either:
* the virtual bottom is currently following the viewport along (`delta1 == 0`) 
* the new viewport has changed direction and is now on the other side
  relative to the virtual bottom (`(delta1 ^ delta2) < 0`) 

Code after:
    const SHORT offRightDelta = srNewViewport.Right - (coordScreenBufferSize.X - 1);
    if (offRightDelta > 0) // the viewport was off the right of the buffer...
    {
        // ...so slide both left/right back into the buffer. This will prevent us
        // from having a negative width later.
        srNewViewport.Right -= offRightDelta;
        srNewViewport.Left = std::max<SHORT>(0, srNewViewport.Left - offRightDelta);
    }
    const SHORT offBottomDelta = srNewViewport.Bottom - (coordScreenBufferSize.Y - 1);
    if (offBottomDelta > 0) // the viewport was off the right of the buffer...
    {
        // ...so slide both top/bottom back into the buffer. This will prevent us
        // from having a negative width later.
        srNewViewport.Bottom -= offBottomDelta;
        srNewViewport.Top = std::max<SHORT>(0, srNewViewport.Top - offBottomDelta);
    }

    // In general we want to avoid moving the virtual bottom unless it's aligned with
    // the visible viewport, so we check whether the changes we're making would cause
    // the bottom of the visible viewport to intersect the virtual bottom at any point.
    // If so, we update the virtual bottom to match. We also update the virtual bottom
    // if it's less than the new viewport height minus 1, because that would otherwise
    // leave the virtual viewport extended past the top of the buffer.
    const auto newViewport = Viewport::FromInclusive(srNewViewport);
    if ((_virtualBottom >= _viewport.BottomInclusive() && _virtualBottom < newViewport.BottomInclusive()) ||
        (_virtualBottom <= _viewport.BottomInclusive() && _virtualBottom > newViewport.BottomInclusive()) ||
        _virtualBottom < newViewport.Height() - 1)
    {
        _virtualBottom = srNewViewport.Bottom;
    }

    _viewport = newViewport;
    Tracing::s_TraceWindowViewport(_viewport);

    // In Conpty mode, call TriggerScroll here without params. By not providing
    // params, the renderer will make sure to update the VtEngine with the
    // updated viewport size. If we don't do this, the engine can get into a
    // torn state on this frame.
    //
    // Without this statement, the engine won't be told about the new view size
    // till the start of the next frame. If any other text gets output before
    // that frame starts, there's a very real chance that it'll cause errors as
    // the engine tries to invalidate those regions.
    const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
    if (gci.IsInVtIoMode() && ServiceLocator::LocateGlobals().pRender)
    {
        ServiceLocator::LocateGlobals().pRender->TriggerScroll();
    }
}

// Routine Description: