AtlasEngine: Fix dirty rects during scrolling (#17583)

This regressed in #15707. By having the `viewportOffset` on the
`Settings` object we accidentally invalidate the entire viewport
every time it scrolls. That doesn't break anything of course,
but it's better to prevent this.

This PR additionally contains a fix for clamping the y coordinates
coming from `Renderer`: Since `viewportCellCount.y` is a count and
thus exclusive, but clamp's max is inclusive, we must subtract 1.

src/renderer/atlas/AtlasEngine.api.cpp

@@ -98,7 +98,7 @@ constexpr HRESULT vec2_narrow(U x, U y, vec2<T>& out) noexcept
 
 [[nodiscard]] HRESULT AtlasEngine::InvalidateHighlight(std::span<const til::point_span> highlights, const TextBuffer& buffer) noexcept
 {
-    const auto viewportOrigin = til::point{ _api.s->viewportOffset.x, _api.s->viewportOffset.y };
+    const auto viewportOrigin = til::point{ _api.viewportOffset.x, _api.viewportOffset.y };
     const auto viewport = til::rect{ 0, 0, _api.s->viewportCellCount.x, _api.s->viewportCellCount.y };
     const auto cellCountX = static_cast<til::CoordType>(_api.s->viewportCellCount.x);
     for (const auto& hi : highlights)
@@ -221,10 +221,7 @@ try
     {
         _api.s.write()->viewportCellCount = viewportCellCount;
     }
-    if (_api.s->viewportOffset != viewportOffset)
-    {
-        _api.s.write()->viewportOffset = viewportOffset;
-    }
+    _api.viewportOffset = viewportOffset;
 
     return S_OK;
 }

src/renderer/atlas/AtlasEngine.cpp

@@ -144,6 +144,10 @@ try
         _p.MarkAllAsDirty();
     }
 
+#if ATLAS_DEBUG_CONTINUOUS_REDRAW
+    _p.MarkAllAsDirty();
+#endif
+
     if (const auto offset = _p.scrollOffset)
     {
         if (offset < 0)
@@ -243,10 +247,6 @@ try
         }
     }
 
-#if ATLAS_DEBUG_CONTINUOUS_REDRAW
-    _p.MarkAllAsDirty();
-#endif
-
     return S_OK;
 }
 CATCH_RETURN()
@@ -293,8 +293,8 @@ CATCH_RETURN()
 
         // get the buffer origin relative to the viewport, and use it to calculate
         // the dirty region to be relative to the buffer origin
-        const til::CoordType offsetX = _p.s->viewportOffset.x;
-        const til::CoordType offsetY = _p.s->viewportOffset.y;
+        const til::CoordType offsetX = _api.viewportOffset.x;
+        const til::CoordType offsetY = _api.viewportOffset.y;
         const til::point bufferOrigin{ -offsetX, -offsetY };
         const auto dr = _api.dirtyRect.to_origin(bufferOrigin);
 
@@ -325,7 +325,7 @@ CATCH_RETURN()
 
 [[nodiscard]] HRESULT AtlasEngine::PrepareLineTransform(const LineRendition lineRendition, const til::CoordType targetRow, const til::CoordType viewportLeft) noexcept
 {
-    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(targetRow, 0, _p.s->viewportCellCount.y));
+    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(targetRow, 0, _p.s->viewportCellCount.y - 1));
     _p.rows[y]->lineRendition = lineRendition;
     _api.lineRendition = lineRendition;
     return S_OK;
@@ -392,7 +392,7 @@ try
     const til::CoordType y = row;
     const til::CoordType x1 = begX;
     const til::CoordType x2 = endX;
-    const auto offset = til::point{ _p.s->viewportOffset.x, _p.s->viewportOffset.y };
+    const auto offset = til::point{ _api.viewportOffset.x, _api.viewportOffset.y };
     auto it = highlights.begin();
     const auto itEnd = highlights.end();
     auto hiStart = it->start - offset;
@@ -459,15 +459,15 @@ CATCH_RETURN()
 [[nodiscard]] HRESULT AtlasEngine::PaintBufferLine(std::span<const Cluster> clusters, til::point coord, const bool fTrimLeft, const bool lineWrapped) noexcept
 try
 {
-    const auto y = gsl::narrow_cast<u16>(clamp<int>(coord.y, 0, _p.s->viewportCellCount.y));
+    const auto y = gsl::narrow_cast<u16>(clamp<int>(coord.y, 0, _p.s->viewportCellCount.y - 1));
 
     if (_api.lastPaintBufferLineCoord.y != y)
     {
         _flushBufferLine();
     }
 
     const auto shift = gsl::narrow_cast<u8>(_api.lineRendition != LineRendition::SingleWidth);
-    const auto x = gsl::narrow_cast<u16>(clamp<int>(coord.x - (_p.s->viewportOffset.x >> shift), 0, _p.s->viewportCellCount.x));
+    const auto x = gsl::narrow_cast<u16>(clamp<int>(coord.x - (_api.viewportOffset.x >> shift), 0, _p.s->viewportCellCount.x));
     auto columnEnd = x;
 
     // _api.bufferLineColumn contains 1 more item than _api.bufferLine, as it represents the
@@ -509,8 +509,8 @@ CATCH_RETURN()
 try
 {
     const auto shift = gsl::narrow_cast<u8>(_api.lineRendition != LineRendition::SingleWidth);
-    const auto x = std::max(0, coordTarget.x - (_p.s->viewportOffset.x >> shift));
-    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(coordTarget.y, 0, _p.s->viewportCellCount.y));
+    const auto x = std::max(0, coordTarget.x - (_api.viewportOffset.x >> shift));
+    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(coordTarget.y, 0, _p.s->viewportCellCount.y - 1));
     const auto from = gsl::narrow_cast<u16>(clamp<til::CoordType>(x << shift, 0, _p.s->viewportCellCount.x - 1));
     const auto to = gsl::narrow_cast<u16>(clamp<size_t>((x + cchLine) << shift, from, _p.s->viewportCellCount.x));
     const auto glColor = gsl::narrow_cast<u32>(gridlineColor) | 0xff000000;
@@ -533,7 +533,7 @@ try
     // As such we got to call _flushBufferLine() here just to be sure.
     _flushBufferLine();
 
-    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.top, 0, _p.s->viewportCellCount.y));
+    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.top, 0, _p.s->viewportCellCount.y - 1));
     const auto from = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.left, 0, _p.s->viewportCellCount.x - 1));
     const auto to = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.right, from, _p.s->viewportCellCount.x));
 
@@ -576,7 +576,7 @@ try
         const auto top = options.coordCursor.y;
         const auto bottom = top + 1;
         const auto shift = gsl::narrow_cast<u8>(_p.rows[top]->lineRendition != LineRendition::SingleWidth);
-        auto left = options.coordCursor.x - (_p.s->viewportOffset.x >> shift);
+        auto left = options.coordCursor.x - (_api.viewportOffset.x >> shift);
         auto right = left + cursorWidth;
 
         left <<= shift;

src/renderer/atlas/AtlasEngine.h

@@ -179,6 +179,9 @@ namespace Microsoft::Console::Render::Atlas
             u16r invalidatedCursorArea = invalidatedAreaNone;
             range<u16> invalidatedRows = invalidatedRowsNone; // x is treated as "top" and y as "bottom"
             i16 scrollOffset = 0;
+
+            // The position of the viewport inside the text buffer (in cells).
+            u16x2 viewportOffset{ 0, 0 };
         } _api;
     };
 }

src/renderer/atlas/common.h

@@ -409,8 +409,6 @@ namespace Microsoft::Console::Render::Atlas
         u16x2 targetSize{ 0, 0 };
         // Size of the portion of the text buffer that we're drawing on the screen.
         u16x2 viewportCellCount{ 0, 0 };
-        // The position of the viewport inside the text buffer (in cells).
-        u16x2 viewportOffset{ 0, 0 };
     };
 
     using GenerationalSettings = til::generational<Settings>;