Dear list
Attached is a patch that changes some details about text justification.
The spacing is distributed proportionally to the em size (if a row mixes
two font heights one can see distinguish spaces in the big font from
spaces in the small font). Moreover an upper bound is set on the spacing
(1.5em) to prevent situations where there are weirdly wide gaps between
words (inspired my Kindle).
Sincerely
Guillaume
>From 3895e6e7265f0767259e7c7987281376aaca4bad Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Sat, 13 Aug 2016 19:03:02 +0100
Subject: [PATCH] On-screen justification: stretch in proportion with the em,
up to a limit
1) Distinguish expanding characters from separators, to fit with Qt's notion of
expanding character which comes from the Unicode std. CountExpanders() is moved
to FontMetrics to fix a discrepancy with the duplicate implementation from
598f7e4a.
2) Make these expanders stretch on-screen proportionally to the em of the font.
If a row mixes large and small text, LyX let us see which spaces are set in the
bigger font.
3) Now that the stretch is defined in ems, add a limit such that an expander
never stretches more than 1.5em to avoid weird and hard to read justified lines.
4) Add a return boolean to setSeparatorExtraWidth for future use.
---
src/Row.cpp | 71 ++++++++++++++++++++++++++----------
src/Row.h | 22 ++++++++---
src/TextMetrics.cpp | 13 +++----
src/frontends/FontMetrics.h | 4 ++
src/frontends/qt4/GuiFontMetrics.cpp | 21 +++++++++++
src/frontends/qt4/GuiFontMetrics.h | 2 +
src/frontends/qt4/GuiPainter.cpp | 3 +-
7 files changed, 102 insertions(+), 34 deletions(-)
diff --git a/src/Row.cpp b/src/Row.cpp
index 58a2380..0cb3b18 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -37,23 +37,39 @@ using support::rtrim;
using frontend::FontMetrics;
+// Maximum length that a space can be stretched when justifying text
+static double const MAX_SPACE_STRETCH = 1.5; //em
+
+
int Row::Element::countSeparators() const
{
if (type != STRING)
return 0;
- // Consecutive spaces count as only one separator.
- bool wasspace = false;
- int nsep = 0;
- for (size_t i = 0 ; i < str.size() ; ++i) {
- if (str[i] == ' ') {
- if (!wasspace) {
- ++nsep;
- wasspace = true;
- }
- } else
- wasspace = false;
- }
- return nsep;
+ return count(str.begin(), str.end(), ' ');
+}
+
+
+int Row::Element::countExpanders() const
+{
+ if (type != STRING)
+ return 0;
+ return theFontMetrics(font).countExpanders(str);
+}
+
+
+int Row::Element::expansionAmount() const
+{
+ if (type != STRING)
+ return 0;
+ return countExpanders() * theFontMetrics(font).em();
+}
+
+
+void Row::Element::setExtra(double extra_per_em)
+{
+ if (type != STRING)
+ return;
+ extra = extra_per_em * theFontMetrics(font).em();
}
@@ -234,7 +250,7 @@ ostream & operator<<(ostream & os, Row::Element const & e)
switch (e.type) {
case Row::STRING:
os << "STRING: `" << to_utf8(e.str) << "' ("
- << e.countSeparators() << " sep.), ";
+ << e.countExpanders() << " expanders.), ";
break;
case Row::VIRTUAL:
os << "VIRTUAL: `" << to_utf8(e.str) << "', ";
@@ -311,13 +327,28 @@ int Row::countSeparators() const
}
-void Row::setSeparatorExtraWidth(double w)
+bool Row::setExtraWidth(int w)
{
- separator = w;
- iterator const end = elements_.end();
- for (iterator it = elements_.begin() ; it != end ; ++it)
- if (it->type == Row::STRING)
- it->extra = w;
+ if (w < 0)
+ // this is not expected to happen (but it does)
+ return false;
+ // amount of expansion: number of expanders time the em value for each
+ // string element
+ int exp_amount = 0;
+ for (Row::Element const & e : elements_)
+ exp_amount += e.expansionAmount();
+ // extra length per unit of separator
+ double extra_per_em = double(w) / exp_amount;
+ if (extra_per_em > MAX_SPACE_STRETCH)
+ // do not stretch more than MAX_SPACE_STRETCH em per expander
+ return false;
+ // add extra length to each element proportionally to its em.
+ for (Row::Element & e : elements_)
+ if (e.type == Row::STRING)
+ e.setExtra(extra_per_em);
+ // update row dimension
+ dim_.wid += w;
+ return true;
}
diff --git a/src/Row.h b/src/Row.h
index f69eeca..7ae9c11 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -61,12 +61,22 @@ public:
: type(t), pos(p), endpos(p + 1), inset(0),
extra(0), font(f), change(ch), final(false) {}
- // Return total width of element, including separator overhead
- // FIXME: Cache this value or the number of separators?
- double full_width() const { return dim.wid + extra * countSeparators(); }
// Return the number of separator in the element (only STRING type)
int countSeparators() const;
+ // Return total width of element, including separator overhead
+ // FIXME: Cache this value or the number of expanders?
+ double full_width() const { return dim.wid + extra * countExpanders(); }
+ // Return the number of expanding characters in the element (only STRING
+ // type).
+ int countExpanders() const;
+ // Return the amount of expansion: the number of expanding characters
+ // that get stretched during justification, times the em of the font
+ // (only STRING type).
+ int expansionAmount() const;
+ // set extra proportionally to the font em value.
+ void setExtra(double extra_per_em);
+
/** Return position in pixels (from the left) of position
* \param i in the row element.
*/
@@ -182,8 +192,10 @@ public:
// Return the number of separators in the row
int countSeparators() const;
- // Set the extra spacing for every separator in STRING elements
- void setSeparatorExtraWidth(double w);
+ // Set the extra spacing for every expanding character in STRING-type
+ // elements. \param w is the total amount of extra width for the row to be
+ // distributed among expanders. \return false if the justification fails.
+ bool setExtraWidth(int w);
///
void add(pos_type pos, Inset const * ins, Dimension const & dim,
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index e1153df..8f7ac82 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -625,18 +625,14 @@ void TextMetrics::computeRowMetrics(Row & row, int width) const
// Common case : there is no hfill, and the alignment will be
// meaningful
switch (getAlign(par, row)) {
- case LYX_ALIGN_BLOCK: {
- int const ns = row.countSeparators();
- // If we have separators, then stretch the row
- if (ns) {
- row.setSeparatorExtraWidth(double(w) / ns);
- row.dimension().wid += w;
- } else if (text_->isRTL(par)) {
+ case LYX_ALIGN_BLOCK:
+ // Expand expanding characters by a total of w
+ if (!row.setExtraWidth(w) && text_->isRTL(par)) {
+ // Justification failed and the text is RTL: align to the right
row.left_margin += w;
row.dimension().wid += w;
}
break;
- }
case LYX_ALIGN_RIGHT:
row.left_margin += w;
row.dimension().wid += w;
@@ -655,6 +651,7 @@ void TextMetrics::computeRowMetrics(Row & row, int width) const
return;
}
+ // Case nh > 0. There are hfill separators.
hfill = w / nh;
hfill_rem = w % nh;
row.dimension().wid += w;
diff --git a/src/frontends/FontMetrics.h b/src/frontends/FontMetrics.h
index e8493d1..8358e2e 100644
--- a/src/frontends/FontMetrics.h
+++ b/src/frontends/FontMetrics.h
@@ -142,6 +142,10 @@ public:
inline int center(char_type c) const {
return (rbearing(c) - lbearing(c)) / 2;
}
+
+ /// return the number of expanding characters taken into account for
+ /// increased inter-word spacing during justification
+ virtual int countExpanders(docstring const & str) const = 0;
};
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp
index eade8cc..6397425 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -214,6 +214,27 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
}
+int GuiFontMetrics::countExpanders(docstring const & str) const
+{
+ // 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;
+ int nexp = 0;
+ for (char_type c : str)
+ if (c > 0x0d && QChar(c).isSpace()) {
+ if (!wasspace) {
+ ++nexp;
+ wasspace = true;
+ }
+ } else
+ wasspace = false;
+ return nexp;
+}
+
+
bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const force) const
{
if (s.empty())
diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h
index 8c8daab..9b4c2c4 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -60,6 +60,8 @@ public:
int & width,
int & ascent,
int & descent) const;
+
+ int countExpanders(docstring const & str) const;
///
int width(QString const & str) const;
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index e613f3f..6c89ff5 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -423,7 +423,8 @@ void GuiPainter::text(int x, int y, docstring const & s,
int textwidth = 0;
if (tw == 0.0)
// Note that we have to take in account space stretching (word spacing)
- textwidth = fm.width(s) + count(s.begin(), s.end(), ' ') * wordspacing;
+ textwidth = fm.width(s) +
+ static_cast<int>(fm.countExpanders(s) * wordspacing);
else
textwidth = static_cast<int>(tw);
--
2.7.4
