Fix 32-bit Unicode truncation and QChar assertion crashes#633
Draft
wineee wants to merge 1 commit intolxqt:masterfrom
Draft
Fix 32-bit Unicode truncation and QChar assertion crashes#633wineee wants to merge 1 commit intolxqt:masterfrom
wineee wants to merge 1 commit intolxqt:masterfrom
Conversation
This commit addresses crashes caused by Qt 6 enforcing strict range checks (<= 0xFFFF) in the QChar(uint) constructor, as well as fixing internal truncation issues for characters outside the Basic Multilingual Plane (BMP). - Converted TerminalDisplay::charClass to return char32_t instead of QChar, fixing assertions in word extraction algorithms. - Replaced QChar-based unicode checks directly initialized from wchar_t with safe static QChar properties (QChar::category, QChar::isSpace) acting on char32_t values. - Replaced slow and unsafe character width calculations with a fast-path surrogate check (QChar::requiresSurrogates), only allocating QStrings for extended glyphs, avoiding rendering loop performance penalties. - Fixed Character(quint16 _c) constructor inadvertently truncating 32-bit codepoints. - Converted CompactHistoryLine properties to uint* rather than quint16* to prevent scrollback from discarding upper bits of wide characters.
wineee
commented
Apr 17, 2026
| * @param _r A set of rendition flags which specify how this character is to be drawn. | ||
| */ | ||
| inline Character(quint16 _c = ' ', | ||
| inline Character(wchar_t _c = ' ', |
Contributor
Author
There was a problem hiding this comment.
kconsole switched from quint16 to uint, and qtermwidget was changed to wchat_t, but it seems some parts were missed.
Starting from Qt6, some function parameters of QChar have been changed from uint to char32_t, which is a C++ standard type. I think it might be better to switch wchar_t to char32_t...
wineee
commented
Apr 17, 2026
| bool lineDraw = isLineChar(newLine[x+0]); | ||
| bool doubleWidth = (x+1 == columnsToUpdate) ? false : (newLine[x+1].character == 0); | ||
| int charWidth = fm.horizontalAdvance(QChar(c)); | ||
| int charWidth = QChar::requiresSurrogates(c) ? fm.horizontalAdvance(QString::fromWCharArray(&c, 1)) : fm.horizontalAdvance(QChar(static_cast<ushort>(c))); |
Contributor
Author
There was a problem hiding this comment.
It's not clear if there is a more efficient approach here, as konsole's logic has changed a bit and didn't use horizontalAdvance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit addresses crashes caused by Qt 6 enforcing strict range checks (<= 0xFFFF) in the QChar(uint) constructor, as well as fixing internal truncation issues for characters outside the Basic Multilingual Plane (BMP).
fix: #632