diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-10-15 15:11:41 +0200 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-10-18 13:05:42 +0200 |
commit | 5335cc4a5a73bbd6d717989b4257660b92000fb6 (patch) | |
tree | 52b65fdf32b46832a5126dd589ace6c647fc1f20 | |
parent | f54044d4a92ab5ef04c11bc3ca9f064e91d97e63 (diff) |
Fix cursor positioning on bidi boundaries
When the cursor is positioned between to script items that have different
writing directions, prioritise the script item that has the same direction
as the paragraph (i.e. the QTextEngine) when deciding where and how to
display the cursor. If visual cursor movement is enabled, the behavior is
unchanged.
As a drive-by, clean up coding style and avoid shadowing of function-
local variables.
Task-number: QTBUG-88529
Pick-to: 6.2
Change-Id: I15227b10b1469d9caf1235b00e4d6f9f64a8b510
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
-rw-r--r-- | src/gui/text/qtextlayout.cpp | 117 | ||||
-rw-r--r-- | tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp | 58 |
2 files changed, 132 insertions, 43 deletions
diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 845642bb6e..e40f0c5fa0 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -1278,28 +1278,44 @@ void QTextLayout::drawCursor(QPainter *p, const QPointF &pos, int cursorPosition qreal x = position.x() + l.cursorToX(cursorPosition); - int itm; - - if (d->visualCursorMovement()) { - if (cursorPosition == sl.from + sl.length) - cursorPosition--; - itm = d->findItem(cursorPosition); - } else - itm = d->findItem(cursorPosition - 1); - QFixed base = sl.base(); QFixed descent = sl.descent; QFixed cursorDescent = descent; bool rightToLeft = d->isRightToLeft(); + + const int realCursorPosition = cursorPosition; + if (d->visualCursorMovement()) { + if (cursorPosition == sl.from + sl.length) + --cursorPosition; + } else { + --cursorPosition; + } + int itm = d->findItem(cursorPosition); + if (itm >= 0) { - const QScriptItem &si = d->layoutData->items.at(itm); - if (si.ascent >= 0) - base = si.ascent; - if (si.descent == 0) - descent = si.descent; - else if (si.descent > 0 && si.descent < descent) - cursorDescent = si.descent; - rightToLeft = si.analysis.bidiLevel % 2; + const QScriptItem *si = &d->layoutData->items.at(itm); + // Same logic as in cursorToX to handle edges between writing directions to prioritise the script item + // that matches the writing direction of the paragraph. + if (d->layoutData->hasBidi && !d->visualCursorMovement() && si->analysis.bidiLevel % 2 != rightToLeft) { + int neighborItem = itm; + if (neighborItem > 0 && si->position == realCursorPosition) + --neighborItem; + else if (neighborItem < d->layoutData->items.count() - 1 && si->position + si->num_glyphs == realCursorPosition) + ++neighborItem; + const bool onBoundary = neighborItem != itm + && si->analysis.bidiLevel != d->layoutData->items[neighborItem].analysis.bidiLevel; + if (onBoundary && rightToLeft != si->analysis.bidiLevel % 2) { + itm = neighborItem; + si = &d->layoutData->items[itm]; + } + } + if (si->ascent >= 0) + base = si->ascent; + if (si->descent == 0) + descent = si->descent; + else if (si->descent > 0 && si->descent < descent) + cursorDescent = si->descent; + rightToLeft = si->analysis.bidiLevel % 2; } qreal y = position.y() + (sl.y + sl.base() + sl.descent - base - descent).toReal(); bool toggleAntialiasing = !(p->renderHints() & QPainter::Antialiasing) @@ -2743,7 +2759,6 @@ qreal QTextLine::cursorToX(int *cursorPos, Edge edge) const int lineEnd = line.from + line.length + line.trailingSpaces; int pos = qBound(line.from, *cursorPos, lineEnd); - int itm; const QCharAttributes *attributes = eng->attributes(); if (!attributes) { *cursorPos = line.from; @@ -2751,38 +2766,54 @@ qreal QTextLine::cursorToX(int *cursorPos, Edge edge) const } while (pos < lineEnd && !attributes[pos].graphemeBoundary) pos++; - if (pos == lineEnd) { - // end of line ensure we have the last item on the line - itm = eng->findItem(pos-1); - } - else - itm = eng->findItem(pos); + // end of line ensure we have the last item on the line + int itm = pos == lineEnd ? eng->findItem(pos-1) : eng->findItem(pos); if (itm < 0) { *cursorPos = line.from; return x.toReal(); } eng->shapeLine(line); - const QScriptItem *si = &eng->layoutData->items[itm]; - if (!si->num_glyphs) + const QScriptItem *scriptItem = &eng->layoutData->items[itm]; + if (!scriptItem->num_glyphs) eng->shape(itm); + if ((scriptItem->analysis.bidiLevel % 2 != eng->isRightToLeft()) && !eng->visualCursorMovement()) { + // If the item we found has a different writing direction than the engine, + // check if the cursor is between two items with different writing direction + int neighborItem = itm; + if (neighborItem > 0 && scriptItem->position == pos) + --neighborItem; + else if (neighborItem < eng->layoutData->items.count() - 1 && scriptItem->position + scriptItem->num_glyphs == pos) + ++neighborItem; + const bool onBoundary = neighborItem != itm && scriptItem->analysis.bidiLevel != eng->layoutData->items[neighborItem].analysis.bidiLevel; + // If we are, prioritise the neighbor item that has the same direction as the engine + if (onBoundary) { + if (eng->isRightToLeft() != scriptItem->analysis.bidiLevel % 2) { + itm = neighborItem; + scriptItem = &eng->layoutData->items[itm]; + if (!scriptItem->num_glyphs) + eng->shape(itm); + } + } + } + const int l = eng->length(itm); - pos = qBound(0, pos - si->position, l); + pos = qBound(0, pos - scriptItem->position, l); - QGlyphLayout glyphs = eng->shapedGlyphs(si); - unsigned short *logClusters = eng->logClusters(si); + QGlyphLayout glyphs = eng->shapedGlyphs(scriptItem); + unsigned short *logClusters = eng->logClusters(scriptItem); Q_ASSERT(logClusters); - int glyph_pos = pos == l ? si->num_glyphs : logClusters[pos]; - if (edge == Trailing && glyph_pos < si->num_glyphs) { + int glyph_pos = pos == l ? scriptItem->num_glyphs : logClusters[pos]; + if (edge == Trailing && glyph_pos < scriptItem->num_glyphs) { // trailing edge is leading edge of next cluster glyph_pos++; - while (glyph_pos < si->num_glyphs && !glyphs.attributes[glyph_pos].clusterStart) + while (glyph_pos < scriptItem->num_glyphs && !glyphs.attributes[glyph_pos].clusterStart) glyph_pos++; } - bool reverse = si->analysis.bidiLevel % 2; + bool reverse = scriptItem->analysis.bidiLevel % 2; // add the items left of the cursor @@ -2827,32 +2858,32 @@ qreal QTextLine::cursorToX(int *cursorPos, Edge edge) const } } - logClusters = eng->logClusters(si); - glyphs = eng->shapedGlyphs(si); - if (si->analysis.flags >= QScriptAnalysis::TabOrObject) { + logClusters = eng->logClusters(scriptItem); + glyphs = eng->shapedGlyphs(scriptItem); + if (scriptItem->analysis.flags >= QScriptAnalysis::TabOrObject) { if (pos == (reverse ? 0 : l)) - x += si->width; + x += scriptItem->width; } else { bool rtl = eng->isRightToLeft(); bool visual = eng->visualCursorMovement(); - int end = qMin(lineEnd, si->position + l) - si->position; + int end = qMin(lineEnd, scriptItem->position + l) - scriptItem->position; if (reverse) { - int glyph_end = end == l ? si->num_glyphs : logClusters[end]; + int glyph_end = end == l ? scriptItem->num_glyphs : logClusters[end]; int glyph_start = glyph_pos; if (visual && !rtl && !(lastLine && itm == (visualOrder[nItems - 1] + firstItem))) glyph_start++; for (int i = glyph_end - 1; i >= glyph_start; i--) x += glyphs.effectiveAdvance(i); - x -= eng->offsetInLigature(si, pos, end, glyph_pos); + x -= eng->offsetInLigature(scriptItem, pos, end, glyph_pos); } else { - int start = qMax(line.from - si->position, 0); + int start = qMax(line.from - scriptItem->position, 0); int glyph_start = logClusters[start]; int glyph_end = glyph_pos; if (!visual || !rtl || (lastLine && itm == visualOrder[0] + firstItem)) glyph_end--; for (int i = glyph_start; i <= glyph_end; i++) x += glyphs.effectiveAdvance(i); - x += eng->offsetInLigature(si, pos, end, glyph_pos); + x += eng->offsetInLigature(scriptItem, pos, end, glyph_pos); } } @@ -2861,7 +2892,7 @@ qreal QTextLine::cursorToX(int *cursorPos, Edge edge) const if (eng->option.wrapMode() != QTextOption::NoWrap && x < 0) x = 0; - *cursorPos = pos + si->position; + *cursorPos = pos + scriptItem->position; return x.toReal(); } diff --git a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp index a4c848c1ec..32a0374fcd 100644 --- a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp +++ b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp @@ -74,6 +74,8 @@ private slots: void cursorToXForTrailingSpaces_data(); void cursorToXForTrailingSpaces(); void cursorToXInvalidInput(); + void cursorToXForBidiBoundaries_data(); + void cursorToXForBidiBoundaries(); void horizontalAlignment_data(); void horizontalAlignment(); @@ -738,6 +740,58 @@ void tst_QTextLayout::cursorToXInvalidInput() QCOMPARE(cursorPos, 3); } +void tst_QTextLayout::cursorToXForBidiBoundaries_data() +{ + QTest::addColumn<Qt::LayoutDirection>("textDirection"); + QTest::addColumn<QString>("text"); + QTest::addColumn<int>("cursorPosition"); + QTest::addColumn<int>("expectedX"); + + QTest::addRow("LTR, abcشزذabc, 0") << Qt::LeftToRight << "abcشزذabc" + << 0 << 0; + QTest::addRow("RTL, abcشزذabc, 9") << Qt::RightToLeft << "abcشزذabc" + << 9 << TESTFONT_SIZE * 3; + QTest::addRow("LTR, abcشزذabc, 3") << Qt::LeftToRight << "abcشزذabc" + << 0 << 0; + QTest::addRow("RTL, abcشزذabc, 6") << Qt::RightToLeft << "abcشزذabc" + << 9 << TESTFONT_SIZE * 3; + + QTest::addRow("LTR, شزذabcشزذ, 0") << Qt::LeftToRight << "شزذabcشزذ" + << 0 << TESTFONT_SIZE * 2; + QTest::addRow("RTL, شزذabcشزذ, 9") << Qt::RightToLeft << "شزذabcشزذ" + << 9 << 0; + QTest::addRow("LTR, شزذabcشزذ, 3") << Qt::LeftToRight << "شزذabcشزذ" + << 3 << TESTFONT_SIZE * 2; + QTest::addRow("RTL, شزذabcشزذ, 3") << Qt::RightToLeft << "شزذabcشزذ" + << 3 << TESTFONT_SIZE * 5; + QTest::addRow("LTR, شزذabcشزذ, 6") << Qt::LeftToRight << "شزذabcشزذ" + << 6 << TESTFONT_SIZE * 5; + QTest::addRow("RTL, شزذabcشزذ, 6") << Qt::RightToLeft << "شزذabcشزذ" + << 6 << TESTFONT_SIZE * 2; +} + +void tst_QTextLayout::cursorToXForBidiBoundaries() +{ + QFETCH(Qt::LayoutDirection, textDirection); + QFETCH(QString, text); + QFETCH(int, cursorPosition); + QFETCH(int, expectedX); + + QTextOption option; + option.setTextDirection(textDirection); + + QTextLayout layout(text, testFont); + layout.setTextOption(option); + layout.beginLayout(); + + QTextLine line = layout.createLine(); + line.setLineWidth(0x10000); + + QCOMPARE(line.cursorToX(cursorPosition), expectedX); + + layout.endLayout(); +} + void tst_QTextLayout::horizontalAlignment_data() { qreal width = TESTFONT_SIZE * 4; @@ -1146,6 +1200,10 @@ void tst_QTextLayout::xToCursorForBidiEnds_data() << 0 << 6; QTest::addRow("RTL, شزذabc") << Qt::RightToLeft << "شزذabc" << 6 << 0; + QTest::addRow("LTR, شزذ123") << Qt::LeftToRight << "شزذ123" + << 0 << 6; + QTest::addRow("RTL, شزذ123") << Qt::RightToLeft << "شزذ123" + << 6 << 0; QTest::addRow("LTR, abcشزذabc") << Qt::LeftToRight << "abcشزذabc" << 0 << 9; |