From 32f06d01ec96ca0d2144ae241f561ce8daa8f244 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Mon, 13 Jul 2020 00:00:36 +0200 Subject: [PATCH] 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 3c7940a049..2eaa43465b 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 first = tm.first(); + pair 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 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 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 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 firstpm = tm.first(); + pair 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 fbbd1e7f26..3cfd6239a3 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 0d92c94199..f59b6e41af 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 426f45edcc..d210174ef9 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 5b16d36ef5..9f55c5a920 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 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 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 b370b1e3a3..006836fb0a 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 first() const; /// - pit_type lastPit() const; - + std::pair 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 725b7abd13..23d6efa541 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 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 0f2a557a6b..5c5241e069 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)) {