[LyX features/breakspace] (almost) Fix Qt4 support
Jean-Marc Lasgouttes
lasgouttes at lyx.org
Thu Nov 10 15:38:35 UTC 2022
The branch, breakspace, has been updated.
discards 1bb2f6261cf2440ab4174856fe23fcf6b343b484 (commit)
discards 1155fd80c21b7c7221857ecc3a900dd4f00f9053 (commit)
discards f3f6f9b0fcd735f2610f971de2de672580034b26 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (1bb2f6261cf2440ab4174856fe23fcf6b343b484)
\
N -- N -- N (50e1bdf320d9481ddec02f9c82cbfb3c369d8e53)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
- Log -----------------------------------------------------------------
commit 50e1bdf320d9481ddec02f9c82cbfb3c369d8e53
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date: Thu Nov 10 17:24:56 2022 +0100
(almost) Fix Qt4 support
When is it detected that our zero-width characters are not really
zero-width, avoid to use them altogether. Currently, this is known to
happen in Qt4 with monospace fonts.
This allow to remove the special Qt4 code in breaskString_helper.
Related to bug #10117.
diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index d2ebf0e..cc3a329 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -146,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!");
}
@@ -349,6 +354,7 @@ struct TextLayoutHelper
/// \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);
/// translate QString index to docstring index
@@ -481,7 +487,7 @@ GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl,
if (auto ptl = qtextlayout_cache_[key])
return ptl;
PROFILE_CACHE_MISS(getTextLayout);
- TextLayoutHelper tlh(s, rtl);
+ TextLayoutHelper tlh(s, rtl, needs_naked_);
auto const ptl = getTextLayout_helper(tlh, wordspacing, font_);
qtextlayout_cache_.insert(key, ptl);
return ptl;
@@ -491,7 +497,7 @@ GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl,
int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl,
double const wordspacing) const
{
- TextLayoutHelper tlh(s, rtl);
+ 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));
@@ -502,7 +508,7 @@ int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl,
int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
double const wordspacing) const
{
- TextLayoutHelper tlh(s, rtl);
+ TextLayoutHelper tlh(s, rtl, needs_naked_);
auto ptl = getTextLayout(tlh, wordspacing);
QTextLine const & tline = ptl->lineForTextPosition(0);
int qpos = tline.xToCursor(x);
@@ -540,7 +546,7 @@ FontMetrics::Breaks
GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
bool rtl, bool force) const
{
- TextLayoutHelper const tlh(s, rtl);
+ TextLayoutHelper const tlh(s, rtl, needs_naked_);
QTextLayout tl;
#ifdef BIDI_USE_FLAG
@@ -582,7 +588,6 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
QTextLine const & line = tl.lineAt(i);
int const line_epos = line.textStart() + line.textLength();
int const epos = tlh.qpos2pos(line_epos);
-#if QT_VERSION >= 0x050000
// 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.
@@ -591,24 +596,8 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
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)));
-#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 ss = s.substr(pos, epos - pos);
- int const wid = width(ss);
- if (!ss.empty() && isSpace(ss.back()))
- ss.pop_back();
- int const nspc_wid = i + 1 < tl.lineCount() ? width(ss) : wid;
-#endif
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 5c32ea9..0187c5c 100644
--- a/src/frontends/qt/GuiFontMetrics.h
+++ b/src/frontends/qt/GuiFontMetrics.h
@@ -119,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<char_type, int> width_cache_;
/// Cache of string widths
commit 23cd38bc1c1fb30568117f337cb6a54966299bf0
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date: Thu Jul 14 18:22:46 2022 +0200
Use TextLayoutHelper in x2pos and pos2x
This leads to substantial code simplification.
No intended behavior change.
Related to ticket #10117.
diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index 5ee01ec..d2ebf0e 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -39,26 +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
-/* Unicode override characters enforce drawing direction
- * Source: http://www.iamcal.com/understanding-bidirectional-text/
- * Right-to-left override is 0x202e and left-to-right override is 0x202d.
- */
-QChar const bidi_override[2] = {0x202d, 0x202e};
-#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
@@ -404,8 +393,12 @@ TextLayoutHelper::TextLayoutHelper(docstring const & s, bool isrtl, bool naked)
qstr += word_joiner;
#ifdef BIDI_USE_OVERRIDE
+ /* 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 (!naked)
- qstr += bidi_override[rtl];
+ qstr += QChar(rtl ? 0x202e : 0x202d);
#endif
// Now translate the string character-by-character.
@@ -498,25 +491,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<QTextLayout const> 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<int>(tl->lineForTextPosition(qpos).cursorToX(qpos));
+ TextLayoutHelper tlh(s, rtl);
+ auto ptl = getTextLayout(tlh, wordspacing);
+ // pos can be negative, see #10506.
+ int const qpos = tlh.pos2qpos(max(pos, 0));
+ return static_cast<int>(ptl->lineForTextPosition(qpos).cursorToX(qpos));
}
int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
double const wordspacing) const
{
- shared_ptr<QTextLayout const> tl = getTextLayout(s, rtl, wordspacing);
- QTextLine const & tline = tl->lineForTextPosition(0);
+ TextLayoutHelper tlh(s, rtl);
+ auto ptl = getTextLayout(tlh, wordspacing);
+ QTextLine const & tline = ptl->lineForTextPosition(0);
int qpos = tline.xToCursor(x);
int newx = static_cast<int>(tline.cursorToX(qpos));
// The value of qpos may be wrong in rtl text (see ticket #10569).
@@ -544,31 +532,7 @@ 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<int>(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);
}
commit 7fdc6046069295b0993a99e34cc3d29fa1224fd7
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date: Thu Jul 14 17:24:33 2022 +0200
Refactor GuiFontMetrics::getLayout
This allows to create a new version that takes an already built
TextLayoutHelper struct as parameter.
No intended change.
See discussion in bug #10117.
diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index 3ee2702..5ee01ec 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -352,8 +352,6 @@ uint qHash(TextLayoutKey const & key)
}
-namespace {
-
// This holds a translation table between the original string and the
// QString that we can use with QTextLayout.
struct TextLayoutHelper
@@ -438,39 +436,60 @@ TextLayoutHelper::TextLayoutHelper(docstring const & s, bool isrtl, bool naked)
//LYXERR0("TLH: " << dump.replace(word_joiner, "|").toStdString());
}
-}
-shared_ptr<QTextLayout const>
-GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl,
- double const wordspacing) const
+namespace {
+
+shared_ptr<QTextLayout>
+getTextLayout_helper(TextLayoutHelper const & tlh, double const wordspacing,
+ QFont font)
{
- 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<QTextLayout>();
ptl->setCacheEnabled(true);
- QFont copy = font_;
- copy.setWordSpacing(wordspacing);
- ptl->setFont(copy);
-
+ 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(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
-#endif
-
-#ifdef BIDI_USE_OVERRIDE
- ptl->setText(bidi_override[rtl] + toqstr(s));
-#else
- ptl->setText(toqstr(s));
+ ptl->setFlags(tlh.rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
#endif
+ ptl->setText(tlh.qstr);
ptl->beginLayout();
ptl->createLine();
ptl->endLayout();
+
+ return ptl;
+}
+
+}
+
+shared_ptr<QTextLayout const>
+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<QTextLayout const>
+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);
+ auto const ptl = getTextLayout_helper(tlh, wordspacing, font_);
qtextlayout_cache_.insert(key, ptl);
return ptl;
}
diff --git a/src/frontends/qt/GuiFontMetrics.h b/src/frontends/qt/GuiFontMetrics.h
index 82c5839..5c32ea9 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<QTextLayout const>
- 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<QTextLayout const>
+ getTextLayout(TextLayoutHelper const & tlh, double const wordspacing) const;
+
/// The font
QFont font_;
commit 201c95a76ea69fb5c6652b657b175db12bcd5e80
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date: Mon Jul 11 23:56:35 2022 +0200
Handle multiple spaces at row break
In order to work around the Qt row breaking algorithm, which considers
multiple spaces as one at QTextLine break, we insert word_joiner unicode
characters beteween each pair of spaces.
The TextLayoutHelper class makes it easy to handle that.
Update Row::Element::rtrim() to only remove one space at row end.
Update support::countExpanders() to count all spaces, without special
handling for consecutive ones.
Fixes bug #10117.
diff --git a/src/Row.cpp b/src/Row.cpp
index 1c17fb2..6edde19 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 <algorithm>
#include <ostream>
@@ -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 7fe067b..3ee2702 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"
@@ -411,7 +411,13 @@ TextLayoutHelper::TextLayoutHelper(docstring const & s, bool isrtl, bool naked)
#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
@@ -599,27 +605,19 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
// 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));
- }
- }
+ 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)));
#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);
+ docstring ss = s.substr(pos, epos - pos);
int const wid = width(ss);
- int const nspc_wid = i + 1 < tl.lineCount() ? width(rtrim(ss)) : wid;
+ if (!ss.empty() && isSpace(ss.back()))
+ ss.pop_back();
+ int const nspc_wid = i + 1 < tl.lineCount() ? width(ss) : wid;
#endif
breaks.emplace_back(epos - pos, wid, nspc_wid);
pos = epos;
diff --git a/src/support/lstrings.cpp b/src/support/lstrings.cpp
index 77c0115..d5b6dea 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;
}
-----------------------------------------------------------------------
Summary of changes:
src/frontends/qt/GuiFontMetrics.cpp | 31 ++++++++++---------------------
src/frontends/qt/GuiFontMetrics.h | 4 ++++
src/support/lstrings.cpp | 13 +++----------
3 files changed, 17 insertions(+), 31 deletions(-)
hooks/post-receive
--
Repository for new features
More information about the lyx-cvs
mailing list