From 0a2b660e649bfc07b5142640b3fea56b1d8f564c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 22 Jul 2024 14:48:52 +0200 Subject: [PATCH] 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 | 7 ++----- src/renderer/atlas/AtlasEngine.cpp | 28 +++++++++++++------------- src/renderer/atlas/AtlasEngine.h | 3 +++ src/renderer/atlas/common.h | 2 -- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 15535ad89f0..17e7d2f8a5e 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -98,7 +98,7 @@ constexpr HRESULT vec2_narrow(U x, U y, vec2& out) noexcept [[nodiscard]] HRESULT AtlasEngine::InvalidateHighlight(std::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(_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; } diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp index 7bc2755721b..8464bef58a3 100644 --- a/src/renderer/atlas/AtlasEngine.cpp +++ b/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(clamp(targetRow, 0, _p.s->viewportCellCount.y)); + const auto y = gsl::narrow_cast(clamp(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,7 +459,7 @@ CATCH_RETURN() [[nodiscard]] HRESULT AtlasEngine::PaintBufferLine(std::span clusters, til::point coord, const bool fTrimLeft, const bool lineWrapped) noexcept try { - const auto y = gsl::narrow_cast(clamp(coord.y, 0, _p.s->viewportCellCount.y)); + const auto y = gsl::narrow_cast(clamp(coord.y, 0, _p.s->viewportCellCount.y - 1)); if (_api.lastPaintBufferLineCoord.y != y) { @@ -467,7 +467,7 @@ try } const auto shift = gsl::narrow_cast(_api.lineRendition != LineRendition::SingleWidth); - const auto x = gsl::narrow_cast(clamp(coord.x - (_p.s->viewportOffset.x >> shift), 0, _p.s->viewportCellCount.x)); + const auto x = gsl::narrow_cast(clamp(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(_api.lineRendition != LineRendition::SingleWidth); - const auto x = std::max(0, coordTarget.x - (_p.s->viewportOffset.x >> shift)); - const auto y = gsl::narrow_cast(clamp(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(clamp(coordTarget.y, 0, _p.s->viewportCellCount.y - 1)); const auto from = gsl::narrow_cast(clamp(x << shift, 0, _p.s->viewportCellCount.x - 1)); const auto to = gsl::narrow_cast(clamp((x + cchLine) << shift, from, _p.s->viewportCellCount.x)); const auto glColor = gsl::narrow_cast(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(clamp(rect.top, 0, _p.s->viewportCellCount.y)); + const auto y = gsl::narrow_cast(clamp(rect.top, 0, _p.s->viewportCellCount.y - 1)); const auto from = gsl::narrow_cast(clamp(rect.left, 0, _p.s->viewportCellCount.x - 1)); const auto to = gsl::narrow_cast(clamp(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(_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; diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index e022a38c3fa..135d0e692a9 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -179,6 +179,9 @@ namespace Microsoft::Console::Render::Atlas u16r invalidatedCursorArea = invalidatedAreaNone; range 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; }; } diff --git a/src/renderer/atlas/common.h b/src/renderer/atlas/common.h index 9651efd45c8..270dd35f9dd 100644 --- a/src/renderer/atlas/common.h +++ b/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;