summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>2021-01-07 08:20:42 +0100
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2021-02-03 20:38:48 +0000
commitdbaac6e5c1c1725055187bd5473f95de78f5b408 (patch)
treee84c39a5b3410af2c58f50be18f43a8a02d3f09b
parent0afad46bb71b106780dd641de768a81d9f6c6a51 (diff)
Remove false Q_UNREACHABLE from shaping code
This was added by 9ff76c27b9031ae7c49c4c9e8b5a3bea1e0e3c78 on the basis that it signifies a shaping error and would later assert or crash. But the line is easily reachable by user code. If Harfbuzz returns 0 glyphs, it just means it is unable to shape the string, for instance if the input string only contains default ignorables (like a ZWJ) and does not have any appropriate glyph to use for replacement. Qt expects there to always be at least one glyph in the output (num_glyphs == 0 is used to indicate shaping is not yet done), so to avoid asserts later on, we simply populate the output with a single 0 token, which is a required entry in the font that is reserved for representing unrepresentable characters. This also adds a test and therefore a zero-width joiner to the test font to reproduce the issue. [ChangeLog][QtGui][Text] Fixed a possible crash with certain fonts when shaping strings consisting only of control characters. Fixes: QTBUG-89155 Change-Id: Ia0dd6a04844c9be90dcab6c464bebe339a3dab11 Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com> (cherry picked from commit fccd419dd632306a4bd85928223e0a56a59510ef) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/gui/text/qtextengine.cpp13
-rw-r--r--tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp16
-rw-r--r--tests/auto/shared/resources/test.ttfbin2008 -> 2128 bytes
3 files changed, 27 insertions, 2 deletions
diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp
index 9a7d3283b8..5161323167 100644
--- a/src/gui/text/qtextengine.cpp
+++ b/src/gui/text/qtextengine.cpp
@@ -1547,12 +1547,21 @@ void QTextEngine::shapeText(int item) const
} else {
si.num_glyphs = shapeTextWithHarfbuzz(si, string, itemLength, fontEngine, itemBoundaries, kerningEnabled);
}
+
if (Q_UNLIKELY(si.num_glyphs == 0)) {
- Q_UNREACHABLE(); // ### report shaping errors somehow
+ if (Q_UNLIKELY(!ensureSpace(si.glyph_data_offset + 1))) {
+ qWarning() << "Unable to allocate space for place-holder glyph";
+ return;
+ }
+
+ si.num_glyphs = 1;
+
+ // Overwrite with 0 token to indicate failure
+ QGlyphLayout g = availableGlyphs(&si);
+ g.glyphs[0] = 0;
return;
}
-
layoutData->used += si.num_glyphs;
QGlyphLayout glyphs = shapedGlyphs(&si);
diff --git a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
index 1429e4cb7f..98e07919ef 100644
--- a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
+++ b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
@@ -66,6 +66,7 @@ private slots:
void boundingRect();
void mixedScripts();
void multiLineBoundingRect();
+ void defaultIgnorables();
private:
int m_testFontId;
@@ -761,6 +762,21 @@ void tst_QGlyphRun::multiLineBoundingRect()
QVERIFY(firstLineGlyphRun.boundingRect().height() < allGlyphRun.boundingRect().height());
}
+void tst_QGlyphRun::defaultIgnorables()
+{
+ QTextLayout layout;
+ layout.setFont(QFont("QtsSpecialTestFont"));
+ layout.setText(QChar(0x200D));
+ layout.beginLayout();
+ layout.createLine();
+ layout.endLayout();
+
+ QList<QGlyphRun> runs = layout.glyphRuns();
+ QCOMPARE(runs.size(), 1);
+ QCOMPARE(runs.at(0).glyphIndexes().size(), 1);
+ QCOMPARE(runs.at(0).glyphIndexes()[0], 0);
+}
+
#endif // QT_NO_RAWFONT
QTEST_MAIN(tst_QGlyphRun)
diff --git a/tests/auto/shared/resources/test.ttf b/tests/auto/shared/resources/test.ttf
index 382b2547b0..b2bb6cd036 100644
--- a/tests/auto/shared/resources/test.ttf
+++ b/tests/auto/shared/resources/test.ttf
Binary files differ