From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Fri, 8 Apr 2016 14:34:46 +0000 (+0200)
Subject: Set vertical margins in redoParagraph, not setRowHeight
X-Git-Tag: 2.3.0alpha1~1624
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=656b7f5ab7de9d96a0ec3379dc47fd138b1caa4f;p=features.git

Set vertical margins in redoParagraph, not setRowHeight

It is actually easier to set the 20 pixels margin in redoParagraph, since it is not necessary to take newlines in account when deciding what is the real last row of the paragraph.

Moreover this solves the following bug (present in 2.1.x too): when the document ends with a newling, the bottom margin disappears.

Update PAINTING_ANALYSIS with new tasks.
---

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index 74aaa99a74..acb033d7fe 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -100,7 +100,25 @@ version.
 
 ==> more versions, no optional parameters.
 
-** make Inset::display() more useful
+** DONE When a document ends with a newline, add the bottom margin anyway
+
+The code that tests for a newline was added at 6bb98d07 in 2007.
+
+** When a paragraph ends with a newline, compute correctly the height of the extra row.
+** Rewrite TextMetrics::completionPosAndDim using row information
+
+Currently it uses setRowHeight in a very weird way. In particular the
+topBottomSpace parameter should be removed after that.
+
+** Rewrite TextMetrics::editXY, checkInsetHit using row information (getPosNearX)?
+
+   The helper version should return a Row::Element instead of an InsetTable.
+
+** TODO make Inset::display() more useful
+
+[This has been started in the features/betterbreak branch. Time will
+tell whether it really helps. The question in particular is the
+handling of separator insets]
 
 Extending the DisplayType enum would allow to remove special cases
 from the code.
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 2aad0cc75b..a1f3892a8c 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -471,6 +471,21 @@ bool TextMetrics::redoParagraph(pit_type const pit)
 		pm.dim().des += row.height();
 	} while (first < par.size() || need_new_row);
 
+	// FIXME: It might be better to move this in another method
+	// specially tailored for the main text.
+	// Top and bottom margin of the document (only at top-level)
+	if (text_->isMainText()) {
+		if (pit == 0) {
+			pm.rows().front().dimension().asc += 20;
+			pm.dim().des += 20;
+		}
+		ParagraphList const & pars = text_->paragraphs();
+		if (pit + 1 == pit_type(pars.size())) {
+			pm.rows().back().dimension().des += 20;
+			pm.dim().des += 20;
+		}
+	}
+
 	if (row_index < pm.rows().size())
 		pm.rows().resize(row_index);
 
@@ -1044,19 +1059,6 @@ void TextMetrics::setRowHeight(Row & row, pit_type const pit,
 	maxasc += int(layoutasc  * 2 / (2 + pars[pit].getDepth()));
 	maxdes += int(layoutdesc * 2 / (2 + pars[pit].getDepth()));
 
-	// FIXME: the correct way is to do the following is to move the
-	// following code in another method specially tailored for the
-	// main Text. The following test is thus bogus.
-	// Top and bottom margin of the document (only at top-level)
-	if (text_->isMainText() && topBottomSpace) {
-		if (pit == 0 && row.pos() == 0)
-			maxasc += 20;
-		if (pit + 1 == pit_type(pars.size()) &&
-		    row.endpos() == par.size() &&
-				!(row.endpos() > 0 && par.isNewline(row.endpos() - 1)))
-			maxdes += 20;
-	}
-
 	row.dimension().asc = maxasc + labeladdon;
 	row.dimension().des = maxdes;
 }