starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-06 06:09:50 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

AtlasEngine: Improve robustness against TextBuffer bugs (#13530)

Browse Source 
The current TextBuffer implementation will happily overwrite the
leading/trailing half of a wide glyph with a narrow one without
padding the other half with whitespace. This could crash AtlasEngine
which aggressively guarded against such inconsistencies.

Closes #13522

## Validation Steps Performed
* Run .bat file linked in #13522
  (Override wide glyph with a single space.)
* `AtlasEngine` doesn't crash ✅
This commit is contained in:
Leonard Hecker
2022-07-19 21:27:28 +02:00
committed by GitHub
parent d2b77509f1
commit 784e4ec93a
2 changed files with 36 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1245,7 +1245,7 @@ void AtlasEngine::_flushBufferLine()
// Task: Replace all characters in this range with unicode replacement characters.
// Input (where "n" is a narrow and "ww" is a wide character):
// _api.bufferLine = "nwwnnw"
// _api.bufferLineColumn = {0, 1, 1, 2, 3, 4, 4, 5}
// _api.bufferLineColumn = {0, 1, 1, 3, 4, 5, 5, 6}
// n w w n n w w
// Solution:
// Iterate through bufferLineColumn until the value changes, because this indicates we passed over a
@@ -1293,9 +1293,13 @@ void AtlasEngine::_flushBufferLine()
if (isTextSimple)
{
size_t beg = 0;
for (size_t i = 0; i < complexityLength; ++i)
{
_emplaceGlyph(mappedFontFace.get(), idx + i, idx + i + 1u);
if (_emplaceGlyph(mappedFontFace.get(), idx + beg, idx + i + 1))
{
beg = i + 1;
}
}
}
else
@@ -1410,8 +1414,10 @@ void AtlasEngine::_flushBufferLine()
{
if (_api.textProps[i].canBreakShapingAfter)
{
_emplaceGlyph(mappedFontFace.get(), a.textPosition + beg, a.textPosition + i + 1);
beg = i + 1;
if (_emplaceGlyph(mappedFontFace.get(), a.textPosition + beg, a.textPosition + i + 1))
{
beg = i + 1;
}
}
}
}
@@ -1419,23 +1425,42 @@ void AtlasEngine::_flushBufferLine()
}
}
}
// ^^^ Look at that amazing 8-fold nesting level. Lovely. <3
void AtlasEngine::_emplaceGlyph(IDWriteFontFace* fontFace, size_t bufferPos1, size_t bufferPos2)
bool AtlasEngine::_emplaceGlyph(IDWriteFontFace* fontFace, size_t bufferPos1, size_t bufferPos2)
{
static constexpr auto replacement = L'\uFFFD';
// This would seriously blow us up otherwise.
Expects(bufferPos1 < bufferPos2 && bufferPos2 <= _api.bufferLine.size());
const auto chars = fontFace ? &_api.bufferLine[bufferPos1] : &replacement;
const auto charCount = fontFace ? bufferPos2 - bufferPos1 : 1;
// _flushBufferLine() ensures that bufferLineColumn.size() > bufferLine.size().
const auto x1 = _api.bufferLineColumn[bufferPos1];
const auto x2 = _api.bufferLineColumn[bufferPos2];
Expects(x1 < x2 && x2 <= _api.cellCount.x);
// x1 == x2, if our TextBuffer and DirectWrite disagree where glyph boundaries are. Example:
// Our line of text contains a wide glyph consisting of 2 surrogate pairs "xx" and "yy".
// If DirectWrite considers the first "xx" to be separate from the second "yy", we'll get:
// _api.bufferLine = "...xxyy..."
// _api.bufferLineColumn = {01233335678}
// ^ ^
// / \
// bufferPos1 bufferPos2
// x1: _api.bufferLineColumn[bufferPos1] == 3
// x1: _api.bufferLineColumn[bufferPos2] == 3
// --> cellCount (which is x2 - x1) is now 0 (invalid).
//
// Assuming that the TextBuffer implementation doesn't have any bugs...
// I'm not entirely certain why this occurs, but to me, a layperson, it appears as if
// IDWriteFontFallback::MapCharacters() doesn't respect extended grapheme clusters.
// It could also possibly be due to a difference in the supported Unicode version.
if (x1 >= x2 || x2 > _api.cellCount.x)
{
return false;
}
const auto chars = fontFace ? &_api.bufferLine[bufferPos1] : &replacement;
const auto charCount = fontFace ? bufferPos2 - bufferPos1 : 1;
const u16 cellCount = x2 - x1;
auto attributes = _api.attributes;
@@ -1503,4 +1528,5 @@ void AtlasEngine::_emplaceGlyph(IDWriteFontFace* fontFace, size_t bufferPos1, si
}
std::fill_n(cellGlyphMappings, cellCount, it);
return true;
}

2
src/renderer/atlas/AtlasEngine.h
View File

@@ -876,7 +876,7 @@ namespace Microsoft::Console::Render
TileHashMap::iterator* _getCellGlyphMapping(u16 x, u16 y) noexcept;
void _setCellFlags(u16r coords, CellFlags mask, CellFlags bits) noexcept;
void _flushBufferLine();
void _emplaceGlyph(IDWriteFontFace* fontFace, size_t bufferPos1, size_t bufferPos2);
bool _emplaceGlyph(IDWriteFontFace* fontFace, size_t bufferPos1, size_t bufferPos2);
// AtlasEngine.api.cpp
void _resolveAntialiasingMode() noexcept;