From 66a3d64346332e47252b37dbc0f80158738987dc Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Thu, 3 Jan 2019 16:03:48 +0100
Subject: [PATCH] Try to compute row height like it should be done

Currently, our computation of row height is not completely standard:
* we ignore completely the QFontMetrics::leading() parameter

* we add arbitrarily 2 hardcoded pixels to the height.

This patch reverses these two choices, which leads to
* slightly larger spacing for MinionPro (which has a big leading).

* an additional spacing of 20% font height that depends on dpi and zoom.

Visual inspection with LibreOffice seems to imply that it disregards
the font leading but uses a interline which is 20% larger than the
font height.
---
 src/Row.cpp                          |  5 ++++-
 src/TextMetrics.cpp                  | 32 +++++++++++++---------------
 src/frontends/FontMetrics.h          |  2 ++
 src/frontends/qt4/GuiFontMetrics.cpp |  6 ++++++
 src/frontends/qt4/GuiFontMetrics.h   |  1 +
 5 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/Row.cpp b/src/Row.cpp
index 697e526094..1a12e9d695 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -376,8 +376,11 @@ void Row::finalizeLast()
 
 	if (elt.type == STRING) {
 		dim_.wid -= elt.dim.wid;
-		elt.dim.wid = theFontMetrics(elt.font).width(elt.str);
+		FontMetrics const & fm = theFontMetrics(elt.font);
+		elt.dim.wid = fm.width(elt.str);
 		dim_.wid += elt.dim.wid;
+		dim_.asc = fm.maxAscent() + fm.leading();
+		dim_.des = fm.maxDescent();
 	}
 }
 
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 29c81da8bd..094b80d9c5 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -48,6 +48,7 @@
 #include "support/convert.h"
 #include "support/debug.h"
 #include "support/lassert.h"
+#include "support/lyxlib.h"
 
 #include <stdlib.h>
 #include <cmath>
@@ -61,6 +62,10 @@ using frontend::FontMetrics;
 
 namespace {
 
+// the somewhat arbitrary leading added between rows. This is 20% of
+// the characters height, inluding the possible leading of the font.
+// 20% is a standard value used by LaTeX and word processors.
+double const extra_leading = 0.2;
 
 int numberOfLabelHfills(Paragraph const & par, Row const & row)
 {
@@ -1088,27 +1093,19 @@ void TextMetrics::setRowHeight(Row & row) const
 	// Initial value for ascent (useful if row is empty).
 	Font const font = displayFont(row.pit(), row.pos());
 	FontMetrics const & fm = theFontMetrics(font);
-	int maxasc = int(fm.maxAscent() * spacing_val);
-	int maxdes = int(fm.maxDescent() * spacing_val);
+	int maxasc = fm.maxAscent() + fm.leading();
+	int maxdes = fm.maxDescent();
 
 	// Find the ascent/descent of the row contents
 	for (Row::Element const & e : row) {
-		if (e.inset) {
-			maxasc = max(maxasc, e.dim.ascent());
-			maxdes = max(maxdes, e.dim.descent());
-		} else {
-			FontMetrics const & fm2 = theFontMetrics(e.font);
-			maxasc = max(maxasc, int(fm2.maxAscent() * spacing_val));
-			maxdes = max(maxdes, int(fm2.maxDescent() * spacing_val));
-		}
+		maxasc = max(maxasc, e.dim.ascent());
+		maxdes = max(maxdes, e.dim.descent());
 	}
 
-	// This is nicer with box insets
-	++maxasc;
-	++maxdes;
-
-	row.dimension().asc = maxasc;
-	row.dimension().des = maxdes;
+	// Add some leading (split between before and after)
+	int const leading = support::iround(extra_leading * (maxasc + maxdes));
+	row.dimension().asc = int((maxasc + leading - leading / 2) * spacing_val);
+	row.dimension().des = int((maxdes + leading / 2) * spacing_val);
 }
 
 
@@ -2009,7 +2006,8 @@ void TextMetrics::completionPosAndDim(Cursor const & cur, int & x, int & y,
 
 int defaultRowHeight()
 {
-	return int(theFontMetrics(sane_font).maxHeight() *  1.2);
+	FontMetrics const & fm = theFontMetrics(sane_font);
+	return support::iround(fm.maxHeight() * (1 + extra_leading) + fm.leading());
 }
 
 } // namespace lyx
diff --git a/src/frontends/FontMetrics.h b/src/frontends/FontMetrics.h
index 4472aa7008..a35a26b481 100644
--- a/src/frontends/FontMetrics.h
+++ b/src/frontends/FontMetrics.h
@@ -60,6 +60,8 @@ public:
 	virtual int maxAscent() const = 0;
 	/// return the maximum descent of the font
 	virtual int maxDescent() const = 0;
+	/// return the default leading of the font (often 0)
+	virtual int leading() const = 0;
 	/// return default dimension of the font.
 	/// \warning \c width is set to zero.
 	virtual Dimension const defaultDimension() const = 0;
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp
index 3911fd1b47..b57f514aa6 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -111,6 +111,12 @@ int GuiFontMetrics::maxDescent() const
 }
 
 
+int GuiFontMetrics::leading() const
+{
+	return metrics_.leading();
+}
+
+
 int GuiFontMetrics::em() const
 {
 	return QFontInfo(font_).pixelSize();
diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h
index 8a2ebe2525..6d50d35425 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -36,6 +36,7 @@ public:
 
 	virtual int maxAscent() const;
 	virtual int maxDescent() const;
+	virtual int leading() const;
 	virtual Dimension const defaultDimension() const;
 	virtual int em() const;
 	virtual int xHeight() const;
-- 
2.39.2