From: Jean-Marc Lasgouttes Date: Mon, 27 Jan 2020 17:38:21 +0000 (+0100) Subject: Adopt a 'belt and braces' approach to bidi forcing X-Git-Tag: 2.3.5~49 X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=e47834104a5d68e0e0a5141fa1824ce34ea2ae46;p=features.git Adopt a 'belt and braces' approach to bidi forcing There are two techniques that I know of for forcing the direction of a string, regardlessly of whether its contents is naturally LtR, RtL or undecided. 1/ The unicode LTR/LTR override characters. This is supposed to be the clean way, however, it does not seem to work with Qt 5.14 (see #11691). 2/ The undocumented QTextLayout::setFlags method. This is used internally and allows to pass the (undocumented) flags Qt::TextForceRightToLeft and Qt::TextForceLeftToRight. This was used until we had issues with Qt 5.11 (see #11284). In order to get the best of both worlds, this patch allows to enable those two methods separately, and actually enables both at the same time by default! Fixes bug #11691. (cherry picked from commit 4d6041a7b68de5856b657cfd3b735596b3d7e0e0) --- diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 63f61f2e3f..67fcb2f0e4 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -29,6 +29,25 @@ using namespace std; using namespace lyx::support; +/* Define what mechanism is used to enforce text direction. Different + * methods work with different Qt versions. Here we try to use both + * methods together. + */ +// Define to use unicode override characters to force direction +#define BIDI_USE_OVERRIDE +// Define to use flag to force direction +#define BIDI_USE_FLAG + +#ifdef BIDI_USE_OVERRIDE +# define BIDI_OFFSET 1 +#else +# define BIDI_OFFSET 0 +#endif + +#if !defined(BIDI_USE_OVERRIDE) && !defined(BIDI_USE_FLAG) +# error "Define at least one of BIDI_USE_OVERRIDE or BIDI_USE_FLAG" +#endif + namespace std { /* @@ -249,7 +268,15 @@ GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, QFont copy = font_; copy.setWordSpacing(wordspacing); ptl->setFont(copy); -#if 1 + +#ifdef BIDI_USE_FLAG + /* Use undocumented flag to enforce drawing direction + * FIXME: This does not work with Qt 5.11 (ticket #11284). + */ + ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); +#endif + +#ifdef BIDI_USE_OVERRIDE /* Use unicode override characters to enforce drawing direction * Source: http://www.iamcal.com/understanding-bidirectional-text/ */ @@ -259,14 +286,10 @@ GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, else // Left-to-right override: forces to draw text left-to-right ptl->setText(QChar(0x202D) + toqstr(s)); -#define TEXTLAYOUT_OFFSET 1 #else - // FIXME: This does not work with Qt 5.11 (ticket #11284). - // Note that both setFlags and the enums are undocumented - ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); ptl->setText(toqstr(s)); -#define TEXTLAYOUT_OFFSET 0 #endif + ptl->beginLayout(); ptl->createLine(); ptl->endLayout(); @@ -285,9 +308,9 @@ int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl, * not be the same when there are high-plan unicode characters * (bug #10443). */ - // TEXTLAYOUT_OFFSET accounts for a possible direction override + // BIDI_OFFSET accounts for a possible direction override // character in front of the string. - int const qpos = toqstr(s.substr(0, pos)).length() + TEXTLAYOUT_OFFSET; + int const qpos = toqstr(s.substr(0, pos)).length() + BIDI_OFFSET; return static_cast(tl->lineForTextPosition(qpos).cursorToX(qpos)); } @@ -331,7 +354,7 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl, #if QT_VERSION < 0x040801 || QT_VERSION >= 0x050100 int pos = qstring_to_ucs4(tl->text().left(qpos)).length(); // there may be a direction override character in front of the string. - return max(pos - TEXTLAYOUT_OFFSET, 0); + return max(pos - BIDI_OFFSET, 0); #else /* Due to QTBUG-25536 in 4.8.1 <= Qt < 5.1.0, the string returned * by QString::toUcs4 (used by qstring_to_ucs4) may have wrong @@ -342,7 +365,7 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl, * under a profiler. */ // there may be a direction override character in front of the string. - qpos = max(qpos - TEXTLAYOUT_OFFSET, 0); + qpos = max(qpos - BIDI_OFFSET, 0); int pos = min(qpos, static_cast(s.length())); while (pos >= 0 && toqstr(s.substr(0, pos)).length() != qpos) --pos; @@ -388,7 +411,14 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x, // Unicode character ZERO WIDTH NO-BREAK SPACE QChar const zerow_nbsp(0xfeff); QString qs = zerow_nbsp + toqstr(s) + zerow_nbsp; -#if 1 +#ifdef BIDI_USE_FLAG + /* Use undocumented flag to enforce drawing direction + * FIXME: This does not work with Qt 5.11 (ticket #11284). + */ + tl.setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); +#endif + +#ifdef BIDI_USE_OVERRIDE /* Use unicode override characters to enforce drawing direction * Source: http://www.iamcal.com/understanding-bidirectional-text/ */ @@ -398,13 +428,8 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x, else // Left-to-right override: forces to draw text left-to-right qs = QChar(0x202D) + qs; - int const offset = 2; -#else - // Alternative version that breaks with Qt5 and arabic text (#10436) - // Note that both setFlags and the enums are undocumented - tl.setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); - int const offset = 1; #endif + int const offset = 1 + BIDI_OFFSET; tl.setText(qs); tl.setFont(font_); diff --git a/status.23x b/status.23x index 5a8ec3c575..098a40013e 100644 --- a/status.23x +++ b/status.23x @@ -113,6 +113,8 @@ What's new - Improve reporting of undefined control sequences in preamble (bug 11844). +- Fix direction of some parts of text in bidi texts (bug 11691). + * INTERNALS