Fix missing text when Harfbuzz fails to shape a substring

This amends fccd419dd6.

If Harfbuzz failed on one of the items in a string and returned
zero glyphs, then we would exit the shaping loop. The mentioned
change fixed a crash related to this when the ignored character
was the only character in the string, but it occurred in a
subitem of a longer string, then we would return and fail to
lay out the rest of the string.

This popped up recently because an update to Harfbuzz has caused
it to return zero glyphs when applying the Apple emoji font to
an isolated variant character (see bug report). When we matched
the symbol to the main font and only the variant character to
the emoji font, we would get in this situation, and end up
exiting the shaping early.

[ChangeLog][QtGui][Text] Fixed a regression which would
sometimes cause text to disappear if the string contained
an unmatched variation selector character.

Pick-to: 6.4
Fixes: QTBUG-108799
Change-Id: I616ab1b2d33c2df731419c5ce06fbc578a625a32
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This commit is contained in:
Eskil Abrahamsen Blomfeldt 2022-11-28 14:24:45 +01:00
parent 64847a3930
commit c9d991de1f
2 changed files with 71 additions and 34 deletions

View File

@ -1528,7 +1528,6 @@ void QTextEngine::shapeText(int item) const
QGlyphLayout g = availableGlyphs(&si);
g.glyphs[0] = 0;
g.attributes[0].clusterStart = true;
g.attributes[0].dontPrint = true;
ushort *log_clusters = logClusters(&si);
for (int i = 0; i < itemLength; ++i)
@ -1675,9 +1674,14 @@ int QTextEngine::shapeTextWithHarfbuzzNG(const QScriptItem &si,
hb_buffer_reverse(buffer);
}
const uint num_glyphs = hb_buffer_get_length(buffer);
uint num_glyphs = hb_buffer_get_length(buffer);
const bool has_glyphs = num_glyphs > 0;
// If Harfbuzz returns zero glyphs, we have to manually add a missing glyph
if (Q_UNLIKELY(!has_glyphs))
num_glyphs = 1;
// ensure we have enough space for shaped glyphs and metrics
if (Q_UNLIKELY(num_glyphs == 0 || !ensureSpace(glyphs_shaped + num_glyphs))) {
if (Q_UNLIKELY(!ensureSpace(glyphs_shaped + num_glyphs))) {
hb_buffer_destroy(buffer);
return 0;
}
@ -1685,35 +1689,44 @@ int QTextEngine::shapeTextWithHarfbuzzNG(const QScriptItem &si,
// fetch the shaped glyphs and metrics
QGlyphLayout g = availableGlyphs(&si).mid(glyphs_shaped, num_glyphs);
ushort *log_clusters = logClusters(&si) + item_pos;
if (Q_LIKELY(has_glyphs)) {
hb_glyph_info_t *infos = hb_buffer_get_glyph_infos(buffer, nullptr);
hb_glyph_position_t *positions = hb_buffer_get_glyph_positions(buffer, nullptr);
uint str_pos = 0;
uint last_cluster = ~0u;
uint last_glyph_pos = glyphs_shaped;
for (uint i = 0; i < num_glyphs; ++i, ++infos, ++positions) {
g.glyphs[i] = infos->codepoint;
hb_glyph_info_t *infos = hb_buffer_get_glyph_infos(buffer, nullptr);
hb_glyph_position_t *positions = hb_buffer_get_glyph_positions(buffer, nullptr);
uint str_pos = 0;
uint last_cluster = ~0u;
uint last_glyph_pos = glyphs_shaped;
for (uint i = 0; i < num_glyphs; ++i, ++infos, ++positions) {
g.glyphs[i] = infos->codepoint;
g.advances[i] = QFixed::fromFixed(positions->x_advance);
g.offsets[i].x = QFixed::fromFixed(positions->x_offset);
g.offsets[i].y = QFixed::fromFixed(positions->y_offset);
g.advances[i] = QFixed::fromFixed(positions->x_advance);
g.offsets[i].x = QFixed::fromFixed(positions->x_offset);
g.offsets[i].y = QFixed::fromFixed(positions->y_offset);
uint cluster = infos->cluster;
if (Q_LIKELY(last_cluster != cluster)) {
g.attributes[i].clusterStart = true;
uint cluster = infos->cluster;
if (Q_LIKELY(last_cluster != cluster)) {
g.attributes[i].clusterStart = true;
// fix up clusters so that the cluster indices will be monotonic
// and thus we never return out-of-order indices
while (last_cluster++ < cluster && str_pos < item_length)
log_clusters[str_pos++] = last_glyph_pos;
last_glyph_pos = i + glyphs_shaped;
last_cluster = cluster;
// fix up clusters so that the cluster indices will be monotonic
// and thus we never return out-of-order indices
while (last_cluster++ < cluster && str_pos < item_length)
log_clusters[str_pos++] = last_glyph_pos;
last_glyph_pos = i + glyphs_shaped;
last_cluster = cluster;
applyVisibilityRules(string[item_pos + str_pos], &g, i, actualFontEngine);
applyVisibilityRules(string[item_pos + str_pos], &g, i, actualFontEngine);
}
}
while (str_pos < item_length)
log_clusters[str_pos++] = last_glyph_pos;
} else { // Harfbuzz did not return a glyph for the character, so we add a placeholder
g.glyphs[0] = 0;
g.advances[0] = QFixed{};
g.offsets[0].x = QFixed{};
g.offsets[0].y = QFixed{};
g.attributes[0].clusterStart = true;
g.attributes[0].dontPrint = true;
log_clusters[0] = glyphs_shaped;
}
while (str_pos < item_length)
log_clusters[str_pos++] = last_glyph_pos;
if (Q_UNLIKELY(engineIdx != 0)) {
for (quint32 i = 0; i < num_glyphs; ++i)

View File

@ -608,15 +608,39 @@ void tst_QGlyphRun::multiLineBoundingRect()
void tst_QGlyphRun::defaultIgnorables()
{
QTextLayout layout;
layout.setFont(QFont("QtsSpecialTestFont"));
layout.setText(QChar(0x200D));
layout.beginLayout();
layout.createLine();
layout.endLayout();
{
QTextLayout layout;
layout.setFont(QFont("QtsSpecialTestFont"));
layout.setText(QChar(0x200D));
layout.beginLayout();
layout.createLine();
layout.endLayout();
QList<QGlyphRun> runs = layout.glyphRuns();
QCOMPARE(runs.size(), 0);
QList<QGlyphRun> runs = layout.glyphRuns();
QCOMPARE(runs.size(), 0);
}
{
QTextLayout layout;
layout.setFont(QFont("QtsSpecialTestFont"));
layout.setText(QStringLiteral("AAA") + QChar(0xFE0F) + QStringLiteral("111"));
layout.beginLayout();
layout.createLine();
layout.endLayout();
QList<QGlyphRun> runs = layout.glyphRuns();
QVERIFY(!runs.isEmpty());
bool hasFullMainFontRun = false;
for (const QGlyphRun &run : runs) {
if (run.rawFont().familyName() == QStringLiteral("QtsSpecialTestFont")
&& run.glyphIndexes().size() == 6) {
hasFullMainFontRun = true;
break;
}
}
QVERIFY(hasFullMainFontRun);
}
}
void tst_QGlyphRun::stringIndexes()