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.

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 2eaa43465b5ebc1b53ff303cbf800000efaaa2ba..3c7940a049d9065eb1bf9a2f6fd28c71f2700250 100644 (file)
--- 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<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) {
+       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<pit_type, ParagraphMetrics const *> 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<pit_type, ParagraphMetrics const *> 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<pit_type, ParagraphMetrics const *> 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<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) {
+       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 3cfd6239a3f1751899800fff6021ead6cc91a446..fbbd1e7f2645440eac78fe2158ea3c74ae9b762c 100644 (file)
--- 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 f59b6e41afdd358485ef9ebfc414efb9482a6245..0d92c94199642d1e5e58036b6fa12bf9a50eb9a0 100644 (file)
--- 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 d210174ef9460b269433dc9ac1c88fc2d7cc318e..426f45edcc10cba3d377f1271fbdcb6009bbaa72 100644 (file)
--- 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 9f55c5a920607b92bbb77f1e65c4331510e4deb4..5b16d36ef51ad279e90c3ddba405162085036c1d 100644 (file)
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -127,18 +127,15 @@ bool TextMetrics::contains(pit_type pit) const
 }
 
 
-pair<pit_type, ParagraphMetrics const *> 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<pit_type, ParagraphMetrics const *> 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 006836fb0a186b0f88795de6a1095a942aaaaf20..b370b1e3a3b0e950af00630c9aeee1a96442c850 100644 (file)
--- a/src/TextMetrics.h
+++ b/src/TextMetrics.h
@@ -45,9 +45,10 @@ public:
        ///
        bool contains(pit_type pit) const;
        ///
-       std::pair<pit_type, ParagraphMetrics const *> first() const;
+       pit_type firstPit() const;
        ///
-       std::pair<pit_type, ParagraphMetrics const *> 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 23d6efa54100d1a0aa2360309c81514c67f17ac8..725b7abd1370dcc32cc3e5fb1d1747a75a2506c0 100644 (file)
--- a/src/frontends/qt/GuiWorkArea.cpp
+++ b/src/frontends/qt/GuiWorkArea.cpp
@@ -1046,9 +1046,8 @@ void GuiWorkArea::generateSyntheticMouseEvent()
        if (tm.empty())
                return;
 
-       pair<pit_type, const ParagraphMetrics *> 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 5c5241e069ba3c9bc1591672a39de31a1111ae7c..0f2a557a6b791f2ae450033fdacec9cb9eed376f 100644 (file)
--- 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)) {