From 035d80407b693662c209cd4f156085d583ad428c Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 25 Jan 2019 10:49:34 +0100 Subject: Fix two smaller bugs in the BiDi engine Remove wrong code changing the Bido level of line separators. This lead to wrong ordering of the string in case the line separator was meant to be ignored and the string should be rendered in one line. Line breaks are anyways already reset to the paragraph level by the algorithm and reordering is done on a line by line basis, so this will work correctly when doing proper line breaking. Secondly fix a small bug found while testing the above change, where we wouldn't set the correct levels for boundary neutrals and explicit embedding chars because we did that processing before we were fully done with the BiDi algorithm. Change-Id: Id88f91cd58d2ab29be864aef34ca1727c1586611 Reviewed-by: Konstantin Ritt Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtextengine.cpp | 34 ++++++++++------------ tests/auto/other/qcomplextext/bidireorderstring.h | 2 +- tests/auto/other/qcomplextext/tst_qcomplextext.cpp | 7 ++++- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index f305b9b7dc..a83ef95c79 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -1108,6 +1108,22 @@ struct QBidiAlgorithm { resolveImplicitLevels(runs); } + BIDI_DEBUG() << "Rule L1:"; + // Rule L1: + bool resetLevel = true; + for (int i = length - 1; i >= 0; --i) { + if (analysis[i].bidiFlags & QScriptAnalysis::BidiResetToParagraphLevel) { + BIDI_DEBUG() << "resetting pos" << i << "to baselevel"; + analysis[i].bidiLevel = baseLevel; + resetLevel = true; + } else if (resetLevel && analysis[i].bidiFlags & QScriptAnalysis::BidiMaybeResetToParagraphLevel) { + BIDI_DEBUG() << "resetting pos" << i << "to baselevel (maybereset flag)"; + analysis[i].bidiLevel = baseLevel; + } else { + resetLevel = false; + } + } + // set directions for BN to the minimum of adjacent chars // This makes is possible to be conformant with the Bidi algorithm even though we don't // remove BN and explicit embedding chars from the stream of characters to reorder @@ -1139,22 +1155,6 @@ struct QBidiAlgorithm { } } - BIDI_DEBUG() << "Rule L1:"; - // Rule L1: - bool resetLevel = true; - for (int i = length - 1; i >= 0; --i) { - if (analysis[i].bidiFlags & QScriptAnalysis::BidiResetToParagraphLevel) { - BIDI_DEBUG() << "resetting pos" << i << "to baselevel"; - analysis[i].bidiLevel = baseLevel; - resetLevel = true; - } else if (resetLevel && analysis[i].bidiFlags & QScriptAnalysis::BidiMaybeResetToParagraphLevel) { - BIDI_DEBUG() << "resetting pos" << i << "to baselevel (maybereset flag)"; - analysis[i].bidiLevel = baseLevel; - } else { - resetLevel = false; - } - } - if (BidiDebugEnabled) { BIDI_DEBUG() << "final resolved levels:"; for (int i = 0; i < length; ++i) @@ -2087,8 +2087,6 @@ void QTextEngine::itemize() const analysis->flags = QScriptAnalysis::Object; break; case QChar::LineSeparator: - if (analysis->bidiLevel % 2) - --analysis->bidiLevel; analysis->flags = QScriptAnalysis::LineOrParagraphSeparator; if (option.flags() & QTextOption::ShowLineAndParagraphSeparators) { const int offset = uc - string; diff --git a/tests/auto/other/qcomplextext/bidireorderstring.h b/tests/auto/other/qcomplextext/bidireorderstring.h index a7401d2c18..b537bf45e4 100644 --- a/tests/auto/other/qcomplextext/bidireorderstring.h +++ b/tests/auto/other/qcomplextext/bidireorderstring.h @@ -78,7 +78,7 @@ const LV logical_visual[] = { { "data42", "foo\nfoo", "foo\nfoo", QChar::DirL }, { "data43", "\327\251\327\234\327\225\327\235\n\327\251\327\234\327\225\327\235", "\327\235\327\225\327\234\327\251\n\327\235\327\225\327\234\327\251", QChar::DirR }, { "data44", "foo\n\327\251\327\234\327\225\327\235", "foo\n\327\235\327\225\327\234\327\251", QChar::DirL }, - { "data45", "\327\251\327\234\327\225\327\235\nfoo", "\327\235\327\225\327\234\327\251\nfoo", QChar::DirR }, + { "data45", "\327\251\327\234\327\225\327\235\nfoo", "foo\n\327\235\327\225\327\234\327\251", QChar::DirR }, { "data46", "\330\250 1.23 \330\250", "\330\250 1.23 \330\250", QChar::DirR }, { "data47", "\331\204\330\250 1.23 \331\202\330\250", "\330\250\331\202 1.23 \330\250\331\204", QChar::DirR }, { "data48", "\330\250 1.2 \330\250", "\330\250 1.2 \330\250", QChar::DirR }, diff --git a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp index c328776089..9ca61a69b4 100644 --- a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp +++ b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp @@ -66,6 +66,11 @@ void tst_QComplexText::bidiReorderString_data() << QString::fromUtf8( data->logical ) << QString::fromUtf8( data->visual ) << (int) data->basicDir; + + QTest::newRow( QByteArray(data->name) + " (doubled)" ) + << (QString::fromUtf8( data->logical ) + QChar(QChar::ParagraphSeparator) + QString::fromUtf8( data->logical )) + << (QString::fromUtf8( data->visual ) + QChar(QChar::ParagraphSeparator) + QString::fromUtf8( data->visual )) + << (int) data->basicDir; data++; } @@ -432,7 +437,7 @@ ushort unicodeForDirection(const QByteArray &direction) { "ET", 0x24 }, { "AN", 0x660 }, { "CS", 0x2c }, - { "B", QChar::ParagraphSeparator }, + { "B", '\n' }, { "S", 0x9 }, { "WS", 0x20 }, { "ON", 0x2a }, -- cgit v1.2.3