From c12987af415c5e0911d7a0a81b8494fbe6307328 Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Sat, 20 Aug 2022 01:48:16 +0200
Subject: [PATCH] AtlasEngine: Fix the fix for LRU state after scrolling
 (#13784)

The absolute disgrace of a fix called 65b71ff failed to account for `std::move`
being unsafe to use for overlapping ranges. While `std::move` works for trivial
types (it happens to delegate to `memmove`), we need to dynamically switch
between that and `std::move_backward` to be correct.

Without this fix the LRU refresh is incorrect and might lead to crashes.

## Validation Steps Performed
I'm working on a new, pure D2D renderer inside AtlasEngine, which uses
the iterators contained in `_r.cellGlyphMapping` to draw text.
I noticed the bug, because scrolling up caused the text to be garbled
and with this fix applied it works as expected.
---
 src/renderer/atlas/AtlasEngine.cpp | 56 ++++++++++++++++++------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp
index c6fad7ca09b..fea31c0fb2d 100644
--- a/src/renderer/atlas/AtlasEngine.cpp
+++ b/src/renderer/atlas/AtlasEngine.cpp
@@ -7,7 +7,6 @@
 #include <shader_ps.h>
 #include <shader_vs.h>
 
-#include "../base/FontCache.h"
 #include "../../interactivity/win32/CustomWindowMessages.h"
 
 // #### NOTE ####
@@ -192,42 +191,53 @@ try
             const auto nothingInvalid = _api.invalidatedRows.x == _api.invalidatedRows.y;
             const auto offset = static_cast<ptrdiff_t>(_api.scrollOffset) * _api.cellCount.x;
             auto count = _r.cells.size();
-            ptrdiff_t srcOffset = 0;
-            ptrdiff_t dstOffset = 0;
 
             if (_api.scrollOffset < 0)
             {
                 // Scroll up (for instance when new text is being written at the end of the buffer).
-                srcOffset = -offset;
-                dstOffset = 0;
-                count += offset;
-
                 const u16 endRow = _api.cellCount.y + _api.scrollOffset;
                 _api.invalidatedRows.x = nothingInvalid ? endRow : std::min<u16>(_api.invalidatedRows.x, endRow);
                 _api.invalidatedRows.y = _api.cellCount.y;
+
+                // scrollOffset/offset = -1
+                // +----------+    +----------+
+                // |          |    | xxxxxxxxx|         + dst  < beg
+                // | xxxxxxxxx| -> |xxxxxxx   |  + src  |      < beg - offset
+                // |xxxxxxx   |    |          |  |      v
+                // +----------+    +----------+  v             < end
+                {
+                    const auto beg = _r.cells.begin();
+                    const auto end = beg + count;
+                    std::move(beg - offset, end, beg);
+                }
+                {
+                    const auto beg = _r.cellGlyphMapping.begin();
+                    const auto end = beg + count;
+                    std::move(beg - offset, end, beg);
+                }
             }
             else
             {
                 // Scroll down.
-                srcOffset = 0;
-                dstOffset = offset;
-                count -= offset;
-
                 _api.invalidatedRows.x = 0;
                 _api.invalidatedRows.y = nothingInvalid ? _api.scrollOffset : std::max<u16>(_api.invalidatedRows.y, _api.scrollOffset);
-            }
 
-            {
-                const auto beg = _r.cells.begin();
-                const auto src = beg + srcOffset;
-                const auto dst = beg + dstOffset;
-                std::move(src, src + count, dst);
-            }
-            {
-                const auto beg = _r.cellGlyphMapping.begin();
-                const auto src = beg + srcOffset;
-                const auto dst = beg + dstOffset;
-                std::move(src, src + count, dst);
+                // scrollOffset/offset = 1
+                // +----------+    +----------+
+                // | xxxxxxxxx|    |          |  + src         < beg
+                // |xxxxxxx   | -> | xxxxxxxxx|  |      ^
+                // |          |    |xxxxxxx   |  v      |      < end - offset
+                // +----------+    +----------+         + dst  < end
+                {
+                    const auto beg = _r.cells.begin();
+                    const auto end = beg + count;
+                    std::move_backward(beg, end - offset, end);
+                }
+                {
+                    const auto beg = _r.cellGlyphMapping.begin();
+                    const auto end = beg + count;
+                    std::move_backward(beg, end - offset, end);
+                }
             }
         }
     }