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 63f61f2e3f619c2806140e83f068524b83c23a72..67fcb2f0e4ba63bebf3d49e6b4fbcda8b7e4f205 100644 (file)
--- 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<int>(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<int>(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 5a8ec3c575538a620b126d5603f4051b26407c29..098a40013e437596281c864608bb3c45050963c6 100644 (file)
--- 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