Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AtlasEngine: Fix a crash when drawing double width rows #13966

Merged
2 commits merged into from
Sep 15, 2022
Merged

AtlasEngine: Fix a crash when drawing double width rows #13966

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cf7f41
AtlasEngine: Fix a crash when drawing double width rows
lhecker Sep 12, 2022
c21e35a
Address feedback, Fix AuditMode
lhecker Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/renderer/atlas/AtlasEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,21 @@ void AtlasEngine::_recreateSizeDependentResources()
_api.glyphProps = Buffer<DWRITE_SHAPING_GLYPH_PROPERTIES>{ projectedGlyphSize };
_api.glyphAdvances = Buffer<f32>{ projectedGlyphSize };
_api.glyphOffsets = Buffer<DWRITE_GLYPH_OFFSET>{ projectedGlyphSize };

// Initialize cellGlyphMapping with valid data (whitespace), so that it can be
// safely used by the TileHashMap refresh logic via makeNewest() in StartPaint().
{
u16x2* coords;
AtlasKey key{ { .cellCount = 1 }, 1, L" " };
AtlasValue value{ CellFlags::None, 1, &coords };

coords[0] = _r.tileAllocator.allocate(_r.glyphs);

const auto it = _r.glyphs.insert(std::move(key), std::move(value));
_r.glyphQueue.emplace_back(it);

std::ranges::fill(_r.cellGlyphMapping, it);
lhecker marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crash occurs when the viewport size changes causing the recreation of the cellGlyphMapping array and the first paint includes non-single-width line rendition rows. In that case the cellGlyphMapping array will have items that are still default-constructed pointers (null pointers in debug, uninitialized data in release mode), leading to a crash.

(I had to submit this comment again, because I moved this new code slightly.)

}

if (!_r.d2dMode)
Expand Down Expand Up @@ -1176,6 +1191,15 @@ void AtlasEngine::_flushBufferLine()
// This would seriously blow us up otherwise.
Expects(_api.bufferLineColumn.size() == _api.bufferLine.size() + 1);

// GH#13962: With the lack of proper LineRendition support, just fill
// the remaining columns with whitespace to prevent any weird artifacts.
for (auto lastColumn = _api.bufferLineColumn.back(); lastColumn < _api.cellCount.x;)
{
++lastColumn;
_api.bufferLine.emplace_back(L' ');
_api.bufferLineColumn.emplace_back(lastColumn);
}

// NOTE:
// This entire function is one huge hack to see if it works.

Expand Down