From a71610b9d72d346ac694518d6dab634f02247e2b Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 11 Nov 2017 12:40:39 +0100 Subject: [PATCH] Remove row crc computation This computation did not make sense anymore since we began to put the contents in the Row object. The fact that it worked was a coincidence. Instead, we set rows as changed() on creation and reset that once they have been drawn. This will allow in the future for a finer definition of what to redraw or not. Also update the PAINTING_ANALYSIS document (cherry picked from commit 9e2da4a3eac83f46ab184ea8d674f84643814017) --- development/PAINTING_ANALYSIS | 36 +++++++++++++++++++++++++++++++++ src/ParagraphMetrics.cpp | 38 ----------------------------------- src/ParagraphMetrics.h | 3 --- src/Row.cpp | 9 +-------- src/Row.h | 6 +----- src/TextMetrics.cpp | 8 +++++--- 6 files changed, 43 insertions(+), 57 deletions(-) diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS index ec3566e06c..32bc93a5ff 100644 --- a/development/PAINTING_ANALYSIS +++ b/development/PAINTING_ANALYSIS @@ -60,6 +60,42 @@ cursor. * Clean-up of drawing code +** Make SinglePar update flag useful again. + +The current code can be very expensive when moving cursor inside a +huge table, for example. We should test the flag again, although this +will probably lead to some glitches here and there. + +** Set Row::changed() in a finer way + +*** singleParUpdate + +When the height of the current paragraph changes, there is no need for +a full screen update. Only the rows after the current one need to have +their position recomputed. + +This is also true when scrolling (how to do that?) + +*** redoParagraph + +It should be possible to check whether the new row is the same as the +old one and keep its changed() status in this case. This would reduce +a lot the amount of stuff to redraw. + +** Put labels and friends in the Row as elements + +It should not be necessary to access the Paragraph object to draw. +Adding the static elements to Row is a lot of work, but worth it IMO. + +** Create a unique row by paragraph and break it afterwards + +This should be a performance gain (only if paragraph breaking still +shows as expensive after the rest is done) + +** do not add the vertical margin of main text to first/last row + +Would make code cleaner. Probably no so difficult. + ** When a paragraph ends with a newline, compute correctly the height of the extra row. ** Merging bv::updateMetrics and tm::metrics diff --git a/src/ParagraphMetrics.cpp b/src/ParagraphMetrics.cpp index 01db6cb934..1d3ce0db5c 100644 --- a/src/ParagraphMetrics.cpp +++ b/src/ParagraphMetrics.cpp @@ -47,8 +47,6 @@ #include "support/lstrings.h" #include "support/textutils.h" -#include - #include #include #include @@ -84,42 +82,6 @@ void ParagraphMetrics::reset(Paragraph const & par) } -size_t ParagraphMetrics::computeRowSignature(Row const & row, - BufferView const & bv) const -{ - boost::crc_32_type crc; - for (pos_type i = row.pos(); i < row.endpos(); ++i) { - if (par_->isInset(i)) { - Inset const * in = par_->getInset(i); - Dimension const d = in->dimension(bv); - int const b[] = { d.wid, d.asc, d.des }; - crc.process_bytes(b, sizeof(b)); - } else { - char_type const b[] = { par_->getChar(i) }; - crc.process_bytes(b, sizeof(char_type)); - } - if (bv.buffer().params().track_changes) { - Change change = par_->lookupChange(i); - char_type const b[] = { static_cast(change.type) }; - // 1 byte is enough to encode Change::Type - crc.process_bytes(b, 1); - } - } - - pos_type const b1[] = { row.sel_beg, row.sel_end }; - crc.process_bytes(b1, sizeof(b1)); - - Dimension const & d = row.dimension(); - int const b2[] = { row.begin_margin_sel, - row.end_margin_sel, - d.wid, d.asc, d.des }; - crc.process_bytes(b2, sizeof(b2)); - crc.process_bytes(&row.separator, sizeof(row.separator)); - - return crc.checksum(); -} - - void ParagraphMetrics::setPosition(int position) { position_ = position; diff --git a/src/ParagraphMetrics.h b/src/ParagraphMetrics.h index 55304fd9ed..63ed0f3cdf 100644 --- a/src/ParagraphMetrics.h +++ b/src/ParagraphMetrics.h @@ -85,9 +85,6 @@ public: /// bool hfillExpansion(Row const & row, pos_type pos) const; - /// - size_t computeRowSignature(Row const &, BufferView const & bv) const; - /// int position() const { return position_; } void setPosition(int position); diff --git a/src/Row.cpp b/src/Row.cpp index 20ff63e22a..ad492c1a7a 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -162,20 +162,13 @@ Row::Row() : separator(0), label_hfill(0), left_margin(0), right_margin(0), sel_beg(-1), sel_end(-1), begin_margin_sel(false), end_margin_sel(false), - changed_(false), crc_(0), + changed_(true), pit_(0), pos_(0), end_(0), right_boundary_(false), flushed_(false), rtl_(false), changebar_(false) {} -void Row::setCrc(size_type crc) const -{ - changed_ = crc != crc_; - crc_ = crc; -} - - bool Row::isMarginSelected(bool left_margin, DocIterator const & beg, DocIterator const & end) const { diff --git a/src/Row.h b/src/Row.h index a2e77fbfb4..a1fedd7e5d 100644 --- a/src/Row.h +++ b/src/Row.h @@ -139,9 +139,7 @@ public: /// bool changed() const { return changed_; } /// - void setChanged(bool c) { changed_ = c; } - /// - void setCrc(size_type crc) const; + void changed(bool c) const { changed_ = c; } /// Set the selection begin and end. /** * This is const because we update the selection status only at draw() @@ -315,8 +313,6 @@ private: /// has the Row appearance changed since last drawing? mutable bool changed_; - /// CRC of row contents. - mutable size_type crc_; /// Index of the paragraph that contains this row pit_type pit_; /// first pos covered by this row diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index cd33f7f190..1f4516f1af 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -468,7 +468,7 @@ bool TextMetrics::redoParagraph(pit_type const pit) row.pit(pit); need_new_row = breakRow(row, right_margin); setRowHeight(row); - row.setChanged(false); + row.changed(true); if (row_index || row.endpos() < par.size() || (row.right_boundary() && par.inInset().lyxCode() != CELL_CODE)) { /* If there is more than one row or the row has been @@ -1888,8 +1888,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const row.end_margin_sel = sel_end.pit() > pit; } - // Row signature; has row changed since last paint? - row.setCrc(pm.computeRowSignature(row, *bv_)); + // has row changed since last paint? bool row_has_changed = row.changed() || bv_->hadHorizScrollOffset(text_, pit, row.pos()) || bv_->needRepaint(text_, row); @@ -1907,6 +1906,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // Paint only the insets if the text itself is // unchanged. rp.paintOnlyInsets(); + row.changed(false); y += row.descent(); continue; } @@ -1958,6 +1958,8 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // Restore full_repaint status. pi.full_repaint = tmp; + + row.changed(false); } //LYXERR(Debug::PAINTING, "."); -- 2.39.5