commit 2ff639a0e6d051c6411fe6655cefc36a4e902211 Author: Jean-Marc Lasgouttes <lasgout...@lyx.org> Date: Wed May 14 17:46:43 2014 +0200
Fix positionning of cursor The old implementation of Row::Element::pos2x and x2pos did not work correctly with Arabic text, because characters can have shapes that depend on context. This new implementation leverages QTextLayout in a simplified way, since only one word is added to the layout. This allows to make Row::Element::x2pos more readable. Fixes: #9115. diff --git a/src/Row.cpp b/src/Row.cpp index 48b4fff..a0c0dc4 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -36,78 +36,65 @@ using frontend::FontMetrics; double Row::Element::pos2x(pos_type const i) const { - LASSERT(i >= pos && i <= endpos, return 0); + // This can happen with inline completion when clicking on the + // row after the completion. + if (i < pos || i > endpos) + return 0; bool const rtl = font.isVisibleRightToLeft(); int w = 0; //handle first the two bounds of the element - if (i == pos) - w = 0; + if (i == pos || type != STRING) + w = rtl ? width() : 0; else if (i == endpos) - w = width(); + w = rtl ? 0 : width(); else { - LASSERT(type == STRING, return 0); FontMetrics const & fm = theFontMetrics(font); - // FIXME Avoid caching of metrics there? - w = fm.width(str.substr(0, i - pos)); + w = fm.pos2x(str, i - pos, font.isVisibleRightToLeft()); } - if (rtl) - return width() - w; - else - return w; + return w; } -pos_type Row::Element::x2pos(double &x, bool const low) const +pos_type Row::Element::x2pos(double &x) const { //lyxerr << "x2pos: x=" << x << " w=" << width() << " " << *this; - // If element is rtl, flip x value bool const rtl = font.isVisibleRightToLeft(); - double x2 = rtl ? (width() - x) : x; - - double last_w = 0; - double w = 0; size_t i = 0; + switch (type) { - case VIRTUAL: - // those elements are actually empty (but they have a width) - break; case STRING: { FontMetrics const & fm = theFontMetrics(font); - // FIXME: implement dichotomy search? - for ( ; i < str.size() ; ++i) { - last_w = w; - w = fm.width(str.substr(0, i + 1)); - if (w > x2) - break; - } + // FIXME: is it really necessary for x to be a double? + int xx = x; + i = fm.x2pos(str, xx, rtl); + x = xx; break; } + case VIRTUAL: + // those elements are actually empty (but they have a width) + i = 0; + x = rtl ? width() : 0; + break; case SEPARATOR: case INSET: case SPACE: - // those elements contain only one position - w = width(); - } - - if (type == STRING && i == str.size()) - x2 = w; - // round to the closest side. The !rtl is here to obtain the - // same rounding as with the old code (this is cosmetic and - // can be eventually removed). - else if (type != VIRTUAL && !low && (x2 - last_w + !rtl > w - x2)) { - x2 = w; - ++i; - } else - x2 = last_w; - - // is element is rtl, flip values back - x = rtl ? width() - x2 : x2; + // those elements contain only one position. Round to + // the closest side. + if (x > width()) { + x = width(); + i = !rtl; + } else { + x = 0; + i = rtl; + } + } //lyxerr << "=> p=" << pos + i << " x=" << x << endl; return pos + i; + } @@ -389,7 +376,8 @@ void Row::shorten_if_needed(pos_type const keep, int const w) // If there is a paragraph marker, it should be taken in account if (elements_.size() == 2) xstr -= back().width(); - pos_type new_pos = front.x2pos(xstr, true); + //FIXME: use FontMetrics::x2pos here?? handle rtl? + pos_type new_pos = front.x2pos(xstr); front.str = front.str.substr(0, new_pos - pos_); front.dim.wid = xstr; front.endpos = new_pos; diff --git a/src/Row.h b/src/Row.h index 7725427..2aa09a6 100644 --- a/src/Row.h +++ b/src/Row.h @@ -71,11 +71,9 @@ public: /** Return character position that is the closest to * pixel position \param x. The value \param x is - * rounded to the actual pixel position. If \param - * short is true, the pixel value is rounded by - * default. + * adjusted to the actual pixel position. */ - pos_type x2pos(double &x, bool low = false) const; + pos_type x2pos(double &x) const; // Returns the position on left side of the element. pos_type left_pos() const; diff --git a/src/frontends/FontMetrics.h b/src/frontends/FontMetrics.h index 59de630..56e47ea 100644 --- a/src/frontends/FontMetrics.h +++ b/src/frontends/FontMetrics.h @@ -77,6 +77,19 @@ public: virtual int width(docstring const & s) const = 0; /// FIXME ?? virtual int signedWidth(docstring const & s) const = 0; + /** + * return the x offset of a position in the string. The + * direction of the string is forced, and the returned value + * is from the left edge of the word, not from the start of the string. + */ + virtual int pos2x(docstring const & s, int pos, bool rtl) const = 0; + /** + * return the position in the string for a given x offset. The + * direction of the string is forced, and the returned value + * is from the left edge of the word, not from the start of the string. + * the offset x is updated to match the closest position in the string. + */ + virtual int x2pos(docstring const & s, int & x, bool rtl) const = 0; /// return char dimension for the font. virtual Dimension const dimension(char_type c) const = 0; /** diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 27d4446..2ac588c 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -23,6 +23,8 @@ #include "support/lassert.h" +#include <QTextLayout> + using namespace std; namespace lyx { @@ -50,7 +52,7 @@ inline QChar const ucs4_to_qchar(char_type const ucs4) } // anon namespace -GuiFontMetrics::GuiFontMetrics(QFont const & font) : metrics_(font, 0) +GuiFontMetrics::GuiFontMetrics(QFont const & font) : font_(font), metrics_(font, 0) { } @@ -140,6 +142,58 @@ int GuiFontMetrics::signedWidth(docstring const & s) const return width(s); } +namespace { +void setTextLayout(QTextLayout & tl, docstring const & s, QFont const & font, + bool const rtl) +{ + QString qs; + /* In LyX, the character direction is forced by the language. + * Therefore, we have to signal that fact to Qt. + * Source: http://www.iamcal.com/understanding-bidirectional-text/ + */ + // Left-to-right override: forces to draw text left-to-right + char_type const LRO = 0x202D; + // Right-to-left override: forces to draw text right-to-left + char_type const RLO = 0x202E; + // Pop directional formatting: return to previous state + char_type const PDF = 0x202C; + if (rtl) + qs = toqstr(RLO + s + PDF); + else + qs = toqstr(LRO + s + PDF); + + tl.setText(qs); + tl.setFont(font); + tl.beginLayout(); + tl.createLine(); + tl.endLayout(); +} +} + + +int GuiFontMetrics::pos2x(docstring const & s, int const pos, bool const rtl) const +{ + QTextLayout tl; + setTextLayout(tl, s, font_, rtl); + return tl.lineForTextPosition(pos + 1).cursorToX(pos + 1); +} + + +int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl) const +{ + QTextLayout tl; + setTextLayout(tl, s, font_, rtl); + int pos = tl.lineForTextPosition(0).xToCursor(x); + // take into account the unicode formatting characters + if (pos > 0) + --pos; + if (pos > int(s.length())) + pos = s.length(); + // correct x value to the actual cursor position. + x = tl.lineForTextPosition(0).cursorToX(pos + 1); + return pos; +} + void GuiFontMetrics::rectText(docstring const & str, int & w, int & ascent, int & descent) const diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h index a2c57ec..ac1e714 100644 --- a/src/frontends/qt4/GuiFontMetrics.h +++ b/src/frontends/qt4/GuiFontMetrics.h @@ -18,6 +18,7 @@ #include <map> +#include <QFont> #include <QFontMetrics> #include <QHash> @@ -41,6 +42,8 @@ public: virtual int rbearing(char_type c) const; virtual int width(docstring const & s) const; virtual int signedWidth(docstring const & s) const; + virtual int pos2x(docstring const & s, int pos, bool rtl) const; + virtual int x2pos(docstring const & s, int & x, bool rtl) const; virtual Dimension const dimension(char_type c) const; virtual void rectText(docstring const & str, @@ -55,6 +58,9 @@ public: int width(QString const & str) const; private: + /// The font + QFont font_; + /// Metrics on the font QFontMetrics metrics_;