From: Jean-Marc Lasgouttes Date: Wed, 16 Nov 2022 09:22:11 +0000 (+0100) Subject: Merge branch 'breakspace' X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=12bff77722205bdec4e78236bd6d8c75e818e717;hp=5bf7f938a4a951b49bff9828bd6d17ee8da7e317;p=features.git Merge branch 'breakspace' This branch improves handling of spaces on display (see #10117): * caret is correctly shown in the middle of double spaces in justified rows; * sequence spaces are correctly shown at the end of rows before automatic row breaks. Moreover, this branch: * streamlines the code that handles spaces in row breaking * improves display in Qt4 : although the improvements outlined above are not present in Qt4 for monospaced fonts, some dsplay glitches are resolved. * improves performance for very long paragraphs (#12598). --- diff --git a/src/Row.cpp b/src/Row.cpp index 1c17fb2992..6edde19e75 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -27,6 +27,7 @@ #include "support/lassert.h" #include "support/lstrings.h" #include "support/lyxlib.h" +#include "support/textutils.h" #include #include @@ -203,14 +204,13 @@ bool Row::Element::splitAt(int const width, int next_width, bool force, void Row::Element::rtrim() { - if (type != STRING) + if (type != STRING || str.empty() || !isSpace(str.back())) return; /* This is intended for strings that have been created by splitAt. - * They may have trailing spaces, but they are not counted in the - * string length (QTextLayout feature, actually). We remove them, - * and decrease endpos, since spaces at row break are invisible. + * If There is a trailing space, we remove it and decrease endpos, + * since spaces at row break are invisible. */ - str = support::rtrim(str); + str.pop_back(); endpos = pos + str.length(); dim.wid = nspc_wid; } diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp index c227c28d25..cc3a32929b 100644 --- a/src/frontends/qt/GuiFontMetrics.cpp +++ b/src/frontends/qt/GuiFontMetrics.cpp @@ -20,8 +20,8 @@ #include "support/convert.h" #include "support/debug.h" #include "support/lassert.h" -#include "support/lstrings.h" // for breakString_helper with qt4 #include "support/lyxlib.h" +#include "support/textutils.h" #define DISABLE_PMPROF #include "support/pmprof.h" @@ -39,21 +39,15 @@ 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 what mechanisms are used to enforce text direction. There + * are two methods that 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 to use QTextLayout 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 @@ -152,6 +146,11 @@ GuiFontMetrics::GuiFontMetrics(QFont const & font) slope_ = defaultSlope; LYXERR(Debug::FONT, "Italic slope: " << slope_); } + // If those characters have a non-zero width, we need to avoid them. + // This happens with Qt4 with monospace fonts + needs_naked_ = width(QString() + QChar(0x2060) + QChar(0x202d) + QChar(0x202e)) > 0; + // if (needs_naked_) + // LYXERR0("Font " << font.family() << " needs naked text layouts!"); } @@ -346,45 +345,150 @@ uint qHash(TextLayoutKey const & key) return std::qHash(key.s) ^ ::qHash(params); } -shared_ptr -GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, - double const wordspacing) const + +// This holds a translation table between the original string and the +// QString that we can use with QTextLayout. +struct TextLayoutHelper { - PROFILE_THIS_BLOCK(getTextLayout); - TextLayoutKey key{s, rtl, wordspacing}; - if (auto ptl = qtextlayout_cache_[key]) - return ptl; - PROFILE_CACHE_MISS(getTextLayout); - auto const ptl = make_shared(); - ptl->setCacheEnabled(true); - QFont copy = font_; - copy.setWordSpacing(wordspacing); - ptl->setFont(copy); + /// Create the helper + /// \c s is the original string + /// \c isrtl is true if the string is right-to-left + /// \c naked is true to disable the insertion of zero width annotations + /// FIXME: remove \c naked argument when Qt4 support goes away. + TextLayoutHelper(docstring const & s, bool isrtl, bool naked = false); -#ifdef BIDI_USE_FLAG - /* Use undocumented flag to enforce drawing direction - * FIXME: This does not work with Qt 5.11 (ticket #11284). + /// translate QString index to docstring index + docstring::size_type qpos2pos(int qpos) const + { + return lower_bound(pos2qpos_.begin(), pos2qpos_.end(), qpos) - pos2qpos_.begin(); + } + + /// Translate docstring index to QString index + int pos2qpos(docstring::size_type pos) const { return pos2qpos_[pos]; } + + // The original string + docstring docstr; + // The mirror string + QString qstr; + // is string right-to-left? + bool rtl; + +private: + // This vector contains the QString pos for each string position + vector pos2qpos_; +}; + + +TextLayoutHelper::TextLayoutHelper(docstring const & s, bool isrtl, bool naked) + : docstr(s), rtl(isrtl) +{ + // Reserve memory for performance purpose + pos2qpos_.reserve(s.size()); + qstr.reserve(2 * s.size()); + + /* Qt will not break at a leading or trailing space, and we need + * that sometimes, see http://www.lyx.org/trac/ticket/9921. + * + * To work around the problem, we enclose the string between + * word joiner characters so that the QTextLayout algorithm will + * agree to break the text at these extremal spaces. */ - ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); -#endif + // Unicode character WORD JOINER + QChar const word_joiner(0x2060); + if (!naked) + qstr += word_joiner; #ifdef BIDI_USE_OVERRIDE - /* Use unicode override characters to enforce drawing direction + /* Unicode override characters enforce drawing direction * Source: http://www.iamcal.com/understanding-bidirectional-text/ + * Left-to-right override is 0x202d and right-to-left override is 0x202e. */ - if (rtl) - // Right-to-left override: forces to draw text right-to-left - ptl->setText(QChar(0x202E) + toqstr(s)); - else - // Left-to-right override: forces to draw text left-to-right - ptl->setText(QChar(0x202D) + toqstr(s)); -#else - ptl->setText(toqstr(s)); + if (!naked) + qstr += QChar(rtl ? 0x202e : 0x202d); #endif + // Now translate the string character-by-character. + bool was_space = false; + for (char_type const c : s) { + // insert a word joiner character between consecutive spaces + bool const is_space = isSpace(c); + if (!naked && is_space && was_space) + qstr += word_joiner; + was_space = is_space; + // Remember the QString index at this point + pos2qpos_.push_back(qstr.size()); + // Performance: UTF-16 characters are easier + if (is_utf16(c)) + qstr += ucs4_to_qchar(c); + else + qstr += toqstr(c); + } + + // Final word joiner (see above) + if (!naked) + qstr += word_joiner; + + // Add virtual position at the end of the string + pos2qpos_.push_back(qstr.size()); + + //QString dump = qstr; + //LYXERR0("TLH: " << dump.replace(word_joiner, "|").toStdString()); +} + + +namespace { + +shared_ptr +getTextLayout_helper(TextLayoutHelper const & tlh, double const wordspacing, + QFont font) +{ + auto const ptl = make_shared(); + ptl->setCacheEnabled(true); + font.setWordSpacing(wordspacing); + ptl->setFont(font); +#ifdef BIDI_USE_FLAG + /* Use undocumented flag to enforce drawing direction + * FIXME: This does not work with Qt 5.11 (ticket #11284). + */ + ptl->setFlags(tlh.rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); +#endif + ptl->setText(tlh.qstr); + ptl->beginLayout(); ptl->createLine(); ptl->endLayout(); + + return ptl; +} + +} + +shared_ptr +GuiFontMetrics::getTextLayout(TextLayoutHelper const & tlh, + double const wordspacing) const +{ + PROFILE_THIS_BLOCK(getTextLayout_TLH); + TextLayoutKey key{tlh.docstr, tlh.rtl, wordspacing}; + if (auto ptl = qtextlayout_cache_[key]) + return ptl; + PROFILE_CACHE_MISS(getTextLayout_TLH); + auto const ptl = getTextLayout_helper(tlh, wordspacing, font_); + qtextlayout_cache_.insert(key, ptl); + return ptl; +} + + +shared_ptr +GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, + double const wordspacing) const +{ + PROFILE_THIS_BLOCK(getTextLayout); + TextLayoutKey key{s, rtl, wordspacing}; + if (auto ptl = qtextlayout_cache_[key]) + return ptl; + PROFILE_CACHE_MISS(getTextLayout); + TextLayoutHelper tlh(s, rtl, needs_naked_); + auto const ptl = getTextLayout_helper(tlh, wordspacing, font_); qtextlayout_cache_.insert(key, ptl); return ptl; } @@ -393,25 +497,20 @@ GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl, double const wordspacing) const { - if (pos <= 0) - pos = 0; - shared_ptr tl = getTextLayout(s, rtl, wordspacing); - /* Since QString is UTF-16 and docstring is UCS-4, the offsets may - * not be the same when there are high-plan unicode characters - * (bug #10443). - */ - // BIDI_OFFSET accounts for a possible direction override - // character in front of the string. - int const qpos = toqstr(s.substr(0, pos)).length() + BIDI_OFFSET; - return static_cast(tl->lineForTextPosition(qpos).cursorToX(qpos)); + TextLayoutHelper tlh(s, rtl, needs_naked_); + auto ptl = getTextLayout(tlh, wordspacing); + // pos can be negative, see #10506. + int const qpos = tlh.pos2qpos(max(pos, 0)); + return static_cast(ptl->lineForTextPosition(qpos).cursorToX(qpos)); } int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl, double const wordspacing) const { - shared_ptr tl = getTextLayout(s, rtl, wordspacing); - QTextLine const & tline = tl->lineForTextPosition(0); + TextLayoutHelper tlh(s, rtl, needs_naked_); + auto ptl = getTextLayout(tlh, wordspacing); + QTextLine const & tline = ptl->lineForTextPosition(0); int qpos = tline.xToCursor(x); int newx = static_cast(tline.cursorToX(qpos)); // The value of qpos may be wrong in rtl text (see ticket #10569). @@ -439,112 +538,24 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl, // correct x value to the actual cursor position. x = newx; - /* Since QString is UTF-16 and docstring is UCS-4, the offsets may - * not be the same when there are high-plan unicode characters - * (bug #10443). - */ -#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 - 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 - * length. We work around the problem by trying all docstring - * positions until the right one is found. This is slow only if - * there are many high-plane Unicode characters. It might be - * worthwhile to implement a dichotomy search if this shows up - * under a profiler. - */ - // there may be a direction override character in front of the string. - 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; - LASSERT(pos > 0 || qpos == 0, /**/); - return pos; -#endif + return tlh.qpos2pos(qpos); } -namespace { - -const int brkStrOffset = 1 + BIDI_OFFSET; - - -QString createBreakableString(docstring const & s, bool rtl, QTextLayout & tl) +FontMetrics::Breaks +GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid, + bool rtl, bool force) const { - /* Qt will not break at a leading or trailing space, and we need - * that sometimes, see http://www.lyx.org/trac/ticket/9921. - * - * To work around the problem, we enclose the string between - * zero-width characters so that the QTextLayout algorithm will - * agree to break the text at these extremal spaces. - */ - // Unicode character ZERO WIDTH NO-BREAK SPACE - QChar const zerow_nbsp(0xfeff); - QString qs = zerow_nbsp + toqstr(s) + zerow_nbsp; + TextLayoutHelper const tlh(s, rtl, needs_naked_); + + QTextLayout tl; #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/ - */ - if (rtl) - // Right-to-left override: forces to draw text right-to-left - qs = QChar(0x202E) + qs; - else - // Left-to-right override: forces to draw text left-to-right - qs = QChar(0x202D) + qs; -#endif - return qs; -} - - -docstring::size_type brkstr2str_pos(QString brkstr, docstring const & str, int pos) -{ - /* Since QString is UTF-16 and docstring is UCS-4, the offsets may - * not be the same when there are high-plan unicode characters - * (bug #10443). - */ - // The variable `brkStrOffset' is here to account for the extra leading characters. - // The ending character zerow_nbsp has to be ignored if the line is complete. - int const qlen = max(pos - brkStrOffset - (pos == brkstr.length()), 0); -#if QT_VERSION < 0x040801 || QT_VERSION >= 0x050100 - auto const len = qstring_to_ucs4(brkstr.mid(brkStrOffset, qlen)).length(); - // Avoid warning - (void)str; -#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 - * length. We work around the problem by trying all docstring - * positions until the right one is found. This is slow only if - * there are many high-plane Unicode characters. It might be - * worthwhile to implement a dichotomy search if this shows up - * under a profiler. - */ - int len = min(qlen, static_cast(str.length())); - while (len >= 0 && toqstr(str.substr(0, len)).length() != qlen) - --len; - LASSERT(len > 0 || qlen == 0, /**/); -#endif - return len; -} - -} - -FontMetrics::Breaks -GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid, - bool rtl, bool force) const -{ - QTextLayout tl; - QString qs = createBreakableString(s, rtl, tl); - tl.setText(qs); + tl.setText(tlh.qstr); tl.setFont(font_); QTextOption to; /* @@ -576,42 +587,17 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid, for (int i = 0 ; i < tl.lineCount() ; ++i) { QTextLine const & line = tl.lineAt(i); int const line_epos = line.textStart() + line.textLength(); - int const epos = brkstr2str_pos(qs, s, line_epos); -#if QT_VERSION >= 0x050000 + int const epos = tlh.qpos2pos(line_epos); // This does not take trailing spaces into account, except for the last line. int const wid = iround(line.naturalTextWidth()); // If the line is not the last one, trailing space is always omitted. int nspc_wid = wid; // For the last line, compute the width without trailing space - if (i + 1 == tl.lineCount()) { - // trim_pos points to the last character that is not a space - auto trim_pos = s.find_last_not_of(from_ascii(" ")); - if (trim_pos == docstring::npos) - nspc_wid = 0; - else if (trim_pos + 1 < s.length()) { - int const num_spaces = s.length() - trim_pos - 1; - // find the position on the line before trailing - // spaces. Remove 1 to account for the ending - // non-breaking space of qs. - nspc_wid = iround(line.cursorToX(line_epos - num_spaces - 1)); - } - } -#else - // With some monospace fonts, the value of horizontalAdvance() - // can be wrong with Qt4. One hypothesis is that the invisible - // characters that we use are given a non-null width. - // FIXME: this is slower than it could be but we'll get rid of Qt4 anyway - docstring const ss = s.substr(pos, epos - pos); - int const wid = width(ss); - int const nspc_wid = i + 1 < tl.lineCount() ? width(rtrim(ss)) : wid; -#endif + if (i + 1 == tl.lineCount() && !s.empty() && isSpace(s.back()) + && line.textStart() <= tlh.pos2qpos(s.size() - 1)) + nspc_wid = iround(line.cursorToX(tlh.pos2qpos(s.size() - 1))); breaks.emplace_back(epos - pos, wid, nspc_wid); pos = epos; -#if 0 - // FIXME: should it be kept in some form? - if ((force && line.textLength() == brkStrOffset) || line_wid > x) - return {-1, line_wid}; -#endif } return breaks; diff --git a/src/frontends/qt/GuiFontMetrics.h b/src/frontends/qt/GuiFontMetrics.h index 82c5839e21..0187c5c85f 100644 --- a/src/frontends/qt/GuiFontMetrics.h +++ b/src/frontends/qt/GuiFontMetrics.h @@ -27,6 +27,8 @@ namespace lyx { namespace frontend { +struct TextLayoutHelper; + struct BreakStringKey { bool operator==(BreakStringKey const & key) const { @@ -97,14 +99,17 @@ public: /// Return a pointer to a cached QTextLayout object std::shared_ptr - getTextLayout(docstring const & s, bool const rtl, - double const wordspacing) const; + getTextLayout(docstring const & s, bool const rtl, double const wordspacing) const; private: Breaks breakString_helper(docstring const & s, int first_wid, int wid, bool rtl, bool force) const; + /// A different version of getTextLayout for internal use + std::shared_ptr + getTextLayout(TextLayoutHelper const & tlh, double const wordspacing) const; + /// The font QFont font_; @@ -114,6 +119,10 @@ private: /// Slope of italic font double slope_; + /// If true, avoid extra annotation in string for QTextLayout + // FIXME: remove wen Qt4 suport goes away + bool needs_naked_ = false; + /// Cache of char widths mutable QHash width_cache_; /// Cache of string widths diff --git a/src/support/lstrings.cpp b/src/support/lstrings.cpp index 77c01151f5..d5b6dea588 100644 --- a/src/support/lstrings.cpp +++ b/src/support/lstrings.cpp @@ -1507,18 +1507,11 @@ int countExpanders(docstring const & str) // Numbers of characters that are expanded by inter-word spacing. These // characters are spaces, except for characters 09-0D which are treated // specially. (From a combination of testing with the notepad found in qt's - // examples, and reading the source code.) In addition, consecutive spaces - // only count as one expander. - bool wasspace = false; + // examples, and reading the source code.) int nexp = 0; for (char_type c : str) - if (c > 0x0d && isSpace(c)) { - if (!wasspace) { - ++nexp; - wasspace = true; - } - } else - wasspace = false; + if (c > 0x0d && isSpace(c)) + ++nexp; return nexp; }