folkslinux
/
windows-terminal
mirror of https://github.com/microsoft/terminal.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Leonard Hecker 2022-08-20 01:48:16 +02:00 committed by GitHub
parent 64bcc0bd25
commit c12987af41
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 33 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/renderer/atlas/AtlasEngine.cpp
View File

@ -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);
}
}
}
}