[LyX/master] Revert "Improve handling of top and bottom margin"

Jean-Marc Lasgouttes lasgouttes at lyx.org
Sun Jul 12 21:36:35 UTC 2020


commit 32f06d01ec96ca0d2144ae241f561ce8daa8f244
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Mon Jul 13 00:00:36 2020 +0200

    Revert "Improve handling of top and bottom margin"
    
    It turns out this is not ready at all.
    
    This reverts commit ff7cdf1b74f5c17a966af24dc70d49fc162f007e.
---
 src/BufferView.cpp               |   77 ++++++++++++++++++-------------------
 src/BufferView.h                 |    5 +--
 src/Row.cpp                      |    1 -
 src/Row.h                        |    8 ----
 src/TextMetrics.cpp              |   40 +++++++++-----------
 src/TextMetrics.h                |   11 +----
 src/frontends/qt/GuiWorkArea.cpp |    5 +-
 src/insets/InsetTabular.cpp      |    2 +-
 8 files changed, 62 insertions(+), 87 deletions(-)

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 3c7940a..2eaa434 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -360,20 +360,6 @@ int BufferView::leftMargin() const
 }
 
 
-int BufferView::topMargin() const
-{
-	// original value was 20px, which is 0.2in at 100dpi
-	return zoomedPixels(20);
-}
-
-
-int BufferView::bottomMargin() const
-{
-	// original value was 20px, which is 0.2in at 100dpi
-	return zoomedPixels(20);
-}
-
-
 int BufferView::inPixels(Length const & len) const
 {
 	Font const font = buffer().params().getFont();
@@ -596,25 +582,29 @@ void BufferView::updateScrollbar()
 	}
 
 	// Look at paragraph heights on-screen
-	for (pit_type pit = tm.firstPit(); pit <= tm.lastPit(); ++pit) {
+	pair<pit_type, ParagraphMetrics const *> first = tm.first();
+	pair<pit_type, ParagraphMetrics const *> last = tm.last();
+	for (pit_type pit = first.first; pit <= last.first; ++pit) {
 		d->par_height_[pit] = tm.parMetrics(pit).height();
 		LYXERR(Debug::SCROLLING, "storing height for pit " << pit << " : "
 			<< d->par_height_[pit]);
 	}
 
-	bool first_visible = tm.firstPit() == 0 && tm.topPosition() >= 0;
-	bool last_visible = tm.lastPit() + 1 == int(parsize) && tm.bottomPosition() <= height_;
+	int top_pos = first.second->position() - first.second->ascent();
+	int bottom_pos = last.second->position() + last.second->descent();
+	bool first_visible = first.first == 0 && top_pos >= 0;
+	bool last_visible = last.first + 1 == int(parsize) && bottom_pos <= height_;
 	if (first_visible && last_visible) {
 		d->scrollbarParameters_.min = 0;
 		d->scrollbarParameters_.max = 0;
 		return;
 	}
 
-	d->scrollbarParameters_.min = tm.topPosition();
-	for (size_t i = 0; i != size_t(tm.firstPit()); ++i)
+	d->scrollbarParameters_.min = top_pos;
+	for (size_t i = 0; i != size_t(first.first); ++i)
 		d->scrollbarParameters_.min -= d->par_height_[i];
-	d->scrollbarParameters_.max = tm.bottomPosition();
-	for (size_t i = tm.lastPit() + 1; i != parsize; ++i)
+	d->scrollbarParameters_.max = bottom_pos;
+	for (size_t i = last.first + 1; i != parsize; ++i)
 		d->scrollbarParameters_.max += d->par_height_[i];
 
 	// The reference is the top position so we remove one page.
@@ -951,9 +941,9 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter)
 		bot_pit = max_pit;
 	}
 
-	if (bot_pit == tm.firstPit() - 1)
+	if (bot_pit == tm.first().first - 1)
 		tm.newParMetricsUp();
-	else if (bot_pit == tm.lastPit() + 1)
+	else if (bot_pit == tm.last().first + 1)
 		tm.newParMetricsDown();
 
 	if (tm.contains(bot_pit)) {
@@ -963,7 +953,8 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter)
 		CursorSlice const & cs = dit.innerTextSlice();
 		int offset = coordOffset(dit).y_;
 		int ypos = pm.position() + offset;
-		Row const & row = pm.getRow(cs.pos(), dit.boundary());
+		Dimension const & row_dim =
+			pm.getRow(cs.pos(), dit.boundary()).dim();
 		int scrolled = 0;
 		if (recenter)
 			scrolled = scroll(ypos - height_/2);
@@ -972,7 +963,7 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter)
 		// the screen height, we scroll to a heuristic value of height_ / 4.
 		// FIXME: This heuristic value should be replaced by a recursive search
 		// for a row in the inset that can be visualized completely.
-		else if (row.height() > height_) {
+		else if (row_dim.height() > height_) {
 			if (ypos < defaultRowHeight())
 				scrolled = scroll(ypos - height_ / 4);
 			else if (ypos > height_ - defaultRowHeight())
@@ -981,14 +972,14 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter)
 
 		// If the top part of the row falls of the screen, we scroll
 		// up to align the top of the row with the top of the screen.
-		else if (ypos - row.totalAscent() < 0 && ypos < height_) {
-			int ynew = row.totalAscent();
+		else if (ypos - row_dim.ascent() < 0 && ypos < height_) {
+			int ynew = row_dim.ascent();
 			scrolled = scrollUp(ynew - ypos);
 		}
 
 		// If the bottom of the row falls of the screen, we scroll down.
-		else if (ypos + row.totalDescent() > height_ && ypos > 0) {
-			int ynew = height_ - row.totalDescent();
+		else if (ypos + row_dim.descent() > height_ && ypos > 0) {
+			int ynew = height_ - row_dim.descent();
 			scrolled = scrollDown(ypos - ynew);
 		}
 
@@ -1006,14 +997,15 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter)
 
 	d->anchor_pit_ = bot_pit;
 	CursorSlice const & cs = dit.innerTextSlice();
-	Row const & row = pm.getRow(cs.pos(), dit.boundary());
+	Dimension const & row_dim =
+		pm.getRow(cs.pos(), dit.boundary()).dim();
 
 	if (recenter)
 		d->anchor_ypos_ = height_/2;
 	else if (d->anchor_pit_ == 0)
 		d->anchor_ypos_ = offset + pm.ascent();
 	else if (d->anchor_pit_ == max_pit)
-		d->anchor_ypos_ = height_ - offset - row.totalDescent();
+		d->anchor_ypos_ = height_ - offset - row_dim.descent();
 	else if (offset > height_)
 		d->anchor_ypos_ = height_ - offset - defaultRowHeight();
 	else
@@ -2449,10 +2441,11 @@ int BufferView::scrollDown(int offset)
 	TextMetrics & tm = d->text_metrics_[text];
 	int const ymax = height_ + offset;
 	while (true) {
-		int bottom_pos = tm.bottomPosition();
+		pair<pit_type, ParagraphMetrics const *> last = tm.last();
+		int bottom_pos = last.second->position() + last.second->descent();
 		if (lyxrc.scroll_below_document)
 			bottom_pos += height_ - minVisiblePart();
-		if (tm.lastPit() + 1 == int(text->paragraphs().size())) {
+		if (last.first + 1 == int(text->paragraphs().size())) {
 			if (bottom_pos <= height_)
 				return 0;
 			offset = min(offset, bottom_pos - height_);
@@ -2473,8 +2466,9 @@ int BufferView::scrollUp(int offset)
 	TextMetrics & tm = d->text_metrics_[text];
 	int ymin = - offset;
 	while (true) {
-		int const top_pos = tm.topPosition();
-		if (tm.firstPit() == 0) {
+		pair<pit_type, ParagraphMetrics const *> first = tm.first();
+		int top_pos = first.second->position() - first.second->ascent();
+		if (first.first == 0) {
 			if (top_pos >= 0)
 				return 0;
 			offset = min(offset, - top_pos);
@@ -2989,7 +2983,7 @@ Point BufferView::coordOffset(DocIterator const & dit) const
 	ParagraphMetrics const & pm = tm.parMetrics(sl.pit());
 
 	LBUFERR(!pm.rows().empty());
-	y -= pm.rows()[0].totalAscent();
+	y -= pm.rows()[0].ascent();
 #if 1
 	// FIXME: document this mess
 	size_t rend;
@@ -3006,7 +3000,7 @@ Point BufferView::coordOffset(DocIterator const & dit) const
 #endif
 	for (size_t rit = 0; rit != rend; ++rit)
 		y += pm.rows()[rit].height();
-	y += pm.rows()[rend].totalAscent();
+	y += pm.rows()[rend].ascent();
 
 	TextMetrics const & bottom_tm = textMetrics(dit.bottom().text());
 
@@ -3195,7 +3189,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret)
 	                         : "\t\t*** START DRAWING ***"));
 	Text & text = buffer_.text();
 	TextMetrics const & tm = d->text_metrics_[&text];
-	int const y = tm.parMetrics(tm.firstPit()).position();
+	int const y = tm.first().second->position();
 	PainterInfo pi(this, pain);
 
 	// Check whether the row where the cursor lives needs to be scrolled.
@@ -3250,7 +3244,8 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret)
 		tm.draw(pi, 0, y);
 
 		// and possibly grey out below
-		int const y2 = tm.bottomPosition();
+		pair<pit_type, ParagraphMetrics const *> lastpm = tm.last();
+		int const y2 = lastpm.second->position() + lastpm.second->descent();
 
 		if (y2 < height_) {
 			Color color = buffer().isInternal()
@@ -3266,7 +3261,9 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret)
 	updateScrollbar();
 
 	// Normalize anchor for next time
-	for (pit_type pit = tm.firstPit(); pit <= tm.lastPit(); ++pit) {
+	pair<pit_type, ParagraphMetrics const *> firstpm = tm.first();
+	pair<pit_type, ParagraphMetrics const *> lastpm = tm.last();
+	for (pit_type pit = firstpm.first; pit <= lastpm.first; ++pit) {
 		ParagraphMetrics const & pm = tm.parMetrics(pit);
 		if (pm.position() + pm.descent() > 0) {
 			if (d->anchor_pit_ != pit
diff --git a/src/BufferView.h b/src/BufferView.h
index fbbd1e7..3cfd623 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -104,12 +104,9 @@ public:
 
 	/// right margin
 	int rightMargin() const;
+
 	/// left margin
 	int leftMargin() const;
-	/// top margin
-	int topMargin() const;
-	/// bottom margin
-	int bottomMargin() const;
 
 	/// return the on-screen size of this length
 	/*
diff --git a/src/Row.cpp b/src/Row.cpp
index 0d92c94..f59b6e4 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -161,7 +161,6 @@ pos_type Row::Element::right_pos() const
 
 Row::Row()
 	: separator(0), label_hfill(0), left_margin(0), right_margin(0),
-	  top_padding(0), bottom_padding(0),
 	  sel_beg(-1), sel_end(-1),
 	  begin_margin_sel(false), end_margin_sel(false),
 	  changed_(true),
diff --git a/src/Row.h b/src/Row.h
index 426f45e..d210174 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -209,10 +209,6 @@ public:
 	int ascent() const { return dim_.asc; }
 	///
 	int descent() const { return dim_.des; }
-	///
-	int totalAscent() const { return ascent() + top_padding; }
-	///
-	int totalDescent() const { return dim_.des + bottom_padding; }
 
 	/// The offset of the left-most cursor position on the row
 	int left_x() const;
@@ -307,10 +303,6 @@ public:
 	int left_margin;
 	/// the right margin of the row
 	int right_margin;
-	/// possible padding above the row
-	int top_padding;
-	/// possible padding below the row
-	int bottom_padding;
 	///
 	mutable pos_type sel_beg;
 	///
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 5b16d36..9f55c5a 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -127,15 +127,18 @@ bool TextMetrics::contains(pit_type pit) const
 }
 
 
-pit_type TextMetrics::firstPit() const
+pair<pit_type, ParagraphMetrics const *> TextMetrics::first() const
 {
-	return par_metrics_.begin()->first;
+	ParMetricsCache::const_iterator it = par_metrics_.begin();
+	return make_pair(it->first, &it->second);
 }
 
 
-pit_type TextMetrics::lastPit() const
+pair<pit_type, ParagraphMetrics const *> TextMetrics::last() const
 {
-	return par_metrics_.rbegin()->first;
+	LBUFERR(!par_metrics_.empty());
+	ParMetricsCache::const_reverse_iterator it = par_metrics_.rbegin();
+	return make_pair(it->first, &it->second);
 }
 
 
@@ -281,20 +284,6 @@ int TextMetrics::rightMargin(pit_type const pit) const
 }
 
 
-int TextMetrics::topPosition() const
-{
-	ParagraphMetrics const & firstpm = par_metrics_.begin()->second;
-	return firstpm.position() - firstpm.ascent();
-}
-
-
-int TextMetrics::bottomPosition() const
-{
-	ParagraphMetrics const & lastpm = par_metrics_.rbegin()->second;
-	return lastpm.position() + lastpm.descent();
-}
-
-
 void TextMetrics::applyOuterFont(Font & font) const
 {
 	FontInfo lf(font_.fontInfo());
@@ -572,14 +561,21 @@ bool TextMetrics::redoParagraph(pit_type const pit, bool const align_rows)
 	// specially tailored for the main text.
 	// Top and bottom margin of the document (only at top-level)
 	if (text_->isMainText()) {
+		// original value was 20px, which is 0.2in at 100dpi
+		int const margin = bv_->zoomedPixels(20);
 		if (pit == 0) {
-			pm.rows().front().top_padding += bv_->topMargin();
-			pm.dim().asc += bv_->topMargin();
+			pm.rows().front().dim().asc += margin;
+			/* coverity thinks that we should update pm.dim().asc
+			 * below, but all the rows heights are actually counted as
+			 * part of the paragraph metric descent see loop above).
+			 */
+			// coverity[copy_paste_error]
+			pm.dim().des += margin;
 		}
 		ParagraphList const & pars = text_->paragraphs();
 		if (pit + 1 == pit_type(pars.size())) {
-			pm.rows().back().bottom_padding += bv_->bottomMargin();
-			pm.dim().des += bv_->bottomMargin();
+			pm.rows().back().dim().des += margin;
+			pm.dim().des += margin;
 		}
 	}
 
diff --git a/src/TextMetrics.h b/src/TextMetrics.h
index b370b1e..006836f 100644
--- a/src/TextMetrics.h
+++ b/src/TextMetrics.h
@@ -45,10 +45,9 @@ public:
 	///
 	bool contains(pit_type pit) const;
 	///
-	pit_type firstPit() const;
+	std::pair<pit_type, ParagraphMetrics const *> first() const;
 	///
-	pit_type lastPit() const;
-
+	std::pair<pit_type, ParagraphMetrics const *> last() const;
 	/// is this row the last in the text?
 	bool isLastRow(Row const & row) const;
 	/// is this row the first in the text?
@@ -124,14 +123,8 @@ public:
 
 	///
 	int rightMargin(ParagraphMetrics const & pm) const;
-	///
 	int rightMargin(pit_type const pit) const;
 
-	/// position of the top of the first paragraph.
-	int topPosition() const;
-	/// position of the bottom of the last paragraph.
-	int bottomPosition() const;
-
 	///
 	void draw(PainterInfo & pi, int x, int y) const;
 
diff --git a/src/frontends/qt/GuiWorkArea.cpp b/src/frontends/qt/GuiWorkArea.cpp
index 725b7ab..23d6efa 100644
--- a/src/frontends/qt/GuiWorkArea.cpp
+++ b/src/frontends/qt/GuiWorkArea.cpp
@@ -1046,8 +1046,9 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 	if (tm.empty())
 		return;
 
-	pit_type const pit = up ? tm.firstPit() : tm.lastPit();
-	ParagraphMetrics const & pm = tm.parMetrics(pit);
+	pair<pit_type, const ParagraphMetrics *> pp = up ? tm.first() : tm.last();
+	ParagraphMetrics const & pm = *pp.second;
+	pit_type const pit = pp.first;
 
 	if (pm.rows().empty())
 		return;
diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index 0f2a557..5c5241e 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -4384,7 +4384,7 @@ void InsetTabular::metrics(MetricsInfo & mi, Dimension & dim) const
 
 			// with LYX_VALIGN_BOTTOM the descent is relative to the last par
 			// = descent of text in last par + bottomOffset:
-			int const lastpardes = tm.parMetrics(tm.lastPit()).descent()
+			int const lastpardes = tm.last().second->descent()
 				+ bottomOffset(mi.base.bv);
 			int offset = 0;
 			switch (tabular.getVAlignment(cell)) {


More information about the lyx-cvs mailing list