From d222e0f48e17eb838d8320f30f82b4c96b3e496e Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sun, 16 Jul 2017 01:25:03 +0200 Subject: [PATCH 01/24] three-stage drawing: add a nodraw stage Normally the two stages of drawing are 1/ compute metrics of insets/rows/paragraphs/mathrow... 2/ draw the elements and cache their positions Now the three stages are 1/ metrics 2/ nodraw: do not draw the elements, but cache their position 3/ draw the elements (and store again their position; it does not seems to hurt performance). Revive the NullPainter: this replaces the setDrawingEnabled mechanism with a painter that does nothing. The advantage is that updatePosCache (renamed from setPosCache) does not need anymore to be invoked from the frontend. updatePosCache (the nodraw stage) is called at the end of BufferView::updateMetrics. --- src/BufferView.cpp | 27 +++++-- src/BufferView.h | 5 +- src/RowPainter.cpp | 2 +- src/TextMetrics.cpp | 18 ++--- src/frontends/Makefile.am | 1 + src/frontends/NullPainter.h | 109 ++++++++++++++++++++++++++ src/frontends/Painter.h | 11 +-- src/frontends/qt4/GuiPainter.cpp | 26 +----- src/frontends/qt4/GuiPainter.h | 3 + src/frontends/qt4/GuiWorkArea.cpp | 14 ++-- src/mathed/InsetMathMacroTemplate.cpp | 18 ++--- 11 files changed, 160 insertions(+), 74 deletions(-) create mode 100644 src/frontends/NullPainter.h diff --git a/src/BufferView.cpp b/src/BufferView.cpp index e141f34c7e..94e448d1e9 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -70,6 +70,7 @@ #include "frontends/Application.h" #include "frontends/Delegates.h" #include "frontends/FontMetrics.h" +#include "frontends/NullPainter.h" #include "frontends/Painter.h" #include "frontends/Selection.h" @@ -2732,6 +2733,9 @@ void BufferView::updateMetrics() d->update_strategy_ = FullScreenUpdate; + // Now update the positions of insets in the cache. + updatePosCache(); + if (lyxerr.debugging(Debug::WORKAREA)) { LYXERR(Debug::WORKAREA, "BufferView::updateMetrics"); d->coord_cache_.dump(); @@ -2739,6 +2743,15 @@ void BufferView::updateMetrics() } +void BufferView::updatePosCache() +{ + // this is the "nodraw" drawing stage: only set the positions of the + // insets in metrics cache. + frontend::NullPainter np; + draw(np); +} + + void BufferView::insertLyXFile(FileName const & fname) { LASSERT(d->cursor_.inTexted(), return); @@ -2984,12 +2997,11 @@ void BufferView::checkCursorScrollOffset(PainterInfo & pi) * at this point. */ // Force the recomputation of inset positions - bool const drawing = pi.pain.isDrawingEnabled(); - pi.pain.setDrawingEnabled(false); + frontend::NullPainter np; + PainterInfo(this, np); // No need to care about vertical position. RowPainter rp(pi, buffer().text(), row, -d->horiz_scroll_offset_, 0); rp.paintText(); - pi.pain.setDrawingEnabled(drawing); } // Current x position of the cursor in pixels @@ -3053,12 +3065,13 @@ void BufferView::draw(frontend::Painter & pain) switch (d->update_strategy_) { case NoScreenUpdate: - // If no screen painting is actually needed, only some the different - // coordinates of insets and paragraphs needs to be updated. + // no screen painting is actually needed. In nodraw stage + // however, the different coordinates of insets and paragraphs + // needs to be updated. LYXERR(Debug::PAINTING, "Strategy: NoScreenUpdate"); pi.full_repaint = true; - pi.pain.setDrawingEnabled(false); - tm.draw(pi, 0, y); + if (pain.isNull()) + tm.draw(pi, 0, y); break; case SingleParUpdate: diff --git a/src/BufferView.h b/src/BufferView.h index 588e9d0665..40af7c63b2 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -283,6 +283,10 @@ public: /// update the internal \c ViewMetricsInfo. void updateMetrics(); + // this is the "nodraw" drawing stage: only set the positions of the + // insets in metrics cache. + void updatePosCache(); + /// TextMetrics const & textMetrics(Text const * t) const; TextMetrics & textMetrics(Text const * t); @@ -303,7 +307,6 @@ public: /// get the position and height of the cursor void cursorPosAndHeight(Point & p, int & h) const; - /// void draw(frontend::Painter & pain); diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp index 1f89b252b9..c4b9034fb4 100644 --- a/src/RowPainter.cpp +++ b/src/RowPainter.cpp @@ -576,7 +576,7 @@ void RowPainter::paintText() paintStringAndSel(e); // Paint the spelling marks if enabled. - if (lyxrc.spellcheck_continuously && pi_.do_spellcheck && pi_.pain.isDrawingEnabled()) + if (lyxrc.spellcheck_continuously && pi_.do_spellcheck && !pi_.pain.isNull()) paintMisspelledMark(e); break; diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 4768157bda..360d83b296 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -1797,8 +1797,8 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const return; size_t const nrows = pm.rows().size(); - // Use fast lane when drawing is disabled. - if (!pi.pain.isDrawingEnabled()) { + // Use fast lane in nodraw stage. + if (pi.pain.isNull()) { for (size_t i = 0; i != nrows; ++i) { Row const & row = pm.rows()[i]; @@ -1850,17 +1850,11 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const if (i) y += row.ascent(); - RowPainter rp(pi, *text_, row, row_x, y); - // It is not needed to draw on screen if we are not inside. bool const inside = (y + row.descent() >= 0 && y - row.ascent() < ww); - pi.pain.setDrawingEnabled(inside); if (!inside) { - // Paint only the insets to set inset cache correctly - // FIXME: remove paintOnlyInsets when we know that positions - // have already been set. - rp.paintOnlyInsets(); + // Inset positions have already been set in nodraw stage. y += row.descent(); continue; } @@ -1889,6 +1883,8 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const text_->getPar(pit).spellCheck(); } + RowPainter rp(pi, *text_, row, row_x, y); + // Don't paint the row if a full repaint has not been requested // and if it has not changed. if (!pi.full_repaint && !row_has_changed) { @@ -1919,7 +1915,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const << " row_selection=" << row.selection() << " full_repaint=" << pi.full_repaint << " row_has_changed=" << row_has_changed - << " drawingEnabled=" << pi.pain.isDrawingEnabled()); + << " null painter=" << pi.pain.isNull()); } // Backup full_repaint status and force full repaint @@ -1947,8 +1943,6 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // Restore full_repaint status. pi.full_repaint = tmp; } - // Re-enable screen drawing for future use of the painter. - pi.pain.setDrawingEnabled(true); //LYXERR(Debug::PAINTING, "."); } diff --git a/src/frontends/Makefile.am b/src/frontends/Makefile.am index 862613a67f..22b7508a7a 100644 --- a/src/frontends/Makefile.am +++ b/src/frontends/Makefile.am @@ -17,6 +17,7 @@ liblyxfrontends_a_SOURCES = \ Delegates.h \ KeyModifier.h \ KeySymbol.h \ + NullPainter.h \ Painter.h \ Clipboard.h \ Selection.h \ diff --git a/src/frontends/NullPainter.h b/src/frontends/NullPainter.h new file mode 100644 index 0000000000..19ac8b66a2 --- /dev/null +++ b/src/frontends/NullPainter.h @@ -0,0 +1,109 @@ +// -*- C++ -*- +/** + * \file NullPainter.h + * This file is part of LyX, the document processor. + * Licence details can be found in the file COPYING. + * + * \author unknown + * \author John Levon + * \author Jean-Marc Lasgouttes + * + * Full author contact details are available in file CREDITS. + */ + +#ifndef NULLPAINTER_H +#define NULLPAINTER_H + +#include "Painter.h" + +namespace lyx { + +namespace frontend { + +/** + * NullPainter - A painter instance that does nothing + */ +class NullPainter : public Painter { +public: + NullPainter() : Painter(1) {} + + ~NullPainter() {} + + /// draw a line from point to point + void line(int, int, int, int, Color, + line_style = line_solid, int = thin_line) {} + + /// + void lines(int const *, int const *, int, Color, + fill_style = fill_none, line_style = line_solid, + int = thin_line) {} + + /// + void path(int const *, int const *, int const *, int const *, + int const *, int const *, int, Color, + fill_style = fill_none, line_style = line_solid, int = thin_line) {} + + /// draw a rectangle + void rectangle(int, int, int, int, Color, + line_style = line_solid, int = thin_line) {} + + /// draw a filled rectangle + void fillRectangle(int, int, int, int, Color) {} + + /// draw an arc + void arc(int, int, unsigned int, unsigned int, int, int, Color) {} + + /// draw a pixel + void point(int, int, Color) {} + + /// draw an image from the image cache + void image(int, int, int, int, graphics::Image const &) {} + + /// draw a string + void text(int, int, docstring const &, FontInfo const &) {} + + /// draw a char + void text(int, int, char_type, FontInfo const &) {} + + /// draw a string + void text(int, int, docstring const &, Font const &, double, double) {} + + /// + void text(int, int, docstring const &, Font const &, + Color, size_type, size_type, double, double) {} + + /// This painter does not paint + bool isNull() const { return true; } + + /// draw the underbar, strikeout, xout, uuline and uwave font attributes + void textDecoration(FontInfo const &, int, int, int) {} + + /** + * Draw a string and enclose it inside a rectangle. If + * back color is specified, the background is cleared with + * the given color. If frame is specified, a thin frame is drawn + * around the text with the given color. + */ + void rectText(int, int, docstring const &, + FontInfo const &, Color, Color) {} + + /// draw a string and enclose it inside a button frame + void buttonText(int, int, docstring const &, + FontInfo const &, Color, Color, int) {} + + /// draw a character of a preedit string for cjk support. + int preeditText(int, int, char_type, FontInfo const &, + preedit_style) { return 0; } + + /// start monochrome painting mode, i.e. map every color into [min,max] + void enterMonochromeMode(Color const &, Color const &) {} + /// leave monochrome painting mode + void leaveMonochromeMode() {} + /// draws a wavy line that can be used for underlining. + void wavyHorizontalLine(int, int, int, ColorCode) {} +}; + +} // namespace frontend +} // namespace lyx + +#endif // NULLPAINTER_H diff --git a/src/frontends/Painter.h b/src/frontends/Painter.h index d03ebf40e8..17253b2f44 100644 --- a/src/frontends/Painter.h +++ b/src/frontends/Painter.h @@ -49,7 +49,7 @@ namespace frontend { */ class Painter { public: - Painter(double pixel_ratio) : drawing_enabled_(true), pixel_ratio_(pixel_ratio) {} + Painter(double pixel_ratio) : pixel_ratio_(pixel_ratio) {} static const int thin_line; @@ -147,11 +147,8 @@ public: Color other, size_type from, size_type to, double wordspacing, double textwidth) = 0; - void setDrawingEnabled(bool drawing_enabled) - { drawing_enabled_ = drawing_enabled; } - - /// Indicate wether real screen drawing shall be done or not. - bool isDrawingEnabled() const { return drawing_enabled_; } + // Returns true if the painter does not actually paint. + virtual bool isNull() const = 0; double pixelRatio() const { return pixel_ratio_; } @@ -183,8 +180,6 @@ public: /// draws a wavy line that can be used for underlining. virtual void wavyHorizontalLine(int x, int y, int width, ColorCode col) = 0; private: - /// - bool drawing_enabled_; /// Ratio between physical pixels and device-independent pixels double pixel_ratio_; }; diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp index e832f23e99..bff8f9dc30 100644 --- a/src/frontends/qt4/GuiPainter.cpp +++ b/src/frontends/qt4/GuiPainter.cpp @@ -171,9 +171,6 @@ void GuiPainter::leaveMonochromeMode() void GuiPainter::point(int x, int y, Color col) { - if (!isDrawingEnabled()) - return; - setQPainterPen(computeColor(col)); drawPoint(x, y); } @@ -184,9 +181,6 @@ void GuiPainter::line(int x1, int y1, int x2, int y2, line_style ls, int lw) { - if (!isDrawingEnabled()) - return; - setQPainterPen(computeColor(col), ls, lw); bool const do_antialiasing = renderHints() & TextAntialiasing && x1 != x2 && y1 != y2 && ls != line_solid_aliased; @@ -202,9 +196,6 @@ void GuiPainter::lines(int const * xp, int const * yp, int np, line_style ls, int lw) { - if (!isDrawingEnabled()) - return; - // double the size if needed // FIXME THREAD static QVector points(32); @@ -247,9 +238,6 @@ void GuiPainter::path(int const * xp, int const * yp, line_style ls, int lw) { - if (!isDrawingEnabled()) - return; - QPainterPath bpath; // This is the starting point, so its control points are meaningless bpath.moveTo(xp[0], yp[0]); @@ -278,9 +266,6 @@ void GuiPainter::rectangle(int x, int y, int w, int h, line_style ls, int lw) { - if (!isDrawingEnabled()) - return; - setQPainterPen(computeColor(col), ls, lw); drawRect(x, y, w, h); } @@ -288,9 +273,6 @@ void GuiPainter::rectangle(int x, int y, int w, int h, void GuiPainter::fillRectangle(int x, int y, int w, int h, Color col) { - if (!isDrawingEnabled()) - return; - fillRect(x, y, w, h, guiApp->colorCache().get(col)); } @@ -298,9 +280,6 @@ void GuiPainter::fillRectangle(int x, int y, int w, int h, Color col) void GuiPainter::arc(int x, int y, unsigned int w, unsigned int h, int a1, int a2, Color col) { - if (!isDrawingEnabled()) - return; - // LyX usings 1/64ths degree, Qt usings 1/16th setQPainterPen(computeColor(col)); bool const do_antialiasing = renderHints() & TextAntialiasing; @@ -317,9 +296,6 @@ void GuiPainter::image(int x, int y, int w, int h, graphics::Image const & i) fillRectangle(x, y, w, h, Color_graphicsbg); - if (!isDrawingEnabled()) - return; - QImage const image = qlimage.image(); QRectF const drect = QRectF(x, y, w, h); QRectF const srect = QRectF(0, 0, image.width(), image.height()); @@ -391,7 +367,7 @@ void GuiPainter::text(int x, int y, docstring const & s, double const wordspacing, double const tw) { //LYXERR0("text: x=" << x << ", s=" << s); - if (s.empty() || !isDrawingEnabled()) + if (s.empty()) return; /* Caution: The following ucs4 to QString conversions work for symbol fonts diff --git a/src/frontends/qt4/GuiPainter.h b/src/frontends/qt4/GuiPainter.h index 9657ed9770..7513965ea6 100644 --- a/src/frontends/qt4/GuiPainter.h +++ b/src/frontends/qt4/GuiPainter.h @@ -37,6 +37,9 @@ public: GuiPainter(QPaintDevice *, double pixel_ratio); virtual ~GuiPainter(); + /// This painter paints + virtual bool isNull() const { return false; } + /// draw a line from point to point virtual void line( int x1, int y1, diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 8bf5242b33..ea6423ac85 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -16,6 +16,11 @@ #include "ColorCache.h" #include "FontLoader.h" +#include "GuiApplication.h" +#include "GuiCompleter.h" +#include "GuiKeySymbol.h" +#include "GuiPainter.h" +#include "GuiView.h" #include "Menus.h" #include "Buffer.h" @@ -26,11 +31,6 @@ #include "Cursor.h" #include "Font.h" #include "FuncRequest.h" -#include "GuiApplication.h" -#include "GuiCompleter.h" -#include "GuiKeySymbol.h" -#include "GuiPainter.h" -#include "GuiView.h" #include "KeySymbol.h" #include "Language.h" #include "LyX.h" @@ -1289,9 +1289,8 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) return; } - GuiPainter pain(d->screen_, pixelRatio()); d->buffer_view_->updateMetrics(); - d->buffer_view_->draw(pain); + d->updateScreen(); // FIXME: shall we use real_current_font here? (see #10478) FontInfo font = d->buffer_view_->cursor().getFont().fontInfo(); FontMetrics const & fm = theFontMetrics(font); @@ -1383,6 +1382,7 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) ps = Painter::preedit_cursor; // draw one character and update cur_x. + GuiPainter pain(d->screen_, pixelRatio()); cur_x += pain.preeditText(cur_x, cur_y, typed_char, font, ps); } diff --git a/src/mathed/InsetMathMacroTemplate.cpp b/src/mathed/InsetMathMacroTemplate.cpp index 17fc2d04a2..a2fc064c00 100644 --- a/src/mathed/InsetMathMacroTemplate.cpp +++ b/src/mathed/InsetMathMacroTemplate.cpp @@ -231,25 +231,17 @@ void InsetDisplayLabelBox::metrics(MetricsInfo & mi, Dimension & dim) const { InsetLabelBox::metrics(mi, dim); if (!parent_.editing(mi.base.bv) - && parent_.cell(parent_.displayIdx()).empty()) { - dim.wid = 0; - dim.asc = 0; - dim.des = 0; - } + && parent_.cell(parent_.displayIdx()).empty()) + dim.clear(); } void InsetDisplayLabelBox::draw(PainterInfo & pi, int x, int y) const { if (parent_.editing(pi.base.bv) - || !parent_.cell(parent_.displayIdx()).empty()) { - InsetLabelBox::draw(pi, x, y); - } else { - bool enabled = pi.pain.isDrawingEnabled(); - pi.pain.setDrawingEnabled(false); - InsetLabelBox::draw(pi, x, y); - pi.pain.setDrawingEnabled(enabled); - } + || !parent_.cell(parent_.displayIdx()).empty() + || pi.pain.isNull()) + InsetLabelBox::draw(pi, x, y); } From 7b8b3e38a1b9ff4d733266a4f3fe0f8fb041da01 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 8 Sep 2017 16:55:11 +0200 Subject: [PATCH 02/24] Do the actual drawing in the paint event Historically, because of two-stage drawing, LyX has been painting on a Pixmap, and this pixmap is copied to screen at paint event time. Now that we have three-stage drawing, it is possible to delay the painting to actual paint event and avoid the intermediate Pixmap. Known bug: the cursor is never erased. --- src/frontends/qt4/GuiWorkArea.cpp | 55 ++++++++----------------- src/frontends/qt4/GuiWorkArea_Private.h | 29 ------------- 2 files changed, 17 insertions(+), 67 deletions(-) diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index ea6423ac85..8ce598583e 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -246,7 +246,7 @@ SyntheticMouseEvent::SyntheticMouseEvent() GuiWorkArea::Private::Private(GuiWorkArea * parent) -: p(parent), screen_(0), buffer_view_(0), lyx_view_(0), +: p(parent), buffer_view_(0), lyx_view_(0), cursor_visible_(false), cursor_(0), need_resize_(false), schedule_redraw_(false), preedit_lines_(1), pixel_ratio_(1.0), @@ -299,7 +299,6 @@ void GuiWorkArea::init() d->cursor_timeout_.setInterval(500); } - d->resetScreen(); // With Qt4.5 a mouse event will happen before the first paint event // so make sure that the buffer view has an up to date metrics. d->buffer_view_->resize(viewport()->width(), viewport()->height()); @@ -313,10 +312,11 @@ void GuiWorkArea::init() setFrameStyle(QFrame::NoFrame); updateWindowTitle(); - viewport()->setAutoFillBackground(false); + //viewport()->setAutoFillBackground(false); // We don't need double-buffering nor SystemBackground on // the viewport because we have our own backing pixmap. - viewport()->setAttribute(Qt::WA_NoSystemBackground); + //viewport()->setAttribute(Qt::WA_NoSystemBackground); + viewport()->setAttribute(Qt::WA_OpaquePaintEvent); setFocusPolicy(Qt::StrongFocus); @@ -344,7 +344,6 @@ GuiWorkArea::~GuiWorkArea() try { d->buffer_view_->buffer().workAreaManager().remove(this); } catch(...) {} - delete d->screen_; delete d->buffer_view_; delete d->cursor_; // Completer has a QObject parent and is thus automatically destroyed. @@ -491,8 +490,7 @@ void GuiWorkArea::redraw(bool update_metrics) } LYXERR(Debug::WORKAREA, "WorkArea::redraw screen"); - d->updateScreen(); - update(0, 0, viewport()->width(), viewport()->height()); + viewport()->update(); /// \warning: scrollbar updating *must* be done after the BufferView is drawn /// because \c BufferView::updateScrollbar() is called in \c BufferView::draw(). @@ -591,7 +589,7 @@ void GuiWorkArea::Private::resizeBufferView() buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); if (cursor_in_view) buffer_view_->scrollToCursor(); - updateScreen(); + p->viewport()->update(); // Update scrollbars which might have changed due different // BufferView dimension. This is especially important when the @@ -1176,20 +1174,12 @@ void GuiWorkArea::resizeEvent(QResizeEvent * ev) } -void GuiWorkArea::Private::update(int x, int y, int w, int h) -{ - p->viewport()->update(x, y, w, h); -} - - void GuiWorkArea::paintEvent(QPaintEvent * ev) { - QRectF const rc = ev->rect(); // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); if (d->needResize()) { - d->resetScreen(); d->resizeBufferView(); if (d->cursor_visible_) { d->hideCursor(); @@ -1197,29 +1187,14 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) } } - QPainter pain(viewport()); - double const pr = pixelRatio(); - QRectF const rcs = QRectF(rc.x() * pr, rc.y() * pr, rc.width() * pr, rc.height() * pr); + GuiPainter pain(viewport(), pixelRatio()); + d->buffer_view_->draw(pain); - if (lyxrc.use_qimage) { - QImage const & image = static_cast(*d->screen_); - pain.drawImage(rc, image, rcs); - } else { - QPixmap const & pixmap = static_cast(*d->screen_); - pain.drawPixmap(rc, pixmap, rcs); - } d->cursor_->draw(pain); ev->accept(); } -void GuiWorkArea::Private::updateScreen() -{ - GuiPainter pain(screen_, p->pixelRatio()); - buffer_view_->draw(pain); -} - - void GuiWorkArea::Private::showCursor(int x, int y, int h, bool l_shape, bool rtl, bool completable) { @@ -1229,9 +1204,8 @@ void GuiWorkArea::Private::showCursor(int x, int y, int h, // We can't use redraw() here because this would trigger a infinite // recursive loop with showCursor(). buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); - updateScreen(); + p->viewport()->update(); updateScrollbar(); - p->viewport()->update(QRect(0, 0, p->viewport()->width(), p->viewport()->height())); schedule_redraw_ = false; // Show the cursor immediately after the update. hideCursor(); @@ -1255,6 +1229,10 @@ void GuiWorkArea::Private::removeCursor() void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) { +//FIXME Broken Feature !! +// I do not think that we are supposed to paint inside this event. Shall we +// just let TextMetrics::breakRow add this to the relevant Row object? +#if 0 QString const & commit_string = e->commitString(); docstring const & preedit_string = qstring_to_ucs4(e->preeditString()); @@ -1290,7 +1268,7 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) } d->buffer_view_->updateMetrics(); - d->updateScreen(); + viewport()->update(); // FIXME: shall we use real_current_font here? (see #10478) FontInfo font = d->buffer_view_->cursor().getFont().fontInfo(); FontMetrics const & fm = theFontMetrics(font); @@ -1299,7 +1277,7 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) int cur_y = d->cursor_->rect().bottom(); // redraw area of preedit string. - update(0, cur_y - height, viewport()->width(), + viewport()->update(0, cur_y - height, viewport()->width(), (height + 1) * d->preedit_lines_); if (preedit_string.empty()) { @@ -1387,8 +1365,9 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) } // update the preedit string screen area. - update(0, cur_y - d->preedit_lines_*height, viewport()->width(), + viewport()->update(0, cur_y - d->preedit_lines_*height, viewport()->width(), (height + 1) * d->preedit_lines_); +#endif // Don't forget to accept the event! e->accept(); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index 76b05c0d36..42597172b1 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -92,10 +92,6 @@ struct GuiWorkArea::Private { Private(GuiWorkArea *); - /// update the passed area. - void update(int x, int y, int w, int h); - /// - void updateScreen(); /// void resizeBufferView(); @@ -123,34 +119,9 @@ struct GuiWorkArea::Private return need_resize_ || p->pixelRatio() != pixel_ratio_; } - void resetScreen() - { - delete screen_; - pixel_ratio_ = p->pixelRatio(); - if (lyxrc.use_qimage) { - QImage *x = - new QImage(static_cast(pixel_ratio_ * p->viewport()->width()), - static_cast(pixel_ratio_ * p->viewport()->height()), - QImage::Format_ARGB32_Premultiplied); -#if QT_VERSION >= 0x050000 - x->setDevicePixelRatio(pixel_ratio_); -#endif - screen_ = x; - } else { - QPixmap *x = - new QPixmap(static_cast(pixel_ratio_ * p->viewport()->width()), - static_cast(pixel_ratio_ * p->viewport()->height())); -#if QT_VERSION >= 0x050000 - x->setDevicePixelRatio(pixel_ratio_); -#endif - screen_ = x; - } - } /// GuiWorkArea * p; /// - QPaintDevice * screen_; - /// BufferView * buffer_view_; /// GuiView * lyx_view_; From ef1d50207088bc40cd0fbbd27229df9e8400e932 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Thu, 20 Jul 2017 23:31:05 +0200 Subject: [PATCH 03/24] Fix caret painting The trick is to remember in BufferView what has been done at the previous draw, so that the row that contained the caret can be repainted if needed. To this end, add an argument paint_caret to BufferView, although painting the caret is not the job of the BufferView (at this point). BufferView::needRepaint will act as an interface with TextMetrics::drawParagraph to know whether the painting of a given row should be forced. Currently everything is done at the top row level, so that, if the caret is in a large table, the whole table will have to be repainted. It is not clear yet that this is necessary. --- src/BufferView.cpp | 54 ++++++++++++++++++++++++++++--- src/BufferView.h | 6 +++- src/MetricsInfo.h | 2 +- src/TextMetrics.cpp | 3 +- src/frontends/qt4/GuiWorkArea.cpp | 5 +-- 5 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 94e448d1e9..3c6adfcd74 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -235,7 +235,7 @@ struct BufferView::Private last_inset_(0), clickable_inset_(false), mouse_position_cache_(), bookmark_edit_position_(-1), gui_(0), - horiz_scroll_offset_(0) + horiz_scroll_offset_(0), repaint_caret_row_(false) { xsel_cache_.set = false; } @@ -312,6 +312,12 @@ struct BufferView::Private /// a slice pointing to the start of the row where cursor was /// at previous draw event CursorSlice last_row_slice_; + + /// a slice pointing to where the cursor has been drawn after the current + /// draw() call. + CursorSlice caret_slice_; + /// indicates whether the caret slice needs to be repainted in this draw() run. + bool repaint_caret_row_; }; @@ -2748,7 +2754,7 @@ void BufferView::updatePosCache() // this is the "nodraw" drawing stage: only set the positions of the // insets in metrics cache. frontend::NullPainter np; - draw(np); + draw(np, false); } @@ -2959,6 +2965,23 @@ void BufferView::setCurrentRowSlice(CursorSlice const & rowSlice) } +namespace { + +bool sliceInRow(CursorSlice const & cs, Text const * text, Row const & row) +{ + return !cs.empty() && cs.text() == text && cs.pit() == row.pit() + && row.pos() <= cs.pos() && cs.pos() <= row.endpos(); +} + +} + + +bool BufferView::needRepaint(Text const * text, Row const & row) const +{ + return d->repaint_caret_row_ && sliceInRow(d->caret_slice_, text, row); +} + + void BufferView::checkCursorScrollOffset(PainterInfo & pi) { CursorSlice rowSlice = d->cursor_.bottom(); @@ -3047,7 +3070,7 @@ void BufferView::checkCursorScrollOffset(PainterInfo & pi) } -void BufferView::draw(frontend::Painter & pain) +void BufferView::draw(frontend::Painter & pain, bool paint_caret) { if (height_ == 0 || width_ == 0) return; @@ -3058,6 +3081,16 @@ void BufferView::draw(frontend::Painter & pain) int const y = tm.first().second->position(); PainterInfo pi(this, pain); + CursorSlice const & bottomSlice = d->cursor_.bottom(); + /** A repaint of the previous cursor row is needed if + * 1/ the caret will be painted and is is not the same than the + * already painted one; + * 2/ the caret will not be painted, but there is already one on + * screen. + */ + d->repaint_caret_row_ = (paint_caret && bottomSlice != d->caret_slice_) + || (! paint_caret && !d->caret_slice_.empty()); + // Check whether the row where the cursor lives needs to be scrolled. // Update the drawing strategy if needed. checkCursorScrollOffset(pi); @@ -3069,9 +3102,14 @@ void BufferView::draw(frontend::Painter & pain) // however, the different coordinates of insets and paragraphs // needs to be updated. LYXERR(Debug::PAINTING, "Strategy: NoScreenUpdate"); - pi.full_repaint = true; - if (pain.isNull()) + pi.full_repaint = false; + if (pain.isNull()) { + pi.full_repaint = true; tm.draw(pi, 0, y); + } else if (d->repaint_caret_row_) { + pi.full_repaint = false; + tm.draw(pi, 0, y); + } break; case SingleParUpdate: @@ -3134,6 +3172,12 @@ void BufferView::draw(frontend::Painter & pain) } LYXERR(Debug::PAINTING, "Found new anchor pit = " << d->anchor_pit_ << " anchor ypos = " << d->anchor_ypos_); + + // Remember what has just been done for the next draw() step + if (paint_caret) + d->caret_slice_ = bottomSlice; + else + d->caret_slice_ = CursorSlice(); } diff --git a/src/BufferView.h b/src/BufferView.h index 40af7c63b2..eed408dae7 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -46,6 +46,7 @@ class PainterInfo; class ParIterator; class ParagraphMetrics; class Point; +class Row; class TexRow; class Text; class TextMetrics; @@ -132,6 +133,9 @@ public: /// Only to be called with good y coordinates (after a bv::metrics) bool needsFitCursor() const; + /// returns true if this row needs to be repainted (to erase caret) + bool needRepaint(Text const * text, Row const & row) const; + // Returns the amount of horizontal scrolling applied to the // top-level row where the cursor lies int horizScrollOffset() const; @@ -308,7 +312,7 @@ public: void cursorPosAndHeight(Point & p, int & h) const; /// - void draw(frontend::Painter & pain); + void draw(frontend::Painter & pain, bool paint_caret); /// get this view's keyboard map handler. Intl & getIntl(); diff --git a/src/MetricsInfo.h b/src/MetricsInfo.h index 2a18cf8cff..ff0b1c6989 100644 --- a/src/MetricsInfo.h +++ b/src/MetricsInfo.h @@ -130,7 +130,7 @@ public: bool selected; /// Whether the spell checker is enabled for the parent bool do_spellcheck; - /// + /// True when it can be assumed that the screen has been cleared bool full_repaint; /// Current background color ColorCode background_color; diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 360d83b296..52717400d5 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -1876,7 +1876,8 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // Row signature; has row changed since last paint? row.setCrc(pm.computeRowSignature(row, *bv_)); bool row_has_changed = row.changed() - || bv_->hadHorizScrollOffset(text_, pit, row.pos()); + || bv_->hadHorizScrollOffset(text_, pit, row.pos()) + || bv_->needRepaint(text_, row); // Take this opportunity to spellcheck the row contents. if (row_has_changed && pi.do_spellcheck && lyxrc.spellcheck_continuously) { diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 8ce598583e..9a9e155f2d 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -1188,9 +1188,10 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) } GuiPainter pain(viewport(), pixelRatio()); - d->buffer_view_->draw(pain); + d->buffer_view_->draw(pain, d->cursor_visible_); - d->cursor_->draw(pain); + if (d->cursor_visible_) + d->cursor_->draw(pain); ev->accept(); } From 418bf09879dfad860e298f0f3b16d765bb1262d5 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 22 Jul 2017 01:19:45 +0200 Subject: [PATCH 04/24] Cleanup and simplify WorkArea code Rename cursor to caret to in order to avoid ambiguity. The caret is now the blinking thing only. Remove unused header contents, and some not so useful methods. No intended change of behavior. --- src/frontends/qt4/FindAndReplace.cpp | 4 +- src/frontends/qt4/GuiApplication.cpp | 11 +- src/frontends/qt4/GuiView.cpp | 8 +- src/frontends/qt4/GuiView.h | 2 +- src/frontends/qt4/GuiWorkArea.cpp | 292 ++++++++++-------------- src/frontends/qt4/GuiWorkArea.h | 12 +- src/frontends/qt4/GuiWorkArea_Private.h | 46 ++-- 7 files changed, 152 insertions(+), 223 deletions(-) diff --git a/src/frontends/qt4/FindAndReplace.cpp b/src/frontends/qt4/FindAndReplace.cpp index 4411074de7..607f1653aa 100644 --- a/src/frontends/qt4/FindAndReplace.cpp +++ b/src/frontends/qt4/FindAndReplace.cpp @@ -65,8 +65,8 @@ FindAndReplaceWidget::FindAndReplaceWidget(GuiView & view) replace_work_area_->setFrameStyle(QFrame::StyledPanel); // We don't want two cursors blinking. - find_work_area_->stopBlinkingCursor(); - replace_work_area_->stopBlinkingCursor(); + find_work_area_->stopBlinkingCaret(); + replace_work_area_->stopBlinkingCaret(); } diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index 0ce130eb69..ee379ffb56 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -122,7 +122,6 @@ #include #include #include -#undef CursorShape #undef None #elif defined(QPA_XCB) #include @@ -1435,7 +1434,7 @@ void GuiApplication::updateCurrentView(FuncRequest const & cmd, DispatchResult & theSelection().haveSelection(bv->cursor().selection()); // update gui - current_view_->restartCursor(); + current_view_->restartCaret(); } if (dr.needMessageUpdate()) { // Some messages may already be translated, so we cannot use _() @@ -2151,7 +2150,7 @@ void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) if (!keysym.isOK()) LYXERR(Debug::KEY, "Empty kbd action (probably composing)"); if (current_view_) - current_view_->restartCursor(); + current_view_->restartCaret(); return; } @@ -2211,7 +2210,7 @@ void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) if (!isPrintable(encoded_last_key)) { LYXERR(Debug::KEY, "Non-printable character! Omitting."); if (current_view_) - current_view_->restartCursor(); + current_view_->restartCaret(); return; } // The following modifier check is not needed on Mac. @@ -2233,7 +2232,7 @@ void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) { if (current_view_) { current_view_->message(_("Unknown function.")); - current_view_->restartCursor(); + current_view_->restartCaret(); } return; } @@ -2248,7 +2247,7 @@ void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) LYXERR(Debug::KEY, "Unknown Action and not isText() -- giving up"); if (current_view_) { current_view_->message(_("Unknown function.")); - current_view_->restartCursor(); + current_view_->restartCaret(); } return; } diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index c587460297..a7d669a774 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -4354,13 +4354,13 @@ Buffer const * GuiView::updateInset(Inset const * inset) } -void GuiView::restartCursor() +void GuiView::restartCaret() { /* When we move around, or type, it's nice to be able to see - * the cursor immediately after the keypress. + * the caret immediately after the keypress. */ if (d.current_work_area_) - d.current_work_area_->startBlinkingCursor(); + d.current_work_area_->startBlinkingCaret(); // Take this occasion to update the other GUI elements. updateDialogs(); @@ -4429,7 +4429,7 @@ void GuiView::resetDialogs() // Now update controls with current buffer. guiApp->setCurrentView(this); restoreLayout(); - restartCursor(); + restartCaret(); } diff --git a/src/frontends/qt4/GuiView.h b/src/frontends/qt4/GuiView.h index ecf4e446c6..68f1734a94 100644 --- a/src/frontends/qt4/GuiView.h +++ b/src/frontends/qt4/GuiView.h @@ -116,7 +116,7 @@ public: /// \return true if the \c FuncRequest has been dispatched. void dispatch(FuncRequest const & cmd, DispatchResult & dr); - void restartCursor(); + void restartCaret(); /// Update the completion popup and the inline completion state. /// If \c start is true, then a new completion might be started. /// If \c keep is true, an active completion will be kept active diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 9a9e155f2d..6773412cb0 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -22,6 +22,7 @@ #include "GuiPainter.h" #include "GuiView.h" #include "Menus.h" +#include "qt_helpers.h" #include "Buffer.h" #include "BufferList.h" @@ -36,7 +37,6 @@ #include "LyX.h" #include "LyXRC.h" #include "LyXVC.h" -#include "qt_helpers.h" #include "Text.h" #include "TextMetrics.h" #include "version.h" @@ -46,7 +46,6 @@ #include "support/convert.h" #include "support/debug.h" -#include "support/gettext.h" #include "support/lassert.h" #include "support/TempFile.h" @@ -68,7 +67,6 @@ #include #include #include -#include #include #include #include @@ -77,8 +75,6 @@ #include #include -#include "support/bind.h" - #include #include @@ -130,17 +126,15 @@ mouse_button::state q_motion_state(Qt::MouseButtons state) namespace frontend { -class CursorWidget { +class CaretWidget { public: - CursorWidget() : rtl_(false), l_shape_(false), completable_(false), - show_(false), x_(0), cursor_width_(0) - { - recomputeWidth(); - } + CaretWidget() : rtl_(false), l_shape_(false), completable_(false), + x_(0), caret_width_(0) + {} void draw(QPainter & painter) { - if (!show_ || !rect_.isValid()) + if (!rect_.isValid()) return; int y = rect_.top(); @@ -149,7 +143,7 @@ public: int bot = rect_.bottom(); // draw vertical line - painter.fillRect(x_, y, cursor_width_, rect_.height(), color_); + painter.fillRect(x_, y, caret_width_, rect_.height(), color_); // draw RTL/LTR indication painter.setPen(color_); @@ -157,7 +151,7 @@ public: if (rtl_) painter.drawLine(x_, bot, x_ - l, bot); else - painter.drawLine(x_, bot, x_ + cursor_width_ + r, bot); + painter.drawLine(x_, bot, x_ + caret_width_ + r, bot); } // draw completion triangle @@ -168,8 +162,8 @@ public: painter.drawLine(x_ - 1, m - d, x_ - 1 - d, m); painter.drawLine(x_ - 1, m + d, x_ - 1 - d, m); } else { - painter.drawLine(x_ + cursor_width_, m - d, x_ + cursor_width_ + d, m); - painter.drawLine(x_ + cursor_width_, m + d, x_ + cursor_width_ + d, m); + painter.drawLine(x_ + caret_width_, m - d, x_ + caret_width_ + d, m); + painter.drawLine(x_ + caret_width_, m + d, x_ + caret_width_ + d, m); } } } @@ -203,17 +197,12 @@ public: r = max(r, TabIndicatorWidth); } - // compute overall rectangle - rect_ = QRect(x - l, y, cursor_width_ + r + l, h); - } - - void show(bool set_show = true) { show_ = set_show; } - void hide() { show_ = false; } - int cursorWidth() const { return cursor_width_; } - void recomputeWidth() { - cursor_width_ = lyxrc.cursor_width + caret_width_ = lyxrc.cursor_width ? lyxrc.cursor_width : 1 + int((lyxrc.currentZoom + 50) / 200.0); + + // compute overall rectangle + rect_ = QRect(x - l, y, caret_width_ + r + l, h); } QRect const & rect() { return rect_; } @@ -226,15 +215,13 @@ private: /// triangle to show that a completion is available bool completable_; /// - bool show_; - /// QColor color_; /// rectangle, possibly with l_shape and completion triangle QRect rect_; /// x position (were the vertical line is drawn) int x_; - - int cursor_width_; + /// the width of the vertical blinking bar + int caret_width_; }; @@ -247,12 +234,34 @@ SyntheticMouseEvent::SyntheticMouseEvent() GuiWorkArea::Private::Private(GuiWorkArea * parent) : p(parent), buffer_view_(0), lyx_view_(0), - cursor_visible_(false), cursor_(0), + caret_(0), caret_visible_(false), need_resize_(false), schedule_redraw_(false), preedit_lines_(1), pixel_ratio_(1.0), completer_(new GuiCompleter(p, p)), dialog_mode_(false), shell_escape_(false), read_only_(false), clean_(true), externally_modified_(false) { + int const time = QApplication::cursorFlashTime() / 2; + if (time > 0) { + caret_timeout_.setInterval(time); + caret_timeout_.start(); + } else { + // let's initialize this just to be safe + caret_timeout_.setInterval(500); + } +} + + +GuiWorkArea::Private::~Private() +{ + // If something is wrong with the buffer, we can ignore it safely + try { + buffer_view_->buffer().workAreaManager().remove(p); + } catch(...) {} + delete buffer_view_; + delete caret_; + // Completer has a QObject parent and is thus automatically destroyed. + // See #4758. + // delete completer_; } @@ -287,23 +296,18 @@ double GuiWorkArea::pixelRatio() const void GuiWorkArea::init() { // Setup the signals - connect(&d->cursor_timeout_, SIGNAL(timeout()), - this, SLOT(toggleCursor())); + connect(&d->caret_timeout_, SIGNAL(timeout()), + this, SLOT(toggleCaret())); - int const time = QApplication::cursorFlashTime() / 2; - if (time > 0) { - d->cursor_timeout_.setInterval(time); - d->cursor_timeout_.start(); - } else { - // let's initialize this just to be safe - d->cursor_timeout_.setInterval(500); - } + // This connection is closed at the same time as this is destroyed. + d->synthetic_mouse_event_.timeout.timeout.connect([this](){ + generateSyntheticMouseEvent(); + }); // With Qt4.5 a mouse event will happen before the first paint event // so make sure that the buffer view has an up to date metrics. d->buffer_view_->resize(viewport()->width(), viewport()->height()); - d->cursor_ = new frontend::CursorWidget(); - d->cursor_->hide(); + d->caret_ = new frontend::CaretWidget(); setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); setAcceptDrops(true); @@ -312,63 +316,33 @@ void GuiWorkArea::init() setFrameStyle(QFrame::NoFrame); updateWindowTitle(); - //viewport()->setAutoFillBackground(false); - // We don't need double-buffering nor SystemBackground on - // the viewport because we have our own backing pixmap. - //viewport()->setAttribute(Qt::WA_NoSystemBackground); + d->updateCursorShape(); + + // we paint our own background viewport()->setAttribute(Qt::WA_OpaquePaintEvent); setFocusPolicy(Qt::StrongFocus); - d->setCursorShape(Qt::IBeamCursor); - - // This connection is closed at the same time as this is destroyed. - d->synthetic_mouse_event_.timeout.timeout.connect([this](){ - generateSyntheticMouseEvent(); - }); - LYXERR(Debug::GUI, "viewport width: " << viewport()->width() << " viewport height: " << viewport()->height()); // Enables input methods for asian languages. // Must be set when creating custom text editing widgets. setAttribute(Qt::WA_InputMethodEnabled, true); - - d->dialog_mode_ = false; } GuiWorkArea::~GuiWorkArea() { - // If something is wrong with the buffer, we can ignore it safely - try { - d->buffer_view_->buffer().workAreaManager().remove(this); - } catch(...) {} - delete d->buffer_view_; - delete d->cursor_; - // Completer has a QObject parent and is thus automatically destroyed. - // See #4758. - // delete completer_; delete d; } -Qt::CursorShape GuiWorkArea::cursorShape() const -{ - return viewport()->cursor().shape(); -} - - -void GuiWorkArea::Private::setCursorShape(Qt::CursorShape shape) -{ - p->viewport()->setCursor(shape); -} - - void GuiWorkArea::Private::updateCursorShape() { - setCursorShape(buffer_view_->clickableInset() - ? Qt::PointingHandCursor : Qt::IBeamCursor); + bool const clickable = buffer_view_ && buffer_view_->clickableInset(); + p->viewport()->setCursor(clickable ? Qt::PointingHandCursor + : Qt::IBeamCursor); } @@ -435,14 +409,14 @@ BufferView const & GuiWorkArea::bufferView() const } -void GuiWorkArea::stopBlinkingCursor() +void GuiWorkArea::stopBlinkingCaret() { - d->cursor_timeout_.stop(); - d->hideCursor(); + d->caret_timeout_.stop(); + d->hideCaret(); } -void GuiWorkArea::startBlinkingCursor() +void GuiWorkArea::startBlinkingCaret() { // do not show the cursor if the view is busy if (view().busy()) @@ -455,14 +429,23 @@ void GuiWorkArea::startBlinkingCursor() if (!d->buffer_view_->cursorInView(p, h)) return; - d->showCursor(); + d->showCaret(); //we're not supposed to cache this value. int const time = QApplication::cursorFlashTime() / 2; if (time <= 0) return; - d->cursor_timeout_.setInterval(time); - d->cursor_timeout_.start(); + d->caret_timeout_.setInterval(time); + d->caret_timeout_.start(); +} + + +void GuiWorkArea::toggleCaret() +{ + if (d->caret_visible_) + d->hideCaret(); + else + d->showCaret(); } @@ -484,9 +467,9 @@ void GuiWorkArea::redraw(bool update_metrics) // update cursor position, because otherwise it has to wait until // the blinking interval is over - if (d->cursor_visible_) { - d->hideCursor(); - d->showCursor(); + if (d->caret_visible_) { + d->hideCaret(); + d->showCaret(); } LYXERR(Debug::WORKAREA, "WorkArea::redraw screen"); @@ -524,9 +507,9 @@ void GuiWorkArea::processKeySym(KeySymbol const & key, KeyModifier mod) } // In order to avoid bad surprise in the middle of an operation, - // we better stop the blinking cursor... - // the cursor gets restarted in GuiView::restartCursor() - stopBlinkingCursor(); + // we better stop the blinking caret... + // the cursor gets restarted in GuiView::restartCaret() + stopBlinkingCaret(); guiApp->processKeySym(key, mod); } @@ -546,7 +529,7 @@ void GuiWorkArea::Private::dispatch(FuncRequest const & cmd) // In order to avoid bad surprise in the middle of an operation, we better stop // the blinking cursor. if (notJustMovingTheMouse) - p->stopBlinkingCursor(); + p->stopBlinkingCaret(); buffer_view_->mouseEventDispatch(cmd); @@ -567,7 +550,7 @@ void GuiWorkArea::Private::dispatch(FuncRequest const & cmd) lyx_view_->clearMessage(); // Show the cursor immediately after any operation - p->startBlinkingCursor(); + p->startBlinkingCaret(); } updateCursorShape(); @@ -578,16 +561,16 @@ void GuiWorkArea::Private::resizeBufferView() { // WARNING: Please don't put any code that will trigger a repaint here! // We are already inside a paint event. - p->stopBlinkingCursor(); + p->stopBlinkingCaret(); // Warn our container (GuiView). p->busy(true); Point point; int h = 0; buffer_view_->cursorPosAndHeight(point, h); - bool const cursor_in_view = buffer_view_->cursorInView(point, h); + bool const caret_in_view = buffer_view_->cursorInView(point, h); buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); - if (cursor_in_view) + if (caret_in_view) buffer_view_->scrollToCursor(); p->viewport()->update(); @@ -603,19 +586,19 @@ void GuiWorkArea::Private::resizeBufferView() // We might be resizing even if the focus is on another widget so we only // restart the cursor if we have the focus. if (p->hasFocus()) - QTimer::singleShot(50, p, SLOT(startBlinkingCursor())); + QTimer::singleShot(50, p, SLOT(startBlinkingCaret())); } -void GuiWorkArea::Private::showCursor() +void GuiWorkArea::Private::showCaret() { - if (cursor_visible_) + if (caret_visible_) return; - Point p; + Point point; int h = 0; - buffer_view_->cursorPosAndHeight(p, h); - if (!buffer_view_->cursorInView(p, h)) + buffer_view_->cursorPosAndHeight(point, h); + if (!buffer_view_->cursorInView(point, h)) return; // RTL or not RTL @@ -638,34 +621,39 @@ void GuiWorkArea::Private::showCursor() && completer_->completionAvailable() && !completer_->popupVisible() && !completer_->inlineVisible(); - cursor_visible_ = true; - cursor_->recomputeWidth(); + caret_visible_ = true; //int cur_x = buffer_view_->getPos(cur).x_; // We may have decided to slide the cursor row so that cursor // is visible. - p.x_ -= buffer_view_->horizScrollOffset(); + point.x_ -= buffer_view_->horizScrollOffset(); - showCursor(p.x_, p.y_, h, l_shape, isrtl, completable); + caret_->update(point.x_, point.y_, h, l_shape, isrtl, completable); + + if (schedule_redraw_) { + // This happens when a graphic conversion is finished. As we don't know + // the size of the new graphics, it's better the update everything. + // We can't use redraw() here because this would trigger a infinite + // recursive loop with showCaret(). + buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); + p->viewport()->update(); + updateScrollbar(); + schedule_redraw_ = false; + return; + } + + p->viewport()->update(caret_->rect()); } -void GuiWorkArea::Private::hideCursor() +void GuiWorkArea::Private::hideCaret() { - if (!cursor_visible_) + if (!caret_visible_) return; - cursor_visible_ = false; - removeCursor(); -} - - -void GuiWorkArea::toggleCursor() -{ - if (d->cursor_visible_) - d->hideCursor(); - else - d->showCursor(); + caret_visible_ = false; + //if (!qApp->focusWidget()) + p->viewport()->update(caret_->rect()); } @@ -688,7 +676,7 @@ void GuiWorkArea::Private::updateScrollbar() void GuiWorkArea::scrollTo(int value) { - stopBlinkingCursor(); + stopBlinkingCaret(); d->buffer_view_->scrollDocView(value, true); if (lyxrc.cursor_follows_scrollbar) { @@ -697,7 +685,7 @@ void GuiWorkArea::scrollTo(int value) d->lyx_view_->updateLayoutList(); } // Show the cursor immediately after any operation. - startBlinkingCursor(); + startBlinkingCaret(); // FIXME QT5 #ifdef Q_WS_X11 QApplication::syncX(); @@ -803,7 +791,7 @@ void GuiWorkArea::focusInEvent(QFocusEvent * e) d->lyx_view_->currentWorkArea()->bufferView().buffer().updateBuffer(); } - startBlinkingCursor(); + startBlinkingCaret(); QAbstractScrollArea::focusInEvent(e); } @@ -811,7 +799,7 @@ void GuiWorkArea::focusInEvent(QFocusEvent * e) void GuiWorkArea::focusOutEvent(QFocusEvent * e) { LYXERR(Debug::DEBUG, "GuiWorkArea::focusOutEvent(): " << this << endl); - stopBlinkingCursor(); + stopBlinkingCaret(); QAbstractScrollArea::focusOutEvent(e); } @@ -1181,53 +1169,21 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) if (d->needResize()) { d->resizeBufferView(); - if (d->cursor_visible_) { - d->hideCursor(); - d->showCursor(); + if (d->caret_visible_) { + d->hideCaret(); + d->showCaret(); } } GuiPainter pain(viewport(), pixelRatio()); - d->buffer_view_->draw(pain, d->cursor_visible_); + d->buffer_view_->draw(pain, d->caret_visible_); - if (d->cursor_visible_) - d->cursor_->draw(pain); + if (d->caret_visible_) + d->caret_->draw(pain); ev->accept(); } -void GuiWorkArea::Private::showCursor(int x, int y, int h, - bool l_shape, bool rtl, bool completable) -{ - if (schedule_redraw_) { - // This happens when a graphic conversion is finished. As we don't know - // the size of the new graphics, it's better the update everything. - // We can't use redraw() here because this would trigger a infinite - // recursive loop with showCursor(). - buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); - p->viewport()->update(); - updateScrollbar(); - schedule_redraw_ = false; - // Show the cursor immediately after the update. - hideCursor(); - p->toggleCursor(); - return; - } - - cursor_->update(x, y, h, l_shape, rtl, completable); - cursor_->show(); - p->viewport()->update(cursor_->rect()); -} - - -void GuiWorkArea::Private::removeCursor() -{ - cursor_->hide(); - //if (!qApp->focusWidget()) - p->viewport()->update(cursor_->rect()); -} - - void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) { //FIXME Broken Feature !! @@ -1254,9 +1210,9 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) // Hide the cursor during the kana-kanji transformation. if (preedit_string.empty()) - startBlinkingCursor(); + startBlinkingCaret(); else - stopBlinkingCursor(); + stopBlinkingCaret(); // last_width : for checking if last preedit string was/wasn't empty. // FIXME THREAD && FIXME @@ -1274,8 +1230,8 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) FontInfo font = d->buffer_view_->cursor().getFont().fontInfo(); FontMetrics const & fm = theFontMetrics(font); int height = fm.maxHeight(); - int cur_x = d->cursor_->rect().left(); - int cur_y = d->cursor_->rect().bottom(); + int cur_x = d->caret_->rect().left(); + int cur_y = d->caret_->rect().bottom(); // redraw area of preedit string. viewport()->update(0, cur_y - height, viewport()->width(), @@ -1295,7 +1251,7 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) // get attributes of input method cursor. // cursor_pos : cursor position in preedit string. size_t cursor_pos = 0; - bool cursor_is_visible = false; + bool caret_is_visible = false; for (int i = 0; i != att.size(); ++i) { if (att.at(i).type == QInputMethodEvent::Cursor) { cursor_pos = att.at(i).start; @@ -1382,7 +1338,7 @@ QVariant GuiWorkArea::inputMethodQuery(Qt::InputMethodQuery query) const // this is the CJK-specific composition window position and // the context menu position when the menu key is pressed. case Qt::ImMicroFocus: - cur_r = d->cursor_->rect(); + cur_r = d->caret_->rect(); if (d->preedit_lines_ != 1) cur_r.moveLeft(10); cur_r.moveBottom(cur_r.bottom() @@ -1509,7 +1465,7 @@ QSize EmbeddedWorkArea::sizeHint () const void EmbeddedWorkArea::disable() { - stopBlinkingCursor(); + stopBlinkingCaret(); if (view().currentWorkArea() != this) return; // No problem if currentMainWorkArea() is 0 (setCurrentWorkArea() diff --git a/src/frontends/qt4/GuiWorkArea.h b/src/frontends/qt4/GuiWorkArea.h index 34b5d3aa4d..596b975c04 100644 --- a/src/frontends/qt4/GuiWorkArea.h +++ b/src/frontends/qt4/GuiWorkArea.h @@ -27,10 +27,6 @@ class QDropEvent; class QToolButton; class QWidget; -#ifdef CursorShape -#undef CursorShape -#endif - namespace lyx { class Buffer; @@ -81,8 +77,6 @@ public: /// GuiCompleter & completer(); - Qt::CursorShape cursorShape() const; - /// Return the GuiView this workArea belongs to GuiView const & view() const; GuiView & view(); @@ -95,9 +89,9 @@ public Q_SLOTS: /// This needs to be public because it is accessed externally by GuiView. void processKeySym(KeySymbol const & key, KeyModifier mod); /// - void stopBlinkingCursor(); + void stopBlinkingCaret(); /// - void startBlinkingCursor(); + void startBlinkingCaret(); Q_SIGNALS: /// @@ -119,7 +113,7 @@ private Q_SLOTS: /// timer to limit triple clicks void doubleClickTimeout(); /// toggle the cursor's visibility - void toggleCursor(); + void toggleCaret(); /// close this work area. /// Slot for Buffer::closing signal. void close(); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index 42597172b1..cae768a689 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -13,30 +13,13 @@ #define WORKAREA_PRIVATE_H #include "FuncRequest.h" -#include "LyXRC.h" #include "support/FileName.h" #include "support/Timeout.h" #include -#include -#include #include -class QContextMenuEvent; -class QDragEnterEvent; -class QDropEvent; -class QKeyEvent; -class QPaintEvent; -class QResizeEvent; -class QToolButton; -class QWheelEvent; -class QWidget; - -#ifdef CursorShape -#undef CursorShape -#endif - namespace lyx { class Buffer; @@ -86,34 +69,30 @@ public: /** * Implementation of the work area (buffer view GUI) */ -class CursorWidget; +class CaretWidget; struct GuiWorkArea::Private { + /// Private(GuiWorkArea *); + /// + ~Private(); + /// void resizeBufferView(); - /// paint the cursor and store the background - void showCursor(int x, int y, int h, - bool l_shape, bool rtl, bool completable); - - /// hide the cursor - void removeCursor(); /// void dispatch(FuncRequest const & cmd0); /// hide the visible cursor, if it is visible - void hideCursor(); + void hideCaret(); /// show the cursor if it is not visible - void showCursor(); + void showCaret(); /// Set the range and value of the scrollbar and connect to its valueChanged /// signal. void updateScrollbar(); /// Change the cursor when the mouse hovers over a clickable inset void updateCursorShape(); - /// - void setCursorShape(Qt::CursorShape shape); bool needResize() const { return need_resize_ || p->pixelRatio() != pixel_ratio_; @@ -125,18 +104,19 @@ struct GuiWorkArea::Private BufferView * buffer_view_; /// GuiView * lyx_view_; - /// is the cursor currently displayed - bool cursor_visible_; /// - QTimer cursor_timeout_; + CaretWidget * caret_; + /// is the cursor currently displayed + bool caret_visible_; + /// + QTimer caret_timeout_; + /// SyntheticMouseEvent synthetic_mouse_event_; /// DoubleClick dc_event_; - /// - CursorWidget * cursor_; /// bool need_resize_; /// From 9ef52806f8ff80bf1325368f07ba07a83172f7da Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sun, 23 Jul 2017 00:56:27 +0200 Subject: [PATCH 05/24] Make input methods support great again This unbreaks input methods by splitting the part of the code that does the actual drawing to a separate paintPreeditText() method which is called from paintEvent(). The proper solution would have been to introduce the preedit string in the Row object, like is done for completion, but this is too complex to do at this point. The only change in behavior is that now the commit string is inserted in one fell swoop, intead of emulating a number of key events. --- src/frontends/qt4/GuiWorkArea.cpp | 194 ++++++++++++------------ src/frontends/qt4/GuiWorkArea_Private.h | 11 +- 2 files changed, 103 insertions(+), 102 deletions(-) diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 6773412cb0..c7f62c4db5 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -39,6 +39,7 @@ #include "LyXVC.h" #include "Text.h" #include "TextMetrics.h" +#include "Undo.h" #include "version.h" #include "graphics/GraphicsImage.h" @@ -1162,105 +1163,31 @@ void GuiWorkArea::resizeEvent(QResizeEvent * ev) } -void GuiWorkArea::paintEvent(QPaintEvent * ev) +void GuiWorkArea::Private::paintPreeditText(GuiPainter & pain) { - // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() - // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); - - if (d->needResize()) { - d->resizeBufferView(); - if (d->caret_visible_) { - d->hideCaret(); - d->showCaret(); - } - } - - GuiPainter pain(viewport(), pixelRatio()); - d->buffer_view_->draw(pain, d->caret_visible_); - - if (d->caret_visible_) - d->caret_->draw(pain); - ev->accept(); -} - - -void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) -{ -//FIXME Broken Feature !! -// I do not think that we are supposed to paint inside this event. Shall we -// just let TextMetrics::breakRow add this to the relevant Row object? -#if 0 - QString const & commit_string = e->commitString(); - docstring const & preedit_string - = qstring_to_ucs4(e->preeditString()); - - if (!commit_string.isEmpty()) { - - LYXERR(Debug::KEY, "preeditString: " << e->preeditString() - << " commitString: " << e->commitString()); - - int key = 0; - - // FIXME Iwami 04/01/07: we should take care also of UTF16 surrogates here. - for (int i = 0; i != commit_string.size(); ++i) { - QKeyEvent ev(QEvent::KeyPress, key, Qt::NoModifier, commit_string[i]); - keyPressEvent(&ev); - } - } - - // Hide the cursor during the kana-kanji transformation. - if (preedit_string.empty()) - startBlinkingCaret(); - else - stopBlinkingCaret(); - - // last_width : for checking if last preedit string was/wasn't empty. - // FIXME THREAD && FIXME - // We could have more than one work area, right? - static bool last_width = false; - if (!last_width && preedit_string.empty()) { - // if last_width is last length of preedit string. - e->accept(); + if (preedit_string_.empty()) return; - } - d->buffer_view_->updateMetrics(); - viewport()->update(); // FIXME: shall we use real_current_font here? (see #10478) - FontInfo font = d->buffer_view_->cursor().getFont().fontInfo(); + FontInfo const font = buffer_view_->cursor().getFont().fontInfo(); FontMetrics const & fm = theFontMetrics(font); - int height = fm.maxHeight(); - int cur_x = d->caret_->rect().left(); - int cur_y = d->caret_->rect().bottom(); - - // redraw area of preedit string. - viewport()->update(0, cur_y - height, viewport()->width(), - (height + 1) * d->preedit_lines_); - - if (preedit_string.empty()) { - last_width = false; - d->preedit_lines_ = 1; - e->accept(); - return; - } - last_width = true; - - // att : stores an IM attribute. - QList const & att = e->attributes(); + int const height = fm.maxHeight(); + int cur_x = caret_->rect().left(); + int cur_y = caret_->rect().bottom(); // get attributes of input method cursor. // cursor_pos : cursor position in preedit string. size_t cursor_pos = 0; - bool caret_is_visible = false; - for (int i = 0; i != att.size(); ++i) { - if (att.at(i).type == QInputMethodEvent::Cursor) { - cursor_pos = att.at(i).start; - cursor_is_visible = att.at(i).length != 0; + bool cursor_is_visible = false; + for (auto const & attr : preedit_attr_) { + if (attr.type == QInputMethodEvent::Cursor) { + cursor_pos = attr.start; + cursor_is_visible = attr.length != 0; break; } } - size_t preedit_length = preedit_string.length(); + size_t const preedit_length = preedit_string_.length(); // get position of selection in input method. // FIXME: isn't there a way to do this simplier? @@ -1269,12 +1196,12 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) // rLength : selected string length in IM. size_t rLength = 0; if (cursor_pos < preedit_length) { - for (int i = 0; i != att.size(); ++i) { - if (att.at(i).type == QInputMethodEvent::TextFormat) { - if (att.at(i).start <= int(cursor_pos) - && int(cursor_pos) < att.at(i).start + att.at(i).length) { - rStart = att.at(i).start; - rLength = att.at(i).length; + for (auto const & attr : preedit_attr_) { + if (attr.type == QInputMethodEvent::TextFormat) { + if (attr.start <= int(cursor_pos) + && int(cursor_pos) < attr.start + attr.length) { + rStart = attr.start; + rLength = attr.length; if (!cursor_is_visible) cursor_pos += rLength; break; @@ -1287,20 +1214,20 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) rLength = 0; } - int const right_margin = d->buffer_view_->rightMargin(); + int const right_margin = buffer_view_->rightMargin(); Painter::preedit_style ps; // Most often there would be only one line: - d->preedit_lines_ = 1; + preedit_lines_ = 1; for (size_t pos = 0; pos != preedit_length; ++pos) { - char_type const typed_char = preedit_string[pos]; + char_type const typed_char = preedit_string_[pos]; // reset preedit string style ps = Painter::preedit_default; // if we reached the right extremity of the screen, go to next line. - if (cur_x + fm.width(typed_char) > viewport()->width() - right_margin) { + if (cur_x + fm.width(typed_char) > p->viewport()->width() - right_margin) { cur_x = right_margin; cur_y += height + 1; - ++d->preedit_lines_; + ++preedit_lines_; } // preedit strings are displayed with dashed underline // and partial strings are displayed white on black indicating @@ -1317,14 +1244,79 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) ps = Painter::preedit_cursor; // draw one character and update cur_x. - GuiPainter pain(d->screen_, pixelRatio()); cur_x += pain.preeditText(cur_x, cur_y, typed_char, font, ps); } +} - // update the preedit string screen area. - viewport()->update(0, cur_y - d->preedit_lines_*height, viewport()->width(), + +void GuiWorkArea::paintEvent(QPaintEvent * ev) +{ + // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() + // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); + + if (d->needResize()) { + d->resizeBufferView(); + if (d->caret_visible_) { + d->hideCaret(); + d->showCaret(); + } + } + + GuiPainter pain(viewport(), pixelRatio()); + d->buffer_view_->draw(pain, d->caret_visible_); + + // The preedit text, if needed + d->paintPreeditText(pain); + + // and the caret + if (d->caret_visible_) + d->caret_->draw(pain); + ev->accept(); +} + + +void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) +{ + LYXERR(Debug::KEY, "preeditString: " << e->preeditString() + << " commitString: " << e->commitString()); + + // insert the processed text in the document (handles undo) + if (!e->commitString().isEmpty()) { + d->buffer_view_->cursor().beginUndoGroup(); + d->buffer_view_->cursor().insert(qstring_to_ucs4(e->commitString())); + d->buffer_view_->updateMetrics(); + d->buffer_view_->cursor().endUndoGroup(); + viewport()->update(); + } + + // Hide the cursor during the test transformation. + if (e->preeditString().isEmpty()) + startBlinkingCaret(); + else + stopBlinkingCaret(); + + if (d->preedit_string_.empty() && e->preeditString().isEmpty()) { + // Nothing to do + e->accept(); + return; + } + + // The preedit text and its attributes will be used in paintPreeditText + d->preedit_string_ = qstring_to_ucs4(e->preeditString()); + d->preedit_attr_ = e->attributes(); + + + // redraw area of preedit string. + int height = d->caret_->rect().height(); + int cur_y = d->caret_->rect().bottom(); + viewport()->update(0, cur_y - height, viewport()->width(), (height + 1) * d->preedit_lines_); -#endif + + if (d->preedit_string_.empty()) { + d->preedit_lines_ = 1; + e->accept(); + return; + } // Don't forget to accept the event! e->accept(); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index cae768a689..7198c569fe 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -27,6 +27,7 @@ class Buffer; namespace frontend { class GuiCompleter; +class GuiPainter; class GuiView; class GuiWorkArea; @@ -94,6 +95,8 @@ struct GuiWorkArea::Private /// Change the cursor when the mouse hovers over a clickable inset void updateCursorShape(); + void paintPreeditText(GuiPainter & pain); + bool needResize() const { return need_resize_ || p->pixelRatio() != pixel_ratio_; } @@ -121,8 +124,14 @@ struct GuiWorkArea::Private bool need_resize_; /// bool schedule_redraw_; - /// + + /// the current preedit text of the input method + docstring preedit_string_; + /// Number of lines used by preedit text int preedit_lines_; + /// the attributes of the preedit text + QList preedit_attr_; + /// Ratio between physical pixels and device-independent pixels /// We save the last used value to detect changes of the /// current pixel_ratio of the viewport. From 8bb7ce4e4a25622b91208e18702fbd52c59bf16f Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sun, 23 Jul 2017 12:39:26 +0200 Subject: [PATCH 06/24] Update the painting process documentation --- development/PAINTING_ANALYSIS | 44 +++++++++++------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS index e53b4b23df..9d625934bb 100644 --- a/development/PAINTING_ANALYSIS +++ b/development/PAINTING_ANALYSIS @@ -67,31 +67,11 @@ drawing from metrics. Other changes are only clean-ups. ** When a paragraph ends with a newline, compute correctly the height of the extra row. -** Rewrite TextMetrics::editXY, checkInsetHit using row information (getPosNearX)? - - The helper version should return a Row::Element instead of an InsetTable. - -** Set inset position during metrics phase - -In order to do that, a no-paint drawing will be initiated after every -redoParagraph. This code path will need to be made as fast as possible. - -Effect: avoid depending on actual drawing having taken place. In turn, -it will allow to do drawing on paint events, like any reasonable -application would do. - ** Cleanup after complete metrics Then the following can be done: + remove hack in InsetMathNest::drawSelection - + remove painting when not inside in drawParagraph + remove Cursor::inCoordCache? -** Paint directly to screen - -Instead of using an intermediary pixmap. I have no idea of how -difficult it will prove. -One benefit will be that subpixel aliasing will work again (#9972) - ** Merging bv::updateMetrics and tm::metrics While the full metrics computation tries hard to limit the number of @@ -107,19 +87,19 @@ necessary to break the whole contents to know the width of the cell. * Description of current drawing mechanism -** Two stage drawing +** Three-stage drawing -There are two parts to drawing the work area: +There are three parts to drawing the work area: + the metrics phase computes the size of insets and breaks the paragraphs into rows. It stores the dimension of insets (both normal and math) in bv::coordCache. - + the drawing phase draws the contents and caches the inset - positions. Since the caching of positions is useful in itself, - there is a provision for drawing "without" drawing when the only - thing we want is to cache inset positions - (Painter::setDrawingEnabled). + + the nodraw drawing phase paints the screen (see below) with a null + painter. The only useful effect is to store the inset positions. + + + an update() signal is sent. This in turn will trigger a paint + event, and the actual screen painting will happen then. The machinery is controlled via bv::processUpdateFlags. This method is called at the end of bv::mouseEventDispatch and in @@ -145,7 +125,7 @@ update flag is Update::None. ** Metrics computation This is triggered by bv::updateMetrics, which calls tm::redoParagraph for -all visible paragraphs. Paragraphs above or below the screen (needed +all visible paragraphs. Some Paragraphs above or below the screen (needed for page up/down) and computed as needed. tm::redoParagraph will call Inset::metrics for each inset. In the case @@ -155,8 +135,9 @@ all the paragraphs of the inset. ** Drawing the work area. -This is done in bv::draw. This method is triggered mainly by -Buffer::changed, which draws all the work areas that show the given buffer. +This is done in bv::draw. This method is triggered by a paint event, +mainly called through Buffer::changed, which draws all the work areas +that show the given buffer. Note that, When Buffer::changed is called outside of bv::processUpdateFlags, it is not clear whether the update strategy @@ -186,3 +167,6 @@ The action depends on the update strategy: + SingleParUpdate: only tries to repaint current paragraph in a way that is not yet very clear to me. + +BufferView::draw can also be called with a null painter from +BufferView::updateMetrics(). From 410605385f1f40d3381fc7236dfb468d100d94f8 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sun, 23 Jul 2017 15:50:35 +0200 Subject: [PATCH 07/24] Remove workaround that is not necessary anymore. This code was necessary to handle cases where the insets positions were not yet in cache. This cannot happen anymore thanks to the nodraw stage. --- development/PAINTING_ANALYSIS | 5 ----- src/BufferView.cpp | 32 ++------------------------------ src/BufferView.h | 3 +-- src/Cursor.cpp | 13 ------------- src/Cursor.h | 2 -- 5 files changed, 3 insertions(+), 52 deletions(-) diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS index 9d625934bb..f734edb3b0 100644 --- a/development/PAINTING_ANALYSIS +++ b/development/PAINTING_ANALYSIS @@ -67,11 +67,6 @@ drawing from metrics. Other changes are only clean-ups. ** When a paragraph ends with a newline, compute correctly the height of the extra row. -** Cleanup after complete metrics - Then the following can be done: - + remove hack in InsetMathNest::drawSelection - + remove Cursor::inCoordCache? - ** Merging bv::updateMetrics and tm::metrics While the full metrics computation tries hard to limit the number of diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 3c6adfcd74..68ba3d7475 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2982,7 +2982,7 @@ bool BufferView::needRepaint(Text const * text, Row const & row) const } -void BufferView::checkCursorScrollOffset(PainterInfo & pi) +void BufferView::checkCursorScrollOffset() { CursorSlice rowSlice = d->cursor_.bottom(); TextMetrics const & tm = textMetrics(rowSlice.text()); @@ -2999,34 +2999,6 @@ void BufferView::checkCursorScrollOffset(PainterInfo & pi) // Set the row on which the cursor lives. setCurrentRowSlice(rowSlice); - // If insets referred to by cursor are not all in the cache, the positions - // need to be recomputed. - if (!d->cursor_.inCoordCache()) { - /** FIXME: the code below adds an extraneous computation of - * inset positions, and can therefore be bad for performance - * (think for example about a very large tabular inset. - * Redawing the row where it is means redrawing the whole - * screen). - * - * The bug that this fixes is the following: assume that there - * is a very large math inset. Upon entering the inset, when - * pressing `End', the row is not scrolled and the cursor is - * not visible. The extra row computation makes sure that the - * inset positions are correctly computed and set in the - * cache. This would not happen if we did not have two-stage - * drawing. - * - * A proper fix would be to always have proper inset positions - * at this point. - */ - // Force the recomputation of inset positions - frontend::NullPainter np; - PainterInfo(this, np); - // No need to care about vertical position. - RowPainter rp(pi, buffer().text(), row, -d->horiz_scroll_offset_, 0); - rp.paintText(); - } - // Current x position of the cursor in pixels int cur_x = getPos(d->cursor_).x_; @@ -3093,7 +3065,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) // Check whether the row where the cursor lives needs to be scrolled. // Update the drawing strategy if needed. - checkCursorScrollOffset(pi); + checkCursorScrollOffset(); switch (d->update_strategy_) { diff --git a/src/BufferView.h b/src/BufferView.h index eed408dae7..1c155685a5 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -42,7 +42,6 @@ class FuncStatus; class Intl; class Inset; class Length; -class PainterInfo; class ParIterator; class ParagraphMetrics; class Point; @@ -374,7 +373,7 @@ private: // Check whether the row where the cursor lives needs to be scrolled. // Update the drawing strategy if needed. - void checkCursorScrollOffset(PainterInfo & pi); + void checkCursorScrollOffset(); /// The minimal size of the document that is visible. Used /// when it is allowed to scroll below the document. diff --git a/src/Cursor.cpp b/src/Cursor.cpp index 2e8060509d..11ee5ea535 100644 --- a/src/Cursor.cpp +++ b/src/Cursor.cpp @@ -453,19 +453,6 @@ int Cursor::currentMode() } -bool Cursor::inCoordCache() const -{ - // the root inset is not in cache, but we do not need it. - if (depth() == 1) - return true; - CoordCache::Insets const & icache = bv_->coordCache().getInsets(); - for (size_t i = 1 ; i < depth() ; ++i) - if (!icache.has(&(*this)[i].inset())) - return false; - return true; -} - - void Cursor::getPos(int & x, int & y) const { Point p = bv().getPos(*this); diff --git a/src/Cursor.h b/src/Cursor.h index 8a8c71996d..5fb52638b7 100644 --- a/src/Cursor.h +++ b/src/Cursor.h @@ -215,8 +215,6 @@ public: /// are we entering a macro name? bool & macromode() { return macromode_; } - /// returns true when all insets in cursor stack are in cache - bool inCoordCache() const; /// returns x,y position void getPos(int & x, int & y) const; /// return logical positions between which the cursor is situated From 94b1d04f2c379291748365a296f2b6378d617f4a Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Thu, 24 Aug 2017 17:37:56 +0200 Subject: [PATCH 08/24] Rename more instances of "cursor" to "caret" Thanks to Pavel for the hint. --- src/BufferView.cpp | 2 +- src/BufferView.h | 4 ++-- src/frontends/qt4/GuiWorkArea.cpp | 31 +++++++++++++------------ src/frontends/qt4/GuiWorkArea.h | 2 +- src/frontends/qt4/GuiWorkArea_Private.h | 6 ++--- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 68ba3d7475..96c7dfe55f 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2892,7 +2892,7 @@ bool BufferView::paragraphVisible(DocIterator const & dit) const } -void BufferView::cursorPosAndHeight(Point & p, int & h) const +void BufferView::caretPosAndHeight(Point & p, int & h) const { Cursor const & cur = cursor(); Font const font = cur.real_current_font; diff --git a/src/BufferView.h b/src/BufferView.h index 1c155685a5..a5222599f2 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -307,8 +307,8 @@ public: bool paragraphVisible(DocIterator const & dit) const; /// is the cursor currently visible in the view bool cursorInView(Point const & p, int h) const; - /// get the position and height of the cursor - void cursorPosAndHeight(Point & p, int & h) const; + /// get the position and height of the caret + void caretPosAndHeight(Point & p, int & h) const; /// void draw(frontend::Painter & pain, bool paint_caret); diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index c7f62c4db5..70dcf3b32c 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -198,6 +198,7 @@ public: r = max(r, TabIndicatorWidth); } + //FIXME: LyXRC::cursor_width should be caret_width caret_width_ = lyxrc.cursor_width ? lyxrc.cursor_width : 1 + int((lyxrc.currentZoom + 50) / 200.0); @@ -209,7 +210,7 @@ public: QRect const & rect() { return rect_; } private: - /// cursor is in RTL or LTR text + /// caret is in RTL or LTR text bool rtl_; /// indication for RTL or LTR bool l_shape_; @@ -425,7 +426,7 @@ void GuiWorkArea::startBlinkingCaret() Point p; int h = 0; - d->buffer_view_->cursorPosAndHeight(p, h); + d->buffer_view_->caretPosAndHeight(p, h); // Don't start blinking if the cursor isn't on screen. if (!d->buffer_view_->cursorInView(p, h)) return; @@ -466,7 +467,7 @@ void GuiWorkArea::redraw(bool update_metrics) d->buffer_view_->cursor().fixIfBroken(); } - // update cursor position, because otherwise it has to wait until + // update caret position, because otherwise it has to wait until // the blinking interval is over if (d->caret_visible_) { d->hideCaret(); @@ -509,7 +510,7 @@ void GuiWorkArea::processKeySym(KeySymbol const & key, KeyModifier mod) // In order to avoid bad surprise in the middle of an operation, // we better stop the blinking caret... - // the cursor gets restarted in GuiView::restartCaret() + // the caret gets restarted in GuiView::restartCaret() stopBlinkingCaret(); guiApp->processKeySym(key, mod); } @@ -528,7 +529,7 @@ void GuiWorkArea::Private::dispatch(FuncRequest const & cmd) cmd.action() != LFUN_MOUSE_MOTION || cmd.button() != mouse_button::none; // In order to avoid bad surprise in the middle of an operation, we better stop - // the blinking cursor. + // the blinking caret. if (notJustMovingTheMouse) p->stopBlinkingCaret(); @@ -550,7 +551,7 @@ void GuiWorkArea::Private::dispatch(FuncRequest const & cmd) // FIXME: let GuiView take care of those. lyx_view_->clearMessage(); - // Show the cursor immediately after any operation + // Show the caret immediately after any operation p->startBlinkingCaret(); } @@ -568,7 +569,7 @@ void GuiWorkArea::Private::resizeBufferView() Point point; int h = 0; - buffer_view_->cursorPosAndHeight(point, h); + buffer_view_->caretPosAndHeight(point, h); bool const caret_in_view = buffer_view_->cursorInView(point, h); buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); if (caret_in_view) @@ -583,9 +584,9 @@ void GuiWorkArea::Private::resizeBufferView() need_resize_ = false; p->busy(false); - // Eventually, restart the cursor after the resize event. + // Eventually, restart the caret after the resize event. // We might be resizing even if the focus is on another widget so we only - // restart the cursor if we have the focus. + // restart the caret if we have the focus. if (p->hasFocus()) QTimer::singleShot(50, p, SLOT(startBlinkingCaret())); } @@ -598,7 +599,7 @@ void GuiWorkArea::Private::showCaret() Point point; int h = 0; - buffer_view_->cursorPosAndHeight(point, h); + buffer_view_->caretPosAndHeight(point, h); if (!buffer_view_->cursorInView(point, h)) return; @@ -616,7 +617,7 @@ void GuiWorkArea::Private::showCaret() if (realfont.language() == latex_language) l_shape = false; - // show cursor on screen + // show caret on screen Cursor & cur = buffer_view_->cursor(); bool completable = cur.inset().showCompletionCursor() && completer_->completionAvailable() @@ -625,7 +626,7 @@ void GuiWorkArea::Private::showCaret() caret_visible_ = true; //int cur_x = buffer_view_->getPos(cur).x_; - // We may have decided to slide the cursor row so that cursor + // We may have decided to slide the cursor row so that caret // is visible. point.x_ -= buffer_view_->horizScrollOffset(); @@ -685,7 +686,7 @@ void GuiWorkArea::scrollTo(int value) // FIXME: let GuiView take care of those. d->lyx_view_->updateLayoutList(); } - // Show the cursor immediately after any operation. + // Show the caret immediately after any operation. startBlinkingCaret(); // FIXME QT5 #ifdef Q_WS_X11 @@ -1289,7 +1290,7 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e) viewport()->update(); } - // Hide the cursor during the test transformation. + // Hide the caret during the test transformation. if (e->preeditString().isEmpty()) startBlinkingCaret(); else @@ -1335,7 +1336,7 @@ QVariant GuiWorkArea::inputMethodQuery(Qt::InputMethodQuery query) const cur_r.moveLeft(10); cur_r.moveBottom(cur_r.bottom() + cur_r.height() * (d->preedit_lines_ - 1)); - // return lower right of cursor in LyX. + // return lower right of caret in LyX. return cur_r; default: return QWidget::inputMethodQuery(query); diff --git a/src/frontends/qt4/GuiWorkArea.h b/src/frontends/qt4/GuiWorkArea.h index 596b975c04..63a9c8364e 100644 --- a/src/frontends/qt4/GuiWorkArea.h +++ b/src/frontends/qt4/GuiWorkArea.h @@ -112,7 +112,7 @@ private Q_SLOTS: void scrollTo(int value); /// timer to limit triple clicks void doubleClickTimeout(); - /// toggle the cursor's visibility + /// toggle the caret's visibility void toggleCaret(); /// close this work area. /// Slot for Buffer::closing signal. diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index 7198c569fe..199ce253d6 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -85,9 +85,9 @@ struct GuiWorkArea::Private /// void dispatch(FuncRequest const & cmd0); - /// hide the visible cursor, if it is visible + /// hide the visible caret, if it is visible void hideCaret(); - /// show the cursor if it is not visible + /// show the caret if it is not visible void showCaret(); /// Set the range and value of the scrollbar and connect to its valueChanged /// signal. @@ -110,7 +110,7 @@ struct GuiWorkArea::Private /// CaretWidget * caret_; - /// is the cursor currently displayed + /// is the caret currently displayed bool caret_visible_; /// QTimer caret_timeout_; From 9bb41f09436ad23fb881dd87e2caf3628b03403f Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 30 Aug 2017 18:05:16 +0200 Subject: [PATCH 09/24] Update insets position in cache in more cases This patch makes sure that, every time a ParagraphMetrics has its position set, the inset positions for the insets held by this paragraph are remembered too. This is complementary to BufferView::updatePosCache, but I do not have hard evidence that this is required other than to increase robustness. It may help in some cases when scrolling the document (scrollbar, cursor up/down, page up/down). --- src/BufferView.cpp | 4 ++++ src/TextMetrics.cpp | 11 +++++++++++ src/TextMetrics.h | 5 +++++ 3 files changed, 20 insertions(+) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 96c7dfe55f..58e4f8640f 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2645,6 +2645,7 @@ bool BufferView::singleParUpdate() // the singlePar optimisation. return false; + tm.updatePosCache(bottom_pit); d->update_strategy_ = SingleParUpdate; LYXERR(Debug::PAINTING, "\ny1: " << pm.position() - pm.ascent() @@ -2698,6 +2699,7 @@ void BufferView::updateMetrics() } } anchor_pm.setPosition(d->anchor_ypos_); + tm.updatePosCache(d->anchor_pit_); LYXERR(Debug::PAINTING, "metrics: " << " anchor pit = " << d->anchor_pit_ @@ -2713,6 +2715,7 @@ void BufferView::updateMetrics() y1 -= pm.descent(); // Save the paragraph position in the cache. pm.setPosition(y1); + tm.updatePosCache(pit1); y1 -= pm.ascent(); } @@ -2726,6 +2729,7 @@ void BufferView::updateMetrics() y2 += pm.ascent(); // Save the paragraph position in the cache. pm.setPosition(y2); + tm.updatePosCache(pit2); y2 += pm.descent(); } diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 52717400d5..5eb85bf118 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -43,6 +43,7 @@ #include "frontends/FontMetrics.h" #include "frontends/Painter.h" +#include "frontends/NullPainter.h" #include "support/debug.h" #include "support/lassert.h" @@ -198,6 +199,14 @@ bool TextMetrics::metrics(MetricsInfo & mi, Dimension & dim, int min_width) } +void TextMetrics::updatePosCache(pit_type pit) const +{ + frontend::NullPainter np; + PainterInfo pi(bv_, np); + drawParagraph(pi, pit, origin_.x_, par_metrics_[pit].position()); +} + + int TextMetrics::rightMargin(ParagraphMetrics const & pm) const { return text_->isMainText() ? pm.rightMargin(*bv_) : 0; @@ -1214,6 +1223,7 @@ void TextMetrics::newParMetricsDown() redoParagraph(pit); par_metrics_[pit].setPosition(last.second.position() + last.second.descent() + par_metrics_[pit].ascent()); + updatePosCache(pit); } @@ -1228,6 +1238,7 @@ void TextMetrics::newParMetricsUp() redoParagraph(pit); par_metrics_[pit].setPosition(first.second.position() - first.second.ascent() - par_metrics_[pit].descent()); + updatePosCache(pit); } // y is screen coordinate diff --git a/src/TextMetrics.h b/src/TextMetrics.h index 3000b218bf..ae99490955 100644 --- a/src/TextMetrics.h +++ b/src/TextMetrics.h @@ -62,6 +62,11 @@ public: /// void newParMetricsUp(); + /// The "nodraw" drawing stage for one single paragraph: set the + /// positions of the insets contained this paragraph in metrics + /// cache. Related to BufferView::updatePosCache. + void updatePosCache(pit_type pit) const; + /// Gets the fully instantiated font at a given position in a paragraph /// Basically the same routine as Paragraph::getFont() in Paragraph.cpp. /// The difference is that this one is used for displaying, and thus we From a1579c583acb22a7f5dae80a9bfe7707774f49a4 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Thu, 14 Sep 2017 15:50:30 +0200 Subject: [PATCH 10/24] Compute metrics when graphics is updated Remove the old schedule_redraw_ mechanism that was only useful because of our synchronous drawing code. Now that actual painting is scheduled instead of forced, it becomes pointless. Rename WorkArea::redraw(bool) to scheduleRedraw(bool), to show that the drawing is not done right away. In GuiView::updateInset, call scheduleRedraw(true), so that metrics are correctly computed (this was the whole point of the exercise). (cherry picked from commit a31d3dc67dce9655bee9f1b0a2bc2188d7d97453) --- src/frontends/WorkArea.h | 4 ++-- src/frontends/WorkAreaManager.cpp | 2 +- src/frontends/qt4/GuiView.cpp | 2 +- src/frontends/qt4/GuiWorkArea.cpp | 24 +++--------------------- src/frontends/qt4/GuiWorkArea.h | 4 +--- src/frontends/qt4/GuiWorkArea_Private.h | 2 -- 6 files changed, 8 insertions(+), 30 deletions(-) diff --git a/src/frontends/WorkArea.h b/src/frontends/WorkArea.h index 8e459ca375..c21555913e 100644 --- a/src/frontends/WorkArea.h +++ b/src/frontends/WorkArea.h @@ -36,8 +36,8 @@ public: /// virtual ~WorkArea() {} - /// redraw the screen, without using existing pixmap - virtual void redraw(bool update_metrics) = 0; + /// Update metrics if needed and schedule a paint event + virtual void scheduleRedraw(bool update_metrics) = 0; /// close this work area. /// Slot for Buffer::closing signal. diff --git a/src/frontends/WorkAreaManager.cpp b/src/frontends/WorkAreaManager.cpp index b98163cecc..c79f08bef7 100644 --- a/src/frontends/WorkAreaManager.cpp +++ b/src/frontends/WorkAreaManager.cpp @@ -35,7 +35,7 @@ void WorkAreaManager::remove(WorkArea * wa) void WorkAreaManager::redrawAll(bool update_metrics) { for (WorkArea * wa : work_areas_) - wa->redraw(update_metrics); + wa->scheduleRedraw(update_metrics); } diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index a7d669a774..785abfdd9b 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -4348,7 +4348,7 @@ Buffer const * GuiView::updateInset(Inset const * inset) continue; Buffer const * buffer = &(wa->bufferView().buffer()); if (inset_buffer == buffer) - wa->scheduleRedraw(); + wa->scheduleRedraw(true); } return inset_buffer; } diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 70dcf3b32c..6520c3aa7c 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -237,7 +237,7 @@ SyntheticMouseEvent::SyntheticMouseEvent() GuiWorkArea::Private::Private(GuiWorkArea * parent) : p(parent), buffer_view_(0), lyx_view_(0), caret_(0), caret_visible_(false), - need_resize_(false), schedule_redraw_(false), preedit_lines_(1), + need_resize_(false), preedit_lines_(1), pixel_ratio_(1.0), completer_(new GuiCompleter(p, p)), dialog_mode_(false), shell_escape_(false), read_only_(false), clean_(true), externally_modified_(false) @@ -451,7 +451,7 @@ void GuiWorkArea::toggleCaret() } -void GuiWorkArea::redraw(bool update_metrics) +void GuiWorkArea::scheduleRedraw(bool update_metrics) { if (!isVisible()) // No need to redraw in this case. @@ -632,18 +632,6 @@ void GuiWorkArea::Private::showCaret() caret_->update(point.x_, point.y_, h, l_shape, isrtl, completable); - if (schedule_redraw_) { - // This happens when a graphic conversion is finished. As we don't know - // the size of the new graphics, it's better the update everything. - // We can't use redraw() here because this would trigger a infinite - // recursive loop with showCaret(). - buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); - p->viewport()->update(); - updateScrollbar(); - schedule_redraw_ = false; - return; - } - p->viewport()->update(caret_->rect()); } @@ -1370,12 +1358,6 @@ bool GuiWorkArea::isFullScreen() const } -void GuiWorkArea::scheduleRedraw() -{ - d->schedule_redraw_ = true; -} - - bool GuiWorkArea::inDialogMode() const { return d->dialog_mode_; @@ -1788,7 +1770,7 @@ void TabWorkArea::on_currentTabChanged(int i) GuiWorkArea * wa = workArea(i); LASSERT(wa, return); wa->setUpdatesEnabled(true); - wa->redraw(true); + wa->scheduleRedraw(true); wa->setFocus(); /// currentWorkAreaChanged(wa); diff --git a/src/frontends/qt4/GuiWorkArea.h b/src/frontends/qt4/GuiWorkArea.h index 63a9c8364e..39d2054217 100644 --- a/src/frontends/qt4/GuiWorkArea.h +++ b/src/frontends/qt4/GuiWorkArea.h @@ -60,13 +60,11 @@ public: /// is GuiView in fullscreen mode? bool isFullScreen() const; /// - void scheduleRedraw(); - /// BufferView & bufferView(); /// BufferView const & bufferView() const; /// - void redraw(bool update_metrics); + void scheduleRedraw(bool update_metrics); /// return true if the key is part of a shortcut bool queryKeySym(KeySymbol const & key, KeyModifier mod) const; diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index 199ce253d6..8be28694cd 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -122,8 +122,6 @@ struct GuiWorkArea::Private /// bool need_resize_; - /// - bool schedule_redraw_; /// the current preedit text of the input method docstring preedit_string_; From c43b6a9ecf7d78a2187384e776d439698633f01f Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Mon, 18 Sep 2017 10:58:07 +0200 Subject: [PATCH 11/24] Remember correctly pixel ratio used for painting This avoids endless resize issues on HiDPI systems (e.g. Retina Mac). Rename pixel_ratio_ to last_pixel_ratio_ to emphasize that this is a cached value. Inline needResize method to make the logic clearer in paintEvent. (cherry picked from commit 6532e5104dfad5416817d89a5f91e53c30cdd523) --- src/frontends/qt4/GuiWorkArea.cpp | 6 ++++-- src/frontends/qt4/GuiWorkArea_Private.h | 6 +----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 6520c3aa7c..f4380da5e8 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -238,7 +238,7 @@ GuiWorkArea::Private::Private(GuiWorkArea * parent) : p(parent), buffer_view_(0), lyx_view_(0), caret_(0), caret_visible_(false), need_resize_(false), preedit_lines_(1), - pixel_ratio_(1.0), + last_pixel_ratio_(1.0), completer_(new GuiCompleter(p, p)), dialog_mode_(false), shell_escape_(false), read_only_(false), clean_(true), externally_modified_(false) { @@ -1243,7 +1243,7 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); - if (d->needResize()) { + if (d->need_resize_ || pixelRatio() != d->last_pixel_ratio_) { d->resizeBufferView(); if (d->caret_visible_) { d->hideCaret(); @@ -1251,6 +1251,8 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) } } + d->last_pixel_ratio_ = pixelRatio(); + GuiPainter pain(viewport(), pixelRatio()); d->buffer_view_->draw(pain, d->caret_visible_); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index 8be28694cd..ef2bb7e1df 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -97,10 +97,6 @@ struct GuiWorkArea::Private void paintPreeditText(GuiPainter & pain); - bool needResize() const { - return need_resize_ || p->pixelRatio() != pixel_ratio_; - } - /// GuiWorkArea * p; /// @@ -133,7 +129,7 @@ struct GuiWorkArea::Private /// Ratio between physical pixels and device-independent pixels /// We save the last used value to detect changes of the /// current pixel_ratio of the viewport. - double pixel_ratio_; + double last_pixel_ratio_; /// GuiCompleter * completer_; From 7b99bf6a375e14882713d96338345d130cab1425 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Mon, 18 Sep 2017 11:21:18 +0200 Subject: [PATCH 12/24] Do not presume what the defaults for a new QPainter are (cherry picked from commit 275d306c73e3e0f60e0742adbcb06cce98c48ee5) --- src/frontends/qt4/GuiPainter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp index bff8f9dc30..b03bb44ad3 100644 --- a/src/frontends/qt4/GuiPainter.cpp +++ b/src/frontends/qt4/GuiPainter.cpp @@ -52,10 +52,10 @@ GuiPainter::GuiPainter(QPaintDevice * device, double pixel_ratio) : QPainter(device), Painter(pixel_ratio), use_pixmap_cache_(false) { - // new QPainter has default QPen: - current_color_ = guiApp->colorCache().get(Color_black); - current_ls_ = line_solid; - current_lw_ = thin_line; + // set cache correctly + current_color_ = pen().color(); + current_ls_ = pen().style() == Qt::DotLine ? line_onoffdash : line_solid; + current_lw_ = pen().width(); } From f6cbc08a7db3c2185b6eae768c026c58f577b0bc Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 27 Sep 2017 17:52:06 +0200 Subject: [PATCH 13/24] Fix bad refresh when changing zoom level Replace the tricky code in LFUN_SCREEN_FONT_UPDATE and replace it with proper use of DispatchResult flags. LFUN_BUFFER_ZOOM* does not need to call LFUN_SCREEN_FONT_UPDATE, since it already does everything that is required. (cherry picked from commit 9df59aac63bbb56d9d5f5ddcccfaa3ebace2f03d) --- src/frontends/qt4/GuiApplication.cpp | 9 +-------- src/frontends/qt4/GuiView.cpp | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index ee379ffb56..110b63ec21 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -1628,14 +1628,7 @@ void GuiApplication::dispatch(FuncRequest const & cmd, DispatchResult & dr) case LFUN_SCREEN_FONT_UPDATE: { // handle the screen font changes. d->font_loader_.update(); - // Backup current_view_ - GuiView * view = current_view_; - // Set current_view_ to zero to forbid GuiWorkArea::redraw() - // to skip the refresh. - current_view_ = 0; - theBufferList().changed(false); - // Restore current_view_ - current_view_ = view; + dr.screenUpdate(Update::Force | Update::FitCursor); break; } diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index 785abfdd9b..2aee2735d3 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -4152,7 +4152,7 @@ void GuiView::dispatch(FuncRequest const & cmd, DispatchResult & dr) // painting so we must reset it. QPixmapCache::clear(); guiApp->fontLoader().update(); - lyx::dispatch(FuncRequest(LFUN_SCREEN_FONT_UPDATE)); + dr.screenUpdate(Update::Force | Update::FitCursor); break; } From 6bd158dcba851beb4c22f8aa79a572b21f1a16d0 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 29 Sep 2017 10:29:39 +0200 Subject: [PATCH 14/24] Improve the logic of caret repainting For some reason the existing code only considered the bottom row that contained the cursor. There is no need for that, and actually it caused painting problems in nested insets. Tweak the logic of repaint_caret_row_ a bit: there is no need for repainting when there is currently no caret on screen. (cherry picked from commit 764a153c69bb9b46a6e6872ce48e06f5f867cc53) --- src/BufferView.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 58e4f8640f..53d374f07f 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -3057,15 +3057,15 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) int const y = tm.first().second->position(); PainterInfo pi(this, pain); - CursorSlice const & bottomSlice = d->cursor_.bottom(); - /** A repaint of the previous cursor row is needed if - * 1/ the caret will be painted and is is not the same than the - * already painted one; - * 2/ the caret will not be painted, but there is already one on - * screen. + /** A repaint of the previous caret row is needed if there is + * caret painted on screen and either + * 1/ a new caret has to be painted at a place different from + * the existing one; + * 2/ there is no need for a caret anymore. */ - d->repaint_caret_row_ = (paint_caret && bottomSlice != d->caret_slice_) - || (! paint_caret && !d->caret_slice_.empty()); + d->repaint_caret_row_ = !d->caret_slice_.empty() && + ((paint_caret && d->cursor_.top() != d->caret_slice_) + || ! paint_caret); // Check whether the row where the cursor lives needs to be scrolled. // Update the drawing strategy if needed. @@ -3151,7 +3151,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) // Remember what has just been done for the next draw() step if (paint_caret) - d->caret_slice_ = bottomSlice; + d->caret_slice_ = d->cursor_.top(); else d->caret_slice_ = CursorSlice(); } From 2b1b33189dc4113d2744cb9a68e691fbca4b884a Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Mon, 2 Oct 2017 17:07:31 +0200 Subject: [PATCH 15/24] Create new method GuiWorkArea::Private:::updateCaretGeometry This replaces a showCaret/hideCaret pair and avoids an update. Also remove an update() call in resizeBufferView: is is not necessary since we are already in a pintEvent handler. (cherry picked from commit add342d088c1b65343234576a35e567507fb2d49) --- src/frontends/qt4/GuiWorkArea.cpp | 28 ++++++++++++------------- src/frontends/qt4/GuiWorkArea_Private.h | 6 ++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index f4380da5e8..c4463e3d3b 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -469,14 +469,12 @@ void GuiWorkArea::scheduleRedraw(bool update_metrics) // update caret position, because otherwise it has to wait until // the blinking interval is over - if (d->caret_visible_) { - d->hideCaret(); - d->showCaret(); - } + d->updateCaretGeometry(); LYXERR(Debug::WORKAREA, "WorkArea::redraw screen"); viewport()->update(); + /// FIXME: is this still true now that paintEvent does the actual painting? /// \warning: scrollbar updating *must* be done after the BufferView is drawn /// because \c BufferView::updateScrollbar() is called in \c BufferView::draw(). d->updateScrollbar(); @@ -574,7 +572,7 @@ void GuiWorkArea::Private::resizeBufferView() buffer_view_->resize(p->viewport()->width(), p->viewport()->height()); if (caret_in_view) buffer_view_->scrollToCursor(); - p->viewport()->update(); + updateCaretGeometry(); // Update scrollbars which might have changed due different // BufferView dimension. This is especially important when the @@ -592,11 +590,8 @@ void GuiWorkArea::Private::resizeBufferView() } -void GuiWorkArea::Private::showCaret() +void GuiWorkArea::Private::updateCaretGeometry() { - if (caret_visible_) - return; - Point point; int h = 0; buffer_view_->caretPosAndHeight(point, h); @@ -631,7 +626,15 @@ void GuiWorkArea::Private::showCaret() point.x_ -= buffer_view_->horizScrollOffset(); caret_->update(point.x_, point.y_, h, l_shape, isrtl, completable); +} + +void GuiWorkArea::Private::showCaret() +{ + if (caret_visible_) + return; + + updateCaretGeometry(); p->viewport()->update(caret_->rect()); } @@ -1243,13 +1246,8 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); - if (d->need_resize_ || pixelRatio() != d->last_pixel_ratio_) { + if (d->need_resize_ || pixelRatio() != d->last_pixel_ratio_) d->resizeBufferView(); - if (d->caret_visible_) { - d->hideCaret(); - d->showCaret(); - } - } d->last_pixel_ratio_ = pixelRatio(); diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index ef2bb7e1df..de20397c71 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -85,10 +85,12 @@ struct GuiWorkArea::Private /// void dispatch(FuncRequest const & cmd0); - /// hide the visible caret, if it is visible - void hideCaret(); + /// recompute the shape and position of the caret + void updateCaretGeometry(); /// show the caret if it is not visible void showCaret(); + /// hide the caret if it is visible + void hideCaret(); /// Set the range and value of the scrollbar and connect to its valueChanged /// signal. void updateScrollbar(); From 4ecbff00192fb097432bc2d0604c6c8d97aa883a Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 11 Oct 2017 18:00:48 +0200 Subject: [PATCH 16/24] Allow multiple calls to processUpdateFlags before redraw The goal of this commit is to ensure that a processUpdateFlags call that requires no redraw will not override a previous one that did require a redraw. To this end, the semantics of the flag argument is now different: its value is now OR'ed with a private update_flags_ variable. This variable is only reset after the buffer view has actually been redrawn. A new Update::ForceRedraw flag has been added. It requires a full redraw but no metrics computation. It is not used in the main code (yet), but avoids to compute metrics repeatedly in consecutive processUpdateFlags calls. The process is now as follows: - if flags is just None, return immediately, there is nothing to do. - the Force flag is honored (full metrics computation) and replaced with ForceDraw. - the FitCursor flag is honored and removed from the flags. - the SinglePar update is added if ForceDraw is not in flags and only the current par has been modified. The remaining flags are only then added to the BufferView update flags, and the update strategy is computed for the next paint event. Finally the dubious call to updateMacros in updateMetrics has been removed for performance reasons. (cherry picked from commit 8d8988de475bf2055f253823e53fd5627be5de78) --- development/PAINTING_ANALYSIS | 21 ++-- src/BufferView.cpp | 196 ++++++++++++++++++---------------- src/BufferView.h | 9 +- src/TextMetrics.cpp | 15 ++- src/update_flags.h | 14 ++- 5 files changed, 141 insertions(+), 114 deletions(-) diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS index f734edb3b0..ec3566e06c 100644 --- a/development/PAINTING_ANALYSIS +++ b/development/PAINTING_ANALYSIS @@ -60,12 +60,6 @@ cursor. * Clean-up of drawing code -The goal is to make painting with drawing disable fast enough that it -can be used after every metrics computation. Then we can separate real -drawing from metrics. - -Other changes are only clean-ups. - ** When a paragraph ends with a newline, compute correctly the height of the extra row. ** Merging bv::updateMetrics and tm::metrics @@ -76,7 +70,7 @@ insets. We should re-use the bv::updateMetrics logic: + transfer all the logic of bv::updateMetrics to tm. + Main InsetText should not be special. -The difficuly for a tall table cell for example, is that it may be +The difficulty for a tall table cell for example, is that it may be necessary to break the whole contents to know the width of the cell. @@ -113,11 +107,19 @@ DecorationUpdate). It triggers a recomputation of the metrics when either: existing metrics. Note that the Update::SinglePar flag is *never* taken into account. +If a computation of metrics has taken place, Force is removed from the +flags and ForceDraw is added instead. + +It is OK to call processUptateFlags several times before an update. In +this case, the effects are cumulative.processUpdateFlags execute the +metrics-related actions, but defers the actual drawing to the next +paint event. + The screen is drawn (with appropriate update strategy), except when update flag is Update::None. -** Metrics computation +** Metrics computation (and nodraw drawing phase) This is triggered by bv::updateMetrics, which calls tm::redoParagraph for all visible paragraphs. Some Paragraphs above or below the screen (needed @@ -127,6 +129,9 @@ tm::redoParagraph will call Inset::metrics for each inset. In the case of text insets, this will invoke recursively tm::metrics, which redoes all the paragraphs of the inset. +At the end of the function, bv::updatePosCache is called. It triggers +a repaint of the document with a NullPainter (a painter that does +nothing). This has the effect of caching all insets positions. ** Drawing the work area. diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 53d374f07f..4e2428d7d7 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -228,7 +228,8 @@ enum ScreenUpdateStrategy { struct BufferView::Private { - Private(BufferView & bv) : update_strategy_(NoScreenUpdate), + Private(BufferView & bv) : update_strategy_(FullScreenUpdate), + update_flags_(Update::Force), wh_(0), cursor_(bv), anchor_pit_(0), anchor_ypos_(0), inlineCompletionUniqueChars_(0), @@ -245,6 +246,8 @@ struct BufferView::Private /// ScreenUpdateStrategy update_strategy_; /// + Update::flags update_flags_; + /// CoordCache coord_cache_; /// Estimated average par height for scrollbar. @@ -445,79 +448,85 @@ bool BufferView::needsFitCursor() const } +namespace { + +// this is for debugging only. +string flagsAsString(Update::flags flags) +{ + if (flags == Update::None) + return "None "; + return string((flags & Update::FitCursor) ? "FitCursor " : "") + + ((flags & Update::Force) ? "Force " : "") + + ((flags & Update::ForceDraw) ? "ForceDraw " : "") + + ((flags & Update::SinglePar) ? "SinglePar " : ""); +} + +} + void BufferView::processUpdateFlags(Update::flags flags) { - // This is close to a hot-path. - LYXERR(Debug::PAINTING, "BufferView::processUpdateFlags()" - << "[fitcursor = " << (flags & Update::FitCursor) - << ", forceupdate = " << (flags & Update::Force) - << ", singlepar = " << (flags & Update::SinglePar) - << "] buffer: " << &buffer_); - - // FIXME Does this really need doing here? It's done in updateBuffer, and - // if the Buffer doesn't need updating, then do the macros? - buffer_.updateMacros(); - - // Now do the first drawing step if needed. This consists on updating - // the CoordCache in updateMetrics(). - // The second drawing step is done in WorkArea::redraw() if needed. - // FIXME: is this still true now that Buffer::changed() is used all over? + LYXERR(Debug::PAINTING, "BufferView::processUpdateFlags( " + << flagsAsString(flags) << ") buffer: " << &buffer_); // Case when no explicit update is requested. - if (!flags) { + if (flags == Update::None) + return; + + // SinglePar is ignored for now (this should probably change). We + // set it ourselves below, at the price of always rebreaking the + // paragraph at cursor. This can be expensive for large tables. + flags = flags & ~Update::SinglePar; + + // First check whether the metrics and inset positions should be updated + if (flags & Update::Force) { + // This will update the CoordCache items and replace Force + // with ForceDraw in flags. + updateMetrics(flags); + } + + // Then make sure that the screen contains the cursor if needed + if (flags & Update::FitCursor) { + if (needsFitCursor()) { + scrollToCursor(d->cursor_, false); + // Metrics have to be recomputed (maybe again) + updateMetrics(flags); + } + flags = flags & ~Update::FitCursor; + } + + // Finally detect whether we can only repaint a single paragraph + if (!(flags & Update::ForceDraw)) { + if (singleParUpdate()) + flags = flags | Update::SinglePar; + else + updateMetrics(flags); + } + + // Add flags to the the update flags. These will be reset to None + // after the redraw is actually done + d->update_flags_ = d->update_flags_ | flags; + LYXERR(Debug::PAINTING, "Cumulative flags: " << flagsAsString(flags)); + + // Now compute the update strategy + // Possibly values in flag are None, Decoration, ForceDraw + LATTEST((d->update_flags_ & ~(Update::None | Update::SinglePar + | Update::Decoration | Update::ForceDraw)) == 0); + + if (d->update_flags_ & Update::ForceDraw) + d->update_strategy_ = FullScreenUpdate; + else if (d->update_flags_ & Update::Decoration) + d->update_strategy_ = DecorationUpdate; + else if (d->update_flags_ & Update::SinglePar) + d->update_strategy_ = SingleParUpdate; + else { // no need to redraw anything. d->update_strategy_ = NoScreenUpdate; - return; - } - - if (flags == Update::Decoration) { - d->update_strategy_ = DecorationUpdate; - buffer_.changed(false); - return; - } - - if (flags == Update::FitCursor - || flags == (Update::Decoration | Update::FitCursor)) { - // tell the frontend to update the screen if needed. - if (needsFitCursor()) { - showCursor(); - return; - } - if (flags & Update::Decoration) { - d->update_strategy_ = DecorationUpdate; - buffer_.changed(false); - return; - } - // no screen update is needed in principle, but this - // could change if cursor row needs horizontal scrolling. - d->update_strategy_ = NoScreenUpdate; - buffer_.changed(false); - return; - } - - bool const full_metrics = flags & Update::Force || !singleParUpdate(); - - if (full_metrics) - // We have to update the full screen metrics. - updateMetrics(); - - if (!(flags & Update::FitCursor)) { - // Nothing to do anymore. Trigger a redraw and return - buffer_.changed(false); - return; - } - - // updateMetrics() does not update paragraph position - // This is done at draw() time. So we need a redraw! - buffer_.changed(false); - - if (needsFitCursor()) { - // The cursor is off screen so ensure it is visible. - // refresh it: - showCursor(); } updateHoveredInset(); + + // Trigger a redraw. + buffer_.changed(false); } @@ -632,8 +641,7 @@ void BufferView::scrollDocView(int const value, bool update) // If the offset is less than 2 screen height, prefer to scroll instead. if (abs(value) <= 2 * height_) { d->anchor_ypos_ -= value; - buffer_.changed(true); - updateHoveredInset(); + processUpdateFlags(Update::Force); return; } @@ -830,12 +838,7 @@ bool BufferView::moveToPosition(pit_type bottom_pit, pos_type bottom_pos, d->cursor_.setCurrentFont(); // Do not forget to reset the anchor (see #9912) d->cursor_.resetAnchor(); - // To center the screen on this new position we need the - // paragraph position which is computed at draw() time. - // So we need a redraw! - buffer_.changed(false); - if (needsFitCursor()) - showCursor(); + processUpdateFlags(Update::FitCursor); } return success; @@ -877,19 +880,15 @@ void BufferView::showCursor() void BufferView::showCursor(DocIterator const & dit, bool recenter, bool update) { - if (scrollToCursor(dit, recenter) && update) { - buffer_.changed(true); - updateHoveredInset(); - } + if (scrollToCursor(dit, recenter) && update) + processUpdateFlags(Update::Force); } void BufferView::scrollToCursor() { - if (scrollToCursor(d->cursor_, false)) { - buffer_.changed(true); - updateHoveredInset(); - } + if (scrollToCursor(d->cursor_, false)) + processUpdateFlags(Update::Force); } @@ -1715,8 +1714,8 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) bool const in_texted = cur.inTexted(); cur.setCursor(doc_iterator_begin(cur.buffer())); cur.selHandle(false); - buffer_.changed(true); - updateHoveredInset(); + // Force an immediate computation of metrics because we need it below + processUpdateFlags(Update::Force); d->text_metrics_[&buffer_.text()].editXY(cur, p.x_, p.y_, true, act == LFUN_SCREEN_UP); @@ -1750,8 +1749,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) if (scroll_value) scroll(scroll_step * scroll_value); } - buffer_.changed(true); - updateHoveredInset(); + dr.screenUpdate(Update::ForceDraw); dr.forceBufferUpdate(); break; } @@ -2646,7 +2644,6 @@ bool BufferView::singleParUpdate() return false; tm.updatePosCache(bottom_pit); - d->update_strategy_ = SingleParUpdate; LYXERR(Debug::PAINTING, "\ny1: " << pm.position() - pm.ascent() << " y2: " << pm.position() + pm.descent() @@ -2657,6 +2654,13 @@ bool BufferView::singleParUpdate() void BufferView::updateMetrics() +{ + updateMetrics(d->update_flags_); + d->update_strategy_ = FullScreenUpdate; +} + + +void BufferView::updateMetrics(Update::flags & update_flags) { if (height_ == 0 || width_ == 0) return; @@ -2741,7 +2745,8 @@ void BufferView::updateMetrics() << " pit1 = " << pit1 << " pit2 = " << pit2); - d->update_strategy_ = FullScreenUpdate; + // metrics is done, full drawing is necessary now + update_flags = (update_flags & ~Update::Force) | Update::ForceDraw; // Now update the positions of insets in the cache. updatePosCache(); @@ -3050,8 +3055,8 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) { if (height_ == 0 || width_ == 0) return; - LYXERR(Debug::PAINTING, "\t\t*** START DRAWING ***"); - + LYXERR(Debug::PAINTING, (pain.isNull() ? "\t\t--- START NODRAW ---" + : "\t\t*** START DRAWING ***")); Text & text = buffer_.text(); TextMetrics const & tm = d->text_metrics_[&text]; int const y = tm.first().second->position(); @@ -3130,7 +3135,8 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) } break; } - LYXERR(Debug::PAINTING, "\n\t\t*** END DRAWING ***"); + LYXERR(Debug::PAINTING, (pain.isNull() ? "\t\t --- END NODRAW ---" + : "\t\t *** END DRAWING ***")); // The scrollbar needs an update. updateScrollbar(); @@ -3141,13 +3147,19 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) 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 + || d->anchor_ypos_ != pm.position()) + LYXERR(Debug::PAINTING, "Found new anchor pit = " << d->anchor_pit_ + << " anchor ypos = " << d->anchor_ypos_); d->anchor_pit_ = pit; d->anchor_ypos_ = pm.position(); break; } } - LYXERR(Debug::PAINTING, "Found new anchor pit = " << d->anchor_pit_ - << " anchor ypos = " << d->anchor_ypos_); + if (!pain.isNull()) { + // reset the update flags, everything has been done + d->update_flags_ = Update::None; + } // Remember what has just been done for the next draw() step if (paint_caret) diff --git a/src/BufferView.h b/src/BufferView.h index a5222599f2..3cbf22a8ba 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -120,11 +120,12 @@ public: /// \return true if the BufferView is at the bottom of the document. bool isBottomScreen() const; - /// perform pending metrics updates. - /** \c Update::FitCursor means first to do a FitCursor, and to + /// Add \p flags to current update flags and trigger an update. + /* If this method is invoked several times before the update + * actually takes place, the effect is cumulative. + * \c Update::FitCursor means first to do a FitCursor, and to * force an update if screen position changes. * \c Update::Force means to force an update in any case. - * \retval true if a screen redraw is needed */ void processUpdateFlags(Update::flags flags); @@ -367,6 +368,8 @@ private: /// Update current paragraph metrics. /// \return true if no further update is needed. bool singleParUpdate(); + /// do the work for the public updateMetrics() + void updateMetrics(Update::flags & update_flags); // Set the row on which the cursor lives. void setCurrentRowSlice(CursorSlice const & rowSlice); diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 5eb85bf118..325b0b9c5a 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -1920,14 +1920,13 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // Instrumentation for testing row cache (see also // 12 lines lower): if (lyxerr.debugging(Debug::PAINTING) - && (row.selection() || pi.full_repaint || row_has_changed)) { - string const foreword = text_->isMainText() ? - "main text redraw " : "inset text redraw: "; - LYXERR(Debug::PAINTING, foreword << "pit=" << pit << " row=" << i - << " row_selection=" << row.selection() - << " full_repaint=" << pi.full_repaint - << " row_has_changed=" << row_has_changed - << " null painter=" << pi.pain.isNull()); + && (row.selection() || pi.full_repaint || row_has_changed)) { + string const foreword = text_->isMainText() ? "main text redraw " + : "inset text redraw: "; + LYXERR0(foreword << "pit=" << pit << " row=" << i + << (row.selection() ? " row_selection": "") + << (pi.full_repaint ? " full_repaint" : "") + << (row_has_changed ? " row_has_changed" : "")); } // Backup full_repaint status and force full repaint diff --git a/src/update_flags.h b/src/update_flags.h index 3e877c1ca1..a40e88c556 100644 --- a/src/update_flags.h +++ b/src/update_flags.h @@ -21,13 +21,16 @@ namespace Update { /// Recenter the screen around the cursor if is found outside the /// visible area. FitCursor = 1, - /// Force a full screen metrics update. + /// Force a full screen metrics update and a full draw. Force = 2, + /// Force a full redraw (but no metrics computations) + ForceDraw = 4, /// Try to rebreak only the current paragraph metrics. - SinglePar = 4, + /// (currently ignored!) + SinglePar = 8, /// Only the inset decorations need to be redrawn, no text metrics /// update is needed. - Decoration = 8 + Decoration = 16 }; inline flags operator|(flags const f, flags const g) @@ -40,6 +43,11 @@ inline flags operator&(flags const f, flags const g) return static_cast(int(f) & int(g)); } +inline flags operator~(flags const f) +{ + return static_cast(~int(f)); +} + } // namespace Update } // namespace lyx From d2e15bd1893e288f3177dbec15b82b723bfca6a6 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 11 Nov 2017 11:57:39 +0100 Subject: [PATCH 17/24] Store change bar information in row element It is wrong to compute this at paint time. In general, painting a row should not require any access to a paragraph object, but we are far from there now. (cherry picked from commit 4858bb3bb68aac142815b530a017e3776d1c7c11) --- src/Row.cpp | 5 ++++- src/Row.h | 7 +++++++ src/RowPainter.cpp | 12 ------------ src/TextMetrics.cpp | 7 ++++++- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/Row.cpp b/src/Row.cpp index db1bd2883c..20ff63e22a 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -164,7 +164,8 @@ Row::Row() begin_margin_sel(false), end_margin_sel(false), changed_(false), crc_(0), pit_(0), pos_(0), end_(0), - right_boundary_(false), flushed_(false), rtl_(false) + right_boundary_(false), flushed_(false), rtl_(false), + changebar_(false) {} @@ -371,6 +372,8 @@ void Row::finalizeLast() if (elt.final) return; elt.final = true; + if (elt.change.changed()) + changebar_ = true; if (elt.type == STRING) { dim_.wid -= elt.dim.wid; diff --git a/src/Row.h b/src/Row.h index 498fd07d7f..a2e77fbfb4 100644 --- a/src/Row.h +++ b/src/Row.h @@ -266,6 +266,11 @@ public: void reverseRTL(bool rtl_par); /// bool isRTL() const { return rtl_; } + /// + bool needsChangeBar() const { return changebar_; } + /// + void needsChangeBar(bool ncb) { changebar_ = ncb; } + /// Find row element that contains \c pos, and compute x offset. const_iterator const findElement(pos_type pos, bool boundary, double & x) const; @@ -326,6 +331,8 @@ private: Dimension dim_; /// true when this row lives in a right-to-left paragraph bool rtl_; + /// true when a changebar should be drawn in the margin + bool changebar_; }; diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp index c4b9034fb4..38bcefadad 100644 --- a/src/RowPainter.cpp +++ b/src/RowPainter.cpp @@ -247,18 +247,6 @@ void RowPainter::paintChange(Row::Element const & e) const void RowPainter::paintChangeBar() const { - pos_type const start = row_.pos(); - pos_type end = row_.endpos(); - - if (par_.size() == end) { - // this is the last row of the paragraph; - // thus, we must also consider the imaginary end-of-par character - end++; - } - - if (start == end || !par_.isChanged(start, end)) - return; - int const height = tm_.isLastRow(row_) ? row_.ascent() : row_.height(); diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 325b0b9c5a..cd33f7f190 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -964,6 +964,10 @@ bool TextMetrics::breakRow(Row & row, int const right_margin) const row.addVirtual(end, docstring(1, char_type(0x00B6)), f, Change()); } + // Is there a end-of-paragaph change? + if (i == end && par.lookupChange(end).changed() && !need_new_row) + row.needsChangeBar(true); + // if the row is too large, try to cut at last separator. In case // of success, reset indication that the row was broken abruptly. int const next_width = max_width_ - leftMargin(row.pit(), row.endpos()) @@ -1937,7 +1941,8 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const rp.paintSelection(); rp.paintAppendix(); rp.paintDepthBar(); - rp.paintChangeBar(); + if (row.needsChangeBar()) + rp.paintChangeBar(); if (i == 0 && !row.isRTL()) rp.paintFirst(); if (i == nrows - 1 && row.isRTL()) From a71610b9d72d346ac694518d6dab634f02247e2b Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 11 Nov 2017 12:40:39 +0100 Subject: [PATCH 18/24] 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, "."); From fa4fc6fc4d97f5c3d3e8ba44ad8b52f750293623 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Thu, 23 Nov 2017 15:38:17 +0100 Subject: [PATCH 19/24] Avoid some caret ghosts When the caret is at end of row, if may happen that it is drawn after the end of the row. In this case caret blinking will not work properly. This patch extends the row background on the left and right by Inset::TEXT_TO_INSET_OFFSET. This is only a hack that will not work if the caret has a ridiculous width like 6. Additionally, introduce some (disabled) debug code that numbers the rows on screen by painting order. Finally, make the code that detects whether the caret was in a given row more precise (take boundary into account). Fixes (mostly, see above) bug #10797. (cherry picked from commit e64ea3576c4534fc647a74d1c9f5e67db39ef783) --- src/BufferView.cpp | 8 +++++--- src/TextMetrics.cpp | 27 +++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 4e2428d7d7..acde1ceef3 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2979,7 +2979,7 @@ namespace { bool sliceInRow(CursorSlice const & cs, Text const * text, Row const & row) { return !cs.empty() && cs.text() == text && cs.pit() == row.pit() - && row.pos() <= cs.pos() && cs.pos() <= row.endpos(); + && row.pos() <= cs.pos() && cs.pos() < row.endpos(); } } @@ -3162,9 +3162,11 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) } // Remember what has just been done for the next draw() step - if (paint_caret) + if (paint_caret) { d->caret_slice_ = d->cursor_.top(); - else + if (d->cursor_.boundary()) + --d->caret_slice_.pos(); + } else d->caret_slice_ = CursorSlice(); } diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 1f4516f1af..42637d7899 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -45,6 +45,7 @@ #include "frontends/Painter.h" #include "frontends/NullPainter.h" +#include "support/convert.h" #include "support/debug.h" #include "support/lassert.h" @@ -1917,8 +1918,16 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const LYXERR(Debug::PAINTING, "Clear rect@(" << max(row_x, 0) << ", " << y - row.ascent() << ")=" << width() << " x " << row.height()); - pi.pain.fillRectangle(max(row_x, 0), y - row.ascent(), - width(), row.height(), pi.background_color); + // FIXME: this is a hack. We know that at least this + // amount of pixels can be cleared on right and left. + // Doing so gets rid of caret ghosts when the cursor is at + // the begining/end of row. However, it will not work if + // the caret has a ridiculous width like 6. (see ticket + // #10797) + pi.pain.fillRectangle(max(row_x, 0) - Inset::TEXT_TO_INSET_OFFSET, + y - row.ascent(), + width() + 2 * Inset::TEXT_TO_INSET_OFFSET, + row.height(), pi.background_color); } // Instrumentation for testing row cache (see also @@ -1956,6 +1965,20 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const row_x + row.right_x() > bv_->workWidth()); y += row.descent(); +#if 0 + // This debug code shows on screen which rows are repainted. + // FIXME: since the updates related to caret blinking restrict + // the painter to a small rectangle, the numbers are not + // updated when this happens. Change the code in + // GuiWorkArea::Private::show/hideCaret if this is important. + static int count = 0; + ++count; + FontInfo fi(sane_font); + fi.setSize(FONT_SIZE_TINY); + fi.setColor(Color_red); + pi.pain.text(row_x, y, convert(count), fi); +#endif + // Restore full_repaint status. pi.full_repaint = tmp; From 9c8e3df86bb3140435e022d67cd7b6f7f8575c71 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 24 Nov 2017 23:36:28 +0100 Subject: [PATCH 20/24] Fixup ac4bcb12 Cursor at end of paragraph should be treated as if boundary was on. (cherry picked from commit d01dd54bf14167e880c1d0a6382b87e622c2c2ed) --- src/BufferView.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index acde1ceef3..23d0055221 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -3164,7 +3164,8 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) // Remember what has just been done for the next draw() step if (paint_caret) { d->caret_slice_ = d->cursor_.top(); - if (d->cursor_.boundary()) + if (d->cursor_.boundary() + || d->cursor_.top().pos() == d->cursor_.top().lastpos()) --d->caret_slice_.pos(); } else d->caret_slice_ = CursorSlice(); From 8af3077753381238e1fb3e21d65017b94778a47c Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 29 Nov 2017 11:16:09 +0100 Subject: [PATCH 21/24] Make sure that rows are repainted when they get (un)selected The bug is the following: when selecting several paragraphs quickly enough, some rows do not get selected.This is a consequence of the removal of row crc, which lead to not taking into account the selection status of the row in the decision to repaint. The solution chosen here is to add a Row::change() helper function to modify row members. This will set the Row changed status whenever the value of the member changes. (cherry picked from commit ae60749f89fb4ee0fca05ac75979d434f6b0401d) --- src/Row.cpp | 16 ++++++++-------- src/Row.h | 23 +++++++++++++++++++++++ src/TextMetrics.cpp | 4 ++-- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/Row.cpp b/src/Row.cpp index ad492c1a7a..3fb87bd7e1 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -203,8 +203,8 @@ void Row::setSelectionAndMargins(DocIterator const & beg, setSelection(beg.pos(), end.pos()); if (selection()) { - end_margin_sel = isMarginSelected(false, beg, end); - begin_margin_sel = isMarginSelected(true, beg, end); + change(end_margin_sel, isMarginSelected(false, beg, end)); + change(begin_margin_sel, isMarginSelected(true, beg, end)); } } @@ -212,18 +212,18 @@ void Row::setSelectionAndMargins(DocIterator const & beg, void Row::setSelection(pos_type beg, pos_type end) const { if (pos_ >= beg && pos_ <= end) - sel_beg = pos_; + change(sel_beg, pos_); else if (beg > pos_ && beg <= end_) - sel_beg = beg; + change(sel_beg, beg); else - sel_beg = -1; + change(sel_beg, -1); if (end_ >= beg && end_ <= end) - sel_end = end_; + change(sel_end,end_); else if (end < end_ && end >= pos_) - sel_end = end; + change(sel_end, end); else - sel_end = -1; + change(sel_end, -1); } diff --git a/src/Row.h b/src/Row.h index a1fedd7e5d..49513d32b9 100644 --- a/src/Row.h +++ b/src/Row.h @@ -136,6 +136,29 @@ public: /// Row(); + /** + * Helper function: set variable \c var to value \c val, and mark + * row as changed is the values were different. This is intended + * for use when changing members of the row object. + */ + template + void change(T1 & var, T2 const val) { + if (var != val) + changed(true); + var = val; + } + /** + * Helper function: set variable \c var to value \c val, and mark + * row as changed is the values were different. This is intended + * for use when changing members of the row object. + * This is the const version, useful for mutable members. + */ + template + void change(T1 & var, T2 const val) const { + if (var != val) + changed(true); + var = val; + } /// bool changed() const { return changed_; } /// diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index 42637d7899..e7bd6190e2 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -1884,9 +1884,9 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const // whether this row is the first or last and update the margins. if (row.selection()) { if (row.sel_beg == 0) - row.begin_margin_sel = sel_beg.pit() < pit; + row.change(row.begin_margin_sel, sel_beg.pit() < pit); if (row.sel_end == sel_end_par.lastpos()) - row.end_margin_sel = sel_end.pit() > pit; + row.change(row.end_margin_sel, sel_end.pit() > pit); } // has row changed since last paint? From c6771569384edc1e100d427365d6a4976bc5bd95 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 25 Nov 2017 12:31:11 +0100 Subject: [PATCH 22/24] Use a backing store on macOS Qt on macOS does not respect the Qt::WA_OpaquePaintEvent attribute and clears the widget backing store at each update. Therefore, we use our own backing store in this case. This restores a simplified version of the code that was removed at 06253dfe. (cherry picked from commit 2316435f2fd2da28c70e1b251b852d3bf6d8011a) --- src/frontends/qt4/GuiWorkArea.cpp | 11 +++++-- src/frontends/qt4/GuiWorkArea_Private.h | 44 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index c4463e3d3b..6df7e4e290 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -306,6 +306,7 @@ void GuiWorkArea::init() generateSyntheticMouseEvent(); }); + d->resetScreen(); // With Qt4.5 a mouse event will happen before the first paint event // so make sure that the buffer view has an up to date metrics. d->buffer_view_->resize(viewport()->width(), viewport()->height()); @@ -1246,12 +1247,15 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) // LYXERR(Debug::PAINTING, "paintEvent begin: x: " << rc.x() // << " y: " << rc.y() << " w: " << rc.width() << " h: " << rc.height()); - if (d->need_resize_ || pixelRatio() != d->last_pixel_ratio_) + if (d->need_resize_ || pixelRatio() != d->last_pixel_ratio_) { + d->resetScreen(); d->resizeBufferView(); + } d->last_pixel_ratio_ = pixelRatio(); - GuiPainter pain(viewport(), pixelRatio()); + GuiPainter pain(d->screenDevice(), pixelRatio()); + d->buffer_view_->draw(pain, d->caret_visible_); // The preedit text, if needed @@ -1260,6 +1264,9 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev) // and the caret if (d->caret_visible_) d->caret_->draw(pain); + + d->updateScreen(ev->rect()); + ev->accept(); } diff --git a/src/frontends/qt4/GuiWorkArea_Private.h b/src/frontends/qt4/GuiWorkArea_Private.h index de20397c71..83012fa99f 100644 --- a/src/frontends/qt4/GuiWorkArea_Private.h +++ b/src/frontends/qt4/GuiWorkArea_Private.h @@ -20,6 +20,14 @@ #include #include +#ifdef Q_OS_MAC +/* Qt on macOS does not respect the Qt::WA_OpaquePaintEvent attribute + * and resets the widget backing store at each update. Therefore, we + * use our own backing store in this case */ +#define LYX_BACKINGSTORE 1 +#include +#endif + namespace lyx { class Buffer; @@ -99,6 +107,38 @@ struct GuiWorkArea::Private void paintPreeditText(GuiPainter & pain); + void resetScreen() { +#ifdef LYX_BACKINGSTORE + int const pr = p->pixelRatio(); + screen_ = QImage(static_cast(pr * p->viewport()->width()), + static_cast(pr * p->viewport()->height()), + QImage::Format_ARGB32_Premultiplied); +# if QT_VERSION >= 0x050000 + screen_.setDevicePixelRatio(pr); +# endif +#endif + } + + QPaintDevice * screenDevice() { +#ifdef LYX_BACKINGSTORE + return &screen_; +#else + return p->viewport(); +#endif + } + +#ifdef LYX_BACKINGSTORE + void updateScreen(QRectF const & rc) { + QPainter qpain(p->viewport()); + double const pr = p->pixelRatio(); + QRectF const rcs = QRectF(rc.x() * pr, rc.y() * pr, + rc.width() * pr, rc.height() * pr); + qpain.drawImage(rc, screen_, rcs); + } +#else + void updateScreen(QRectF const & ) {} +#endif + /// GuiWorkArea * p; /// @@ -106,6 +146,10 @@ struct GuiWorkArea::Private /// GuiView * lyx_view_; +#ifdef LYX_BACKINGSTORE + /// + QImage screen_; +#endif /// CaretWidget * caret_; /// is the caret currently displayed From be35ba31bfff429330d628b848ecc7d65265590d Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Sat, 6 Jan 2018 20:46:06 +0100 Subject: [PATCH 23/24] Fix compilation in monolithic mode (cherry picked from commit 3aa10c8dbaf4813fb30d6e9487a5d50720c7affd) --- src/frontends/qt4/GuiApplication.cpp | 1 + src/frontends/qt4/GuiInclude.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index 110b63ec21..323e654a41 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -122,6 +122,7 @@ #include #include #include +#undef CursorShape #undef None #elif defined(QPA_XCB) #include diff --git a/src/frontends/qt4/GuiInclude.cpp b/src/frontends/qt4/GuiInclude.cpp index aa2fc65725..edbf7a19c4 100644 --- a/src/frontends/qt4/GuiInclude.cpp +++ b/src/frontends/qt4/GuiInclude.cpp @@ -31,9 +31,9 @@ #include "insets/InsetListingsParams.h" #include "insets/InsetInclude.h" -#include #include #include +#include #include From 61c5769e3981825b5fef9aecf512679e94e0fc53 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Mon, 8 Jan 2018 11:49:40 +0100 Subject: [PATCH 24/24] Fix ghost caret This fixes a regression in e64ea357, where a test has been (badly) tightened to avoid that two consecutive rows may be redrawn to get rid of caret ghosts. The test prohibited empty rows from being redrawn. Moreover, improve the test of cursor boundary to avoid the case where cursor position is already 0. Fixes bug #10952. (cherry picked from commit 66c677b9463feb0687a8228603f86eddd4e859fd) --- src/BufferView.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 23d0055221..36ac46cdde 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2978,8 +2978,14 @@ namespace { bool sliceInRow(CursorSlice const & cs, Text const * text, Row const & row) { + /* The normal case is the last line. The previous line takes care + * of empty rows (e.g. empty paragraphs). Cursor boundary issues + * are taken care of when setting caret_slice_ in + * BufferView::draw. + */ return !cs.empty() && cs.text() == text && cs.pit() == row.pit() - && row.pos() <= cs.pos() && cs.pos() < row.endpos(); + && ((row.pos() == row.endpos() && row.pos() == cs.pos()) + || (row.pos() <= cs.pos() && cs.pos() < row.endpos())); } } @@ -3164,8 +3170,9 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) // Remember what has just been done for the next draw() step if (paint_caret) { d->caret_slice_ = d->cursor_.top(); - if (d->cursor_.boundary() - || d->cursor_.top().pos() == d->cursor_.top().lastpos()) + if (d->caret_slice_.pos() > 0 + && (d->cursor_.boundary() + || d->caret_slice_.pos() == d->caret_slice_.lastpos())) --d->caret_slice_.pos(); } else d->caret_slice_ = CursorSlice();