Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Jean-Marc Lasgouttes]

OK.

JMarc

Le 6 novembre 2016 16:33:09 GMT+01:00, Guillaume Munch  a écrit :
>
>Most of the times it is already clear from the other lines that the
>text 
>is justified. In addition, on-screen justification cannot be much more 
>than cosmetic anyway, since there is an option to disable it. The goal 
>is to increase readability, and additional clues would only clutter, in
>
>my opinion.

Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Guillaume Munch]

Le 06/11/2016 à 16:11, Jean-Marc Lasgouttes a écrit :

Something I meant to mention: shall we add some on screen cue that the
row has not been completely stretched ?

JMarc


Most of the times it is already clear from the other lines that the text 
is justified. In addition, on-screen justification cannot be much more 
than cosmetic anyway, since there is an option to disable it. The goal 
is to increase readability, and additional clues would only clutter, in 
my opinion.

Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Jean-Marc Lasgouttes]

Something I meant to mention: shall we add some on screen cue that the row has 
not been completely stretched ?

JMarc

Le 6 novembre 2016 14:57:30 GMT+01:00, Guillaume Munch  a écrit :
>Le 29/08/2016 à 14:16, Jean-Marc Lasgouttes a écrit :
>>
>> The patch looks good.
>
>It's in at b30f8d3c4b

Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Guillaume Munch]

Le 29/08/2016 à 14:16, Jean-Marc Lasgouttes a écrit :


The patch looks good.


It's in at b30f8d3c4b

Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Guillaume Munch]

Le 29/08/2016 à 13:16, Jean-Marc Lasgouttes a écrit :

Le 29/08/2016 à 00:29, Guillaume Munch a écrit :

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).


The patch looks good. Where does the 1.5em come from?


Arbitrary and can be adjusted. You can experiment yourself.



Does this fix #10117?


Not at all.

Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

[Jean-Marc Lasgouttes]

Le 29/08/2016 à 00:29, Guillaume Munch a écrit :

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).


The patch looks good. Where does the 1.5em come from?

Does this fix #10117?

JMarc

[patch] On-screen justification: stretch in proportion with the em, up to a limit

[Guillaume Munch]

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 
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