From ff7cdf1b74f5c17a966af24dc70d49fc162f007e Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sun, 12 Jul 2020 20:11:27 +0200 Subject: [PATCH] Improve handling of top and bottom margin The 20px space on top and bottom of document have traditionally been obtained by adding the to the ascent/descent of the first/last row. This reads to annoyances like selections that are drawn in these margins and issues with the nesting marker. The change is to add the values to a separate member of the Row object, and to add new Row::total(Ascent|Descent) methods that add the effect of this padding. Moreover, some methods are added to TextMetrics to simplify the BufferView code. Fixes bug #9545. --- 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, 87 insertions(+), 62 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 2eaa43465b..3c7940a049 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -360,6 +360,20 @@ 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(); @@ -582,29 +596,25 @@ void BufferView::updateScrollbar() } // Look at paragraph heights on-screen - pair first = tm.first(); - pair last = tm.last(); - for (pit_type pit = first.first; pit <= last.first; ++pit) { + for (pit_type pit = tm.firstPit(); pit <= tm.lastPit(); ++pit) { d->par_height_[pit] = tm.parMetrics(pit).height(); LYXERR(Debug::SCROLLING, "storing height for pit " << pit << " : " << d->par_height_[pit]); } - 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_; + bool first_visible = tm.firstPit() == 0 && tm.topPosition() >= 0; + bool last_visible = tm.lastPit() + 1 == int(parsize) && tm.bottomPosition() <= height_; if (first_visible && last_visible) { d->scrollbarParameters_.min = 0; d->scrollbarParameters_.max = 0; return; } - d->scrollbarParameters_.min = top_pos; - for (size_t i = 0; i != size_t(first.first); ++i) + d->scrollbarParameters_.min = tm.topPosition(); + for (size_t i = 0; i != size_t(tm.firstPit()); ++i) d->scrollbarParameters_.min -= d->par_height_[i]; - d->scrollbarParameters_.max = bottom_pos; - for (size_t i = last.first + 1; i != parsize; ++i) + d->scrollbarParameters_.max = tm.bottomPosition(); + for (size_t i = tm.lastPit() + 1; i != parsize; ++i) d->scrollbarParameters_.max += d->par_height_[i]; // The reference is the top position so we remove one page. @@ -941,9 +951,9 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter) bot_pit = max_pit; } - if (bot_pit == tm.first().first - 1) + if (bot_pit == tm.firstPit() - 1) tm.newParMetricsUp(); - else if (bot_pit == tm.last().first + 1) + else if (bot_pit == tm.lastPit() + 1) tm.newParMetricsDown(); if (tm.contains(bot_pit)) { @@ -953,8 +963,7 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter) CursorSlice const & cs = dit.innerTextSlice(); int offset = coordOffset(dit).y_; int ypos = pm.position() + offset; - Dimension const & row_dim = - pm.getRow(cs.pos(), dit.boundary()).dim(); + Row const & row = pm.getRow(cs.pos(), dit.boundary()); int scrolled = 0; if (recenter) scrolled = scroll(ypos - height_/2); @@ -963,7 +972,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_dim.height() > height_) { + else if (row.height() > height_) { if (ypos < defaultRowHeight()) scrolled = scroll(ypos - height_ / 4); else if (ypos > height_ - defaultRowHeight()) @@ -972,14 +981,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_dim.ascent() < 0 && ypos < height_) { - int ynew = row_dim.ascent(); + else if (ypos - row.totalAscent() < 0 && ypos < height_) { + int ynew = row.totalAscent(); scrolled = scrollUp(ynew - ypos); } // If the bottom of the row falls of the screen, we scroll down. - else if (ypos + row_dim.descent() > height_ && ypos > 0) { - int ynew = height_ - row_dim.descent(); + else if (ypos + row.totalDescent() > height_ && ypos > 0) { + int ynew = height_ - row.totalDescent(); scrolled = scrollDown(ypos - ynew); } @@ -997,15 +1006,14 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool const recenter) d->anchor_pit_ = bot_pit; CursorSlice const & cs = dit.innerTextSlice(); - Dimension const & row_dim = - pm.getRow(cs.pos(), dit.boundary()).dim(); + Row const & row = pm.getRow(cs.pos(), dit.boundary()); 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_dim.descent(); + d->anchor_ypos_ = height_ - offset - row.totalDescent(); else if (offset > height_) d->anchor_ypos_ = height_ - offset - defaultRowHeight(); else @@ -2441,11 +2449,10 @@ int BufferView::scrollDown(int offset) TextMetrics & tm = d->text_metrics_[text]; int const ymax = height_ + offset; while (true) { - pair last = tm.last(); - int bottom_pos = last.second->position() + last.second->descent(); + int bottom_pos = tm.bottomPosition(); if (lyxrc.scroll_below_document) bottom_pos += height_ - minVisiblePart(); - if (last.first + 1 == int(text->paragraphs().size())) { + if (tm.lastPit() + 1 == int(text->paragraphs().size())) { if (bottom_pos <= height_) return 0; offset = min(offset, bottom_pos - height_); @@ -2466,9 +2473,8 @@ int BufferView::scrollUp(int offset) TextMetrics & tm = d->text_metrics_[text]; int ymin = - offset; while (true) { - pair first = tm.first(); - int top_pos = first.second->position() - first.second->ascent(); - if (first.first == 0) { + int const top_pos = tm.topPosition(); + if (tm.firstPit() == 0) { if (top_pos >= 0) return 0; offset = min(offset, - top_pos); @@ -2983,7 +2989,7 @@ Point BufferView::coordOffset(DocIterator const & dit) const ParagraphMetrics const & pm = tm.parMetrics(sl.pit()); LBUFERR(!pm.rows().empty()); - y -= pm.rows()[0].ascent(); + y -= pm.rows()[0].totalAscent(); #if 1 // FIXME: document this mess size_t rend; @@ -3000,7 +3006,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].ascent(); + y += pm.rows()[rend].totalAscent(); TextMetrics const & bottom_tm = textMetrics(dit.bottom().text()); @@ -3189,7 +3195,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.first().second->position(); + int const y = tm.parMetrics(tm.firstPit()).position(); PainterInfo pi(this, pain); // Check whether the row where the cursor lives needs to be scrolled. @@ -3244,8 +3250,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) tm.draw(pi, 0, y); // and possibly grey out below - pair lastpm = tm.last(); - int const y2 = lastpm.second->position() + lastpm.second->descent(); + int const y2 = tm.bottomPosition(); if (y2 < height_) { Color color = buffer().isInternal() @@ -3261,9 +3266,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) updateScrollbar(); // Normalize anchor for next time - pair firstpm = tm.first(); - pair lastpm = tm.last(); - for (pit_type pit = firstpm.first; pit <= lastpm.first; ++pit) { + for (pit_type pit = tm.firstPit(); pit <= tm.lastPit(); ++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 3cfd6239a3..fbbd1e7f26 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -104,9 +104,12 @@ 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 f59b6e41af..0d92c94199 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -161,6 +161,7 @@ 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 d210174ef9..426f45edcc 100644 --- a/src/Row.h +++ b/src/Row.h @@ -209,6 +209,10 @@ 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; @@ -303,6 +307,10 @@ 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 9f55c5a920..5b16d36ef5 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -127,18 +127,15 @@ bool TextMetrics::contains(pit_type pit) const } -pair TextMetrics::first() const +pit_type TextMetrics::firstPit() const { - ParMetricsCache::const_iterator it = par_metrics_.begin(); - return make_pair(it->first, &it->second); + return par_metrics_.begin()->first; } -pair TextMetrics::last() const +pit_type TextMetrics::lastPit() const { - LBUFERR(!par_metrics_.empty()); - ParMetricsCache::const_reverse_iterator it = par_metrics_.rbegin(); - return make_pair(it->first, &it->second); + return par_metrics_.rbegin()->first; } @@ -284,6 +281,20 @@ 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()); @@ -561,21 +572,14 @@ 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().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; + pm.rows().front().top_padding += bv_->topMargin(); + pm.dim().asc += bv_->topMargin(); } ParagraphList const & pars = text_->paragraphs(); if (pit + 1 == pit_type(pars.size())) { - pm.rows().back().dim().des += margin; - pm.dim().des += margin; + pm.rows().back().bottom_padding += bv_->bottomMargin(); + pm.dim().des += bv_->bottomMargin(); } } diff --git a/src/TextMetrics.h b/src/TextMetrics.h index 006836fb0a..b370b1e3a3 100644 --- a/src/TextMetrics.h +++ b/src/TextMetrics.h @@ -45,9 +45,10 @@ public: /// bool contains(pit_type pit) const; /// - std::pair first() const; + pit_type firstPit() const; /// - std::pair last() const; + pit_type lastPit() const; + /// is this row the last in the text? bool isLastRow(Row const & row) const; /// is this row the first in the text? @@ -123,8 +124,14 @@ 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 23d6efa541..725b7abd13 100644 --- a/src/frontends/qt/GuiWorkArea.cpp +++ b/src/frontends/qt/GuiWorkArea.cpp @@ -1046,9 +1046,8 @@ void GuiWorkArea::generateSyntheticMouseEvent() if (tm.empty()) return; - pair pp = up ? tm.first() : tm.last(); - ParagraphMetrics const & pm = *pp.second; - pit_type const pit = pp.first; + pit_type const pit = up ? tm.firstPit() : tm.lastPit(); + ParagraphMetrics const & pm = tm.parMetrics(pit); if (pm.rows().empty()) return; diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp index 5c5241e069..0f2a557a6b 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.last().second->descent() + int const lastpardes = tm.parMetrics(tm.lastPit()).descent() + bottomOffset(mi.base.bv); int offset = 0; switch (tabular.getVAlignment(cell)) {