Skip to content

Commit

Permalink
Fix several regressions in font selection
Browse files Browse the repository at this point in the history
In Qt 5.3.0 a change was added which automatically adapts Common
script to surrounding scripts in accordance with the Unicode tr#24.

This broke *a lot* of cases of font selection because the font
selection algorithm is not prepared for handling characters with
adapted scripts. We need to disable this change for now and redo it
later with patches to font selection to avoid the regressions.

[ChangeLog][Text] Fixed several regressions in font selection when
combining different writing systems in the same text.

Task-number: QTBUG-39930
Task-number: QTBUG-39860
Change-Id: Id02b5ae2403c06542ed5d81e7c4deb2e0c7d816e
Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com>
  • Loading branch information
Eskil Abrahamsen Blomfeldt committed Aug 20, 2014
1 parent 29787ec commit 198009d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
6 changes: 6 additions & 0 deletions src/corelib/tools/qunicodetools.cpp
Expand Up @@ -710,6 +710,7 @@ Q_CORE_EXPORT void initScripts(const ushort *string, int length, uchar *scripts)
script = QChar::Script(prop->script);
}

#if 0 // ### Disabled due to regressions. The font selection algorithm is not prepared for this change.
if (Q_LIKELY(script != QChar::Script_Common)) {
// override preceding Common-s
while (sor > 0 && scripts[sor - 1] == QChar::Script_Common)
Expand All @@ -719,13 +720,16 @@ Q_CORE_EXPORT void initScripts(const ushort *string, int length, uchar *scripts)
if (sor > 0)
script = scripts[sor - 1];
}
#endif

while (sor < eor)
scripts[sor++] = script;

script = prop->script;
}
eor = length;

#if 0 // ### Disabled due to regressions. The font selection algorithm is not prepared for this change.
if (Q_LIKELY(script != QChar::Script_Common)) {
// override preceding Common-s
while (sor > 0 && scripts[sor - 1] == QChar::Script_Common)
Expand All @@ -735,6 +739,8 @@ Q_CORE_EXPORT void initScripts(const ushort *string, int length, uchar *scripts)
if (sor > 0)
script = scripts[sor - 1];
}
#endif

while (sor < eor)
scripts[sor++] = script;
}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/text/qtextengine.cpp
Expand Up @@ -124,7 +124,7 @@ class Itemizer {
for (int i = start + 1; i < end; ++i) {
if (m_analysis[i].bidiLevel == m_analysis[start].bidiLevel
&& m_analysis[i].flags == m_analysis[start].flags
&& m_analysis[i].script == m_analysis[start].script
&& (m_analysis[i].script == m_analysis[start].script || m_string[i] == QLatin1Char('.'))
&& m_analysis[i].flags < QScriptAnalysis::SpaceTabOrObject
&& i - start < MaxItemLength)
continue;
Expand Down
Binary file modified tests/auto/gui/text/qglyphrun/test.ttf
Binary file not shown.
20 changes: 19 additions & 1 deletion tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
Expand Up @@ -77,6 +77,7 @@ private slots:
void setRawData();
void setRawDataAndGetAsVector();
void boundingRect();
void mixedScripts();

private:
int m_testFontId;
Expand Down Expand Up @@ -399,7 +400,7 @@ void tst_QGlyphRun::setRawDataAndGetAsVector()
void tst_QGlyphRun::drawNonExistentGlyphs()
{
QVector<quint32> glyphIndexes;
glyphIndexes.append(3);
glyphIndexes.append(4);

QVector<QPointF> glyphPositions;
glyphPositions.append(QPointF(0, 0));
Expand Down Expand Up @@ -725,6 +726,23 @@ void tst_QGlyphRun::boundingRect()
QCOMPARE(glyphs.boundingRect(), boundingRect);
}

void tst_QGlyphRun::mixedScripts()
{
QString s;
s += QChar(0x31); // The character '1'
s += QChar(0xbc14); // Hangul character

QTextLayout layout;
layout.setFont(m_testFont);
layout.setText(s);
layout.beginLayout();
layout.createLine();
layout.endLayout();

QList<QGlyphRun> glyphRuns = layout.glyphRuns();
QCOMPARE(glyphRuns.size(), 2);
}

#endif // QT_NO_RAWFONT

QTEST_MAIN(tst_QGlyphRun)
Expand Down
22 changes: 15 additions & 7 deletions tests/auto/gui/text/qtextscriptengine/tst_qtextscriptengine.cpp
Expand Up @@ -1258,21 +1258,29 @@ void tst_QTextScriptEngine::thaiWithZWJ()
QTextLayout layout(s, font);
QTextEngine *e = layout.engine();
e->itemize();
QCOMPARE(e->layoutData->items.size(), 3);
QCOMPARE(e->layoutData->items.size(), 11);

for (int item = 0; item < e->layoutData->items.size(); ++item)
e->shape(item);

QCOMPARE(e->layoutData->items[0].num_glyphs, ushort(15)); // Thai: The ZWJ and ZWNJ characters are inherited, so should be part of the thai script
QCOMPARE(e->layoutData->items[1].num_glyphs, ushort(1)); // Han: Kanji for tree
QCOMPARE(e->layoutData->items[2].num_glyphs, ushort(2)); // Thai: Thai character followed by superscript "a" which is of inherited type
QCOMPARE(e->layoutData->items[0].num_glyphs, ushort(7)); // Thai: The ZWJ and ZWNJ characters are inherited, so should be part of the thai script
QCOMPARE(e->layoutData->items[1].num_glyphs, ushort(1)); // Common: The smart quotes cannot be handled by thai, so should be a separate item
QCOMPARE(e->layoutData->items[2].num_glyphs, ushort(1)); // Thai: Thai character
QCOMPARE(e->layoutData->items[3].num_glyphs, ushort(1)); // Common: Ellipsis
QCOMPARE(e->layoutData->items[4].num_glyphs, ushort(1)); // Thai: Thai character
QCOMPARE(e->layoutData->items[5].num_glyphs, ushort(1)); // Common: Smart quote
QCOMPARE(e->layoutData->items[6].num_glyphs, ushort(1)); // Thai: Thai character
QCOMPARE(e->layoutData->items[7].num_glyphs, ushort(1)); // Common: \xA0 = non-breaking space. Could be useful to have in thai, but not currently implemented
QCOMPARE(e->layoutData->items[8].num_glyphs, ushort(1)); // Thai: Thai character
QCOMPARE(e->layoutData->items[9].num_glyphs, ushort(1)); // Japanese: Kanji for tree
QCOMPARE(e->layoutData->items[10].num_glyphs, ushort(2)); // Thai: Thai character followed by superscript "a" which is of inherited type

//A quick sanity check - check all the characters are individual clusters
unsigned short *logClusters = e->layoutData->logClustersPtr;
for (int i = 0; i <= 14; i++)
for (int i = 0; i < 7; i++)
QCOMPARE(logClusters[i], ushort(i));
QCOMPARE(logClusters[15], ushort(0));
QCOMPARE(logClusters[16], ushort(0));
for (int i = 0; i < 10; i++)
QCOMPARE(logClusters[i+7], ushort(0));
#ifndef Q_OS_MAC
// ### Result differs for HarfBuzz-NG
QCOMPARE(logClusters[17], ushort(1));
Expand Down

0 comments on commit 198009d

Please sign in to comment.