On Tue, Jul 21, 2015 at 6:01 PM, Jean-Marc Lasgouttes <lasgout...@lyx.org> wrote: > commit f65f3adbf73283a288f5d186b51b6b98c9ecb51d > Author: Jean-Marc Lasgouttes <lasgout...@lyx.org> > Date: Sun Jul 19 01:22:10 2015 +0200 > > Do not break row elements at spaces > > The goal of this commit is to make painting faster by reducing the > number of strings to paint. To this end, it is necessary to include > spaces in row elements. > > Also importantly, this commit should fix existing problems with line > breaking in chinese text. > > * TextMetrics::breakRow: do not do anything special for word separators. > > * Row::add: when adding a character to a row element, keep the string > width updated. If need be, it is possible to tweak this by updating > every 10 characters, for example. > > * GuiFontMetrics::breakAt (new): use QTextLayout to break text either > at word boundary or at an arbitrary width. > > * Row::Element::breakAt: use the above method. > > * Row::shortenIfNeeded: simplify now that because there is no need for > handling separator elements. This will be taken care of by the > improved breakAt. > > Two things remain to be done: > > * remove all traces of separator row element > > * re-implement text justification.
This commit seems to have caused a regression in continuous spellcheck
for me. Continuous spellcheck still works in some cases but not in
all. Attached is a case where it does not work. Two words are
misspelled: "tzhat" and "pointwise-consistent". Note that Tools >
Spellchecker still works as expected.
Scott
On Tue, Jul 21, 2015 at 6:01 PM, Jean-Marc Lasgouttes
<lasgout...@lyx.org> wrote:
> commit f65f3adbf73283a288f5d186b51b6b98c9ecb51d
> Author: Jean-Marc Lasgouttes <lasgout...@lyx.org>
> Date: Sun Jul 19 01:22:10 2015 +0200
>
> Do not break row elements at spaces
>
> The goal of this commit is to make painting faster by reducing the
> number of strings to paint. To this end, it is necessary to include
> spaces in row elements.
>
> Also importantly, this commit should fix existing problems with line
> breaking in chinese text.
>
> * TextMetrics::breakRow: do not do anything special for word separators.
>
> * Row::add: when adding a character to a row element, keep the string
> width updated. If need be, it is possible to tweak this by updating
> every 10 characters, for example.
>
> * GuiFontMetrics::breakAt (new): use QTextLayout to break text either
> at word boundary or at an arbitrary width.
>
> * Row::Element::breakAt: use the above method.
>
> * Row::shortenIfNeeded: simplify now that because there is no need for
> handling separator elements. This will be taken care of by the
> improved breakAt.
>
> Two things remain to be done:
>
> * remove all traces of separator row element
>
> * re-implement text justification.
>
> diff --git a/src/Row.cpp b/src/Row.cpp
> index 57f02e2..5118f17 100644
> --- a/src/Row.cpp
> +++ b/src/Row.cpp
> @@ -24,6 +24,7 @@
>
> #include "support/debug.h"
> #include "support/lassert.h"
> +#include "support/lstrings.h"
> #include "support/lyxalgo.h"
>
> #include <ostream>
> @@ -32,6 +33,7 @@ using namespace std;
>
> namespace lyx {
>
> +using support::rtrim;
> using frontend::FontMetrics;
>
> double Row::Element::pos2x(pos_type const i) const
> @@ -96,24 +98,21 @@ pos_type Row::Element::x2pos(int &x) const
> }
>
>
> -bool Row::Element::breakAt(int w)
> +bool Row::Element::breakAt(int w, bool force)
> {
> if (type != STRING || dim.wid <= w)
> return false;
>
> bool const rtl = font.isVisibleRightToLeft();
> - if (rtl)
> - w = dim.wid - w;
> - pos_type new_pos = x2pos(w);
> - if (new_pos == pos)
> - return false;
> - str = str.substr(0, new_pos - pos);
> - if (rtl)
> - dim.wid -= w;
> - else
> - dim.wid = w;
> - endpos = new_pos;
> - return true;
> + FontMetrics const & fm = theFontMetrics(font);
> + int x = w;
> + if(fm.breakAt(str, x, rtl, force)) {
> + dim.wid = x;
> + endpos = pos + str.length();
> + //lyxerr << "breakAt(" << w << ") Row element Broken at " <<
> x << "(w(str)=" << fm.width(str) << "): e=" << *this << endl;
> + return true;
> + }
> + return false;
> }
>
>
> @@ -278,6 +277,7 @@ void Row::finalizeLast()
> elt.final = true;
>
> if (elt.type == STRING) {
> + dim_.wid -= elt.dim.wid;
> elt.dim.wid = theFontMetrics(elt.font).width(elt.str);
> dim_.wid += elt.dim.wid;
> }
> @@ -304,8 +304,11 @@ void Row::add(pos_type const pos, char_type const c,
> Element e(STRING, pos, f, ch);
> elements_.push_back(e);
> }
> + dim_.wid -= back().dim.wid;
> back().str += c;
> back().endpos = pos + 1;
> + back().dim.wid = theFontMetrics(back().font).width(back().str);
> + dim_.wid += back().dim.wid;
> }
>
>
> @@ -357,33 +360,17 @@ void Row::shortenIfNeeded(pos_type const keep, int
> const w)
> {
> if (empty() || width() <= w)
> return;
> -
> Elements::iterator const beg = elements_.begin();
> Elements::iterator const end = elements_.end();
> - Elements::iterator last_sep = elements_.end();
> - int last_width = 0;
> int wid = left_margin;
>
> Elements::iterator cit = beg;
> for ( ; cit != end ; ++cit) {
> - if (cit->type == SEPARATOR && cit->pos >= keep) {
> - last_sep = cit;
> - last_width = wid;
> - }
> - if (wid + cit->dim.wid > w)
> + if (cit->endpos >= keep && wid + cit->dim.wid > w)
> break;
> wid += cit->dim.wid;
> }
>
> - if (last_sep != end) {
> - // We have found a suitable separator. This is the
> - // common case.
> - end_ = last_sep->endpos;
> - dim_.wid = last_width;
> - elements_.erase(last_sep, end);
> - return;
> - }
> -
> if (cit == end) {
> // This should not happen since the row is too long.
> LYXERR0("Something is wrong cannot shorten row: " << *this);
> @@ -397,23 +384,41 @@ void Row::shortenIfNeeded(pos_type const keep, int
> const w)
> wid -= cit->dim.wid;
> }
>
> + // Try to break this row cleanly (at word boundary)
> + if (cit->breakAt(w - wid, false)) {
> + end_ = cit->endpos;
> + // after breakAt, there may be spaces at the end of the
> + // string, but they are not counted in the string length
> + // (qtextlayout feature, actually). We remove them, but do not
> + // change the endo of the row, since the spaces at row break
> + // are invisible.
> + cit->str = rtrim(cit->str);
> + cit->endpos = cit->pos + cit->str.length();
> + dim_.wid = wid + cit->dim.wid;
> + // If there are other elements, they should be removed.
> + elements_.erase(next(cit, 1), end);
> + return;
> + }
> +
> if (cit != beg) {
> - // There is no separator, but several elements (probably
> - // insets) have been added. We can cut at this place.
> + // There is no separator, but several elements have been
> + // added. We can cut right here.
> end_ = cit->pos;
> dim_.wid = wid;
> elements_.erase(cit, end);
> return;
> }
>
> - /* If we are here, it means that we have not found a separator
> - * to shorten the row. There is one case where we can do
> - * something: when we have one big string, maybe with some
> - * other things after it.
> + /* If we are here, it means that we have not found a separator to
> + * shorten the row. Let's try to break it again, but not at word
> + * boundary this time.
> */
> - if (cit->breakAt(w - left_margin)) {
> + if (cit->breakAt(w - wid, true)) {
> end_ = cit->endpos;
> - dim_.wid = left_margin + cit->dim.wid;
> + // See comment above.
> + cit->str = rtrim(cit->str);
> + cit->endpos = cit->pos + cit->str.length();
> + dim_.wid = wid + cit->dim.wid;
> // If there are other elements, they should be removed.
> elements_.erase(next(cit, 1), end);
> }
> diff --git a/src/Row.h b/src/Row.h
> index 9039ff8..490aa6b 100644
> --- a/src/Row.h
> +++ b/src/Row.h
> @@ -86,10 +86,12 @@ public:
> * adjusted to the actual pixel position.
> */
> pos_type x2pos(int &x) const;
> - /** Break the element if possible, so that its width is
> - * less then \param w. Returns true on success.
> + /** Break the element if possible, so that its width is less
> + * than \param w. Returns true on success. When \param force
> + * is true, the string is cut at any place, other wise it
> + * respects the row breaking rules of characters.
> */
> - bool breakAt(int w);
> + bool breakAt(int w, bool force);
>
> // Returns the position on left side of the element.
> pos_type left_pos() const;
> diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
> index 2f314b5..c3017d8 100644
> --- a/src/TextMetrics.cpp
> +++ b/src/TextMetrics.cpp
> @@ -819,7 +819,7 @@ void TextMetrics::breakRow(Row & row, int const
> right_margin, pit_type const pit
> // or the end of the par, then build a representation of the row.
> pos_type i = pos;
> FontIterator fi = FontIterator(*this, par, pit, pos);
> - while (i < end && row.width() < width) {
> + while (i < end && row.width() <= width) {
> char_type c = par.getChar(i);
> // The most special cases are handled first.
> if (par.isInset(i)) {
> @@ -837,13 +837,6 @@ void TextMetrics::breakRow(Row & row, int const
> right_margin, pit_type const pit
> int const add = max(fm.width(par.layout().labelsep),
> labelEnd(pit) - row.width());
> row.addSpace(i, add, *fi, par.lookupChange(i));
> - } else if (par.isLineSeparator(i)) {
> - // In theory, no inset has this property. If
> - // this is done, a new addSeparator which
> - // takes an inset as parameter should be
> - // added.
> - LATTEST(!par.isInset(i));
> - row.addSeparator(i, c, *fi, par.lookupChange(i));
> } else if (c == '\t')
> row.addSpace(i,
> theFontMetrics(*fi).width(from_ascii(" ")),
> *fi, par.lookupChange(i));
> @@ -905,6 +898,7 @@ void TextMetrics::breakRow(Row & row, int const
> right_margin, pit_type const pit
>
> // make sure that the RTL elements are in reverse ordering
> row.reverseRTL(is_rtl);
> + //LYXERR0("breakrow: row is " << row);
> }
>
>
> diff --git a/src/frontends/FontMetrics.h b/src/frontends/FontMetrics.h
> index 53f57a2..84000d1 100644
> --- a/src/frontends/FontMetrics.h
> +++ b/src/frontends/FontMetrics.h
> @@ -93,6 +93,14 @@ public:
> * 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;
> + /**
> + * Break string at width at most x.
> + * \return true if successful
> + * \param rtl is true for right-to-left layout
> + * \param force is false for breaking at word separator, true for
> + * arbitrary position.
> + */
> + virtual bool breakAt(docstring & s, int & x, bool rtl, bool force)
> 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 b488b68..804813a 100644
> --- a/src/frontends/qt4/GuiFontMetrics.cpp
> +++ b/src/frontends/qt4/GuiFontMetrics.cpp
> @@ -183,6 +183,31 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x,
> bool const rtl) const
> }
>
>
> +bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool
> const force) const
> +{
> + if (s.empty())
> + return false;
> + QTextLayout tl;
> + tl.setText(toqstr(s));
> + tl.setFont(font_);
> + // Note that both setFlags and the enums are undocumented
> + tl.setFlags(rtl ? Qt::TextForceRightToLeft :
> Qt::TextForceLeftToRight);
> + QTextOption to;
> + to.setWrapMode(force ? QTextOption::WrapAnywhere :
> QTextOption::WordWrap);
> + tl.setTextOption(to);
> + tl.beginLayout();
> + QTextLine line = tl.createLine();
> + line.setLineWidth(x);
> + tl.createLine();
> + tl.endLayout();
> + if (int(line.naturalTextWidth()) > x)
> + return false;
> + x = int(line.naturalTextWidth());
> + s = s.substr(0, line.textLength());
> + return true;
> +}
> +
> +
> 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 7555929..bc6b24a 100644
> --- a/src/frontends/qt4/GuiFontMetrics.h
> +++ b/src/frontends/qt4/GuiFontMetrics.h
> @@ -45,6 +45,7 @@ public:
> 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 bool breakAt(docstring & s, int & x, bool rtl, bool force)
> const;
> virtual Dimension const dimension(char_type c) const;
>
> virtual void rectText(docstring const & str,
regression-f65f3adb.21.lyx
Description: application/lyx
