Le 09/12/2016 à 15:58, Jean-Marc Lasgouttes a écrit :
Le 07/12/2016 à 16:10, Jean-Marc Lasgouttes a écrit :
Le 07/12/2016 à 12:15, Jean-Marc Lasgouttes a écrit :
I'll post a new version to try soon.

Here is a patch to play with. It is not perfect, but I would be
interested to know whether it improves performance with Qt4.

I have improved a bit the patch (fixed a couple bugs)  and now it also
comes with built-in profiling.
Just comment-out the
#define DISABLE_PMPROF
at the beginning to enable profiling of the 3 main caches.

Here is what I get on User Guide by using PageDown over the whole file
(not very good for the cache)


Here is the same test with Qt5:

#pmprof# getTextLayout: 19.07usec, count=19115, total=364.61msec
   hit: 53%, 4.17usec, count=10135, total=42.25msec
  miss: 46%, 35.90usec, count=8980, total=322.35msec
#pmprof# breakAt: 42.10usec, count=3981, total=167.60msec
   hit: 69%, 3.44usec, count=2762, total=9.51msec
  miss: 30%, 129.69usec, count=1219, total=158.09msec
#pmprof# width: 6.09usec, count=153658, total=936.00msec
   hit: 89%, 0.47usec, count=137232, total=64.39msec
  miss: 10%, 53.06usec, count=16426, total=871.61msec

Note that these numbers are misleading. Indeed, since Qt5 does its own caching, the times in case of "miss" take in account the fact that there is a miss also in the lower level Qt caching. Therefore the miss times are artificially large.

To have a better view of the effect of the cache, one can run the same test with cache disabled (comment out #define CACHE_FONT_METRICS in updated patch in attachment).

#pmprof# getTextLayout: 17.09usec, count=19224, total=328.52msec
#pmprof# breakAt: 76.84usec, count=4074, total=313.06msec
#pmprof# width: 17.18usec, count=152237, total=2614.93msec

These numbers show that the effect is actually slightly negative for getTextLayout. We might want though to skip the caching of QTextLayout on Qt5 (moreover they can use a lot of memory). The two other ones seem good to keep.

I'd be interested to see other tests, especially on MacOS and Windows.

JMarc
>From bd638c0c3ced186b81d7161042b62c5439b5382b Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Tue, 5 Jul 2016 14:06:22 +0200
Subject: [PATCH] Add caching for the QTextLayout objects we use

The QTextLayout handling is terribly slow on Qt 4.8.7, but some
caching has been added in Qt5 that makes it much faster. For some
reason, it is not that slow with Qt 4.8.1.

This commit introduces some caching, which should probably only be
active for Qt 4. This caching only happens when CACHE_FONT_METRICS is
defined.

[NOTE: this version has profiling hooks, enabled by commenting out the line
  #define DISABLE_PMPROF
that should eventually be removed.]

Improve the caching of QTextLayout objects used for pos2x and x2pos
and use them for drawing too. We originally used a trivial caching
of the last QTextLayout, but now they are kept in a QCache.
Moreover, caching is enabled for these QTextLayout object (not sure
what this does).

This patch also adds some caching in the breakAt method, the only user of
QTextLayout which did not have some kind of caching already.

Finally the cached QTextLayout objects are also used by the painter
for drawing text.

For some weird reasons related to Argument-dependent look-up, the
qHash(docstring) function has to be defined in std namespace, since
lyx::docstring is actually std::basic_string<wchar_t>.
---
 src/frontends/qt4/GuiFontMetrics.cpp | 156 +++++++++++++++++++++++++----------
 src/frontends/qt4/GuiFontMetrics.h   |  33 +++++---
 src/frontends/qt4/GuiPainter.cpp     |  13 ++-
 src/support/convert.cpp              |   7 ++
 4 files changed, 151 insertions(+), 58 deletions(-)

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp
index d3c89f1..ff61481 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -21,11 +21,36 @@
 
 #include "insets/Inset.h"
 
+#include "support/convert.h"
 #include "support/lassert.h"
 
+//#define DISABLE_PMPROF
+#include "support/pmprof.h"
+
+#ifdef CACHE_FONT_METRICS
+#include <QByteArray>
+#endif
+
 using namespace std;
 using namespace lyx::support;
 
+#ifdef CACHE_FONT_METRICS
+namespace std {
+
+/*
+ * Argument-dependent lookup implies that this function shall be
+ * declared in the namespace of its argument. But this is std
+ * namespace, since lyx::docstring is just std::basic_string<wchar_t>.
+ */
+uint qHash(lyx::docstring const & s)
+{
+	return qHash(QByteArray(reinterpret_cast<char const *>(s.data()),
+	                        s.size() * sizeof(lyx::docstring::value_type)));
+}
+
+}
+#endif
+
 namespace lyx {
 namespace frontend {
 
@@ -51,11 +76,17 @@ inline QChar const ucs4_to_qchar(char_type const ucs4)
 } // anon namespace
 
 
-// Limit strwidth_cache_ size to 512kB of string data
+/*
+ * Limit (strwidth|breakat)_cache_ size to 512kB of string data.
+ * Limit qtextlayout_cache_ size to 500 elements (we do not know the
+ * size of the QTextLayout objects anyway).
+ * Note that all these numbers are arbitrary.
+ */
 GuiFontMetrics::GuiFontMetrics(QFont const & font)
-	: font_(font), metrics_(font, 0),
-	  strwidth_cache_(1 << 19),
-	  tl_cache_rtl_(false), tl_cache_wordspacing_(-1.0)
+	: font_(font), metrics_(font, 0)
+#ifdef CACHE_FONT_METRICS
+	, strwidth_cache_(1 << 19), breakat_cache_(1 << 19),  qtextlayout_cache_(500)
+#endif
 {
 }
 
@@ -140,14 +171,15 @@ int GuiFontMetrics::rbearing(char_type c) const
 
 int GuiFontMetrics::width(docstring const & s) const
 {
-	QByteArray qba =
-		QByteArray(reinterpret_cast<char const *>(s.data()),
-		           s.size() * sizeof(docstring::value_type));
-	int * pw = strwidth_cache_[qba];
+	PROFILE_THIS_BLOCK(width)
+#ifdef CACHE_FONT_METRICS
+	int * pw = strwidth_cache_[s];
 	if (pw)
 		return *pw;
 	// For some reason QMetrics::width returns a wrong value with Qt5
 	// int w = metrics_.width(toqstr(s));
+	PROFILE_CACHE_MISS(width)
+#endif
 	QTextLayout tl;
 	tl.setText(toqstr(s));
 	tl.setFont(font_);
@@ -155,8 +187,9 @@ int GuiFontMetrics::width(docstring const & s) const
 	QTextLine line = tl.createLine();
 	tl.endLayout();
 	int w = int(line.naturalTextWidth());
-
-	strwidth_cache_.insert(qba, new int(w), qba.size());
+#ifdef CACHE_FONT_METRICS
+	strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type));
+#endif
 	return w;
 }
 
@@ -179,26 +212,34 @@ int GuiFontMetrics::signedWidth(docstring const & s) const
 }
 
 
-QTextLayout const &
-GuiFontMetrics::getTextLayout(docstring const & s, QFont font,
-                              bool const rtl, double const wordspacing) const
+QTextLayout const *
+GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl,
+                              double const wordspacing) const
 {
-	if (s != tl_cache_s_ || font != tl_cache_font_ || rtl != tl_cache_rtl_
-	    || wordspacing != tl_cache_wordspacing_) {
-		tl_cache_.setText(toqstr(s));
-		font.setWordSpacing(wordspacing);
-		tl_cache_.setFont(font);
+	PROFILE_THIS_BLOCK(getTextLayout)
+	QTextLayout * ptl;
+#ifdef CACHE_FONT_METRICS
+	docstring const s_cache = s + (rtl ? "r" : "l") + convert<docstring>(wordspacing);
+	ptl = qtextlayout_cache_[s_cache];
+	if (!ptl) {
+		PROFILE_CACHE_MISS(getTextLayout)
+#endif
+		ptl = new QTextLayout();
+		ptl->setCacheEnabled(true);
+		ptl->setText(toqstr(s));
+		QFont copy = font_;
+		copy.setWordSpacing(wordspacing);
+		ptl->setFont(copy);
 		// Note that both setFlags and the enums are undocumented
-		tl_cache_.setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
-		tl_cache_.beginLayout();
-		tl_cache_.createLine();
-		tl_cache_.endLayout();
-		tl_cache_s_ = s;
-		tl_cache_font_ = font;
-		tl_cache_rtl_ = rtl;
-		tl_cache_wordspacing_ = wordspacing;
+		ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
+		ptl->beginLayout();
+		ptl->createLine();
+		ptl->endLayout();
+#ifdef CACHE_FONT_METRICS
+		qtextlayout_cache_.insert(s_cache, ptl);
 	}
-	return tl_cache_;
+#endif
+	return ptl;
 }
 
 
@@ -207,32 +248,32 @@ int GuiFontMetrics::pos2x(docstring const & s, int const pos, bool const rtl,
 {
 	if (pos <= 0)
 		return rtl ? width(s) : 0;
-	QTextLayout const & tl = getTextLayout(s, font_, rtl, wordspacing);
+	QTextLayout const * tl = getTextLayout(s, rtl, wordspacing);
 	/* Since QString is UTF-16 and docstring is UCS-4, the offsets may
 	 * not be the same when there are high-plan unicode characters
 	 * (bug #10443).
 	 */
 	int const qpos = toqstr(s.substr(0, pos)).length();
-	return static_cast<int>(tl.lineForTextPosition(qpos).cursorToX(qpos));
+	return static_cast<int>(tl->lineForTextPosition(qpos).cursorToX(qpos));
 }
 
 
 int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
                           double const wordspacing) const
 {
-	QTextLayout const & tl = getTextLayout(s, font_, rtl, wordspacing);
-	int const qpos = tl.lineForTextPosition(0).xToCursor(x);
+	QTextLayout const * tl = getTextLayout(s, rtl, wordspacing);
+	int const qpos = tl->lineForTextPosition(0).xToCursor(x);
 	// correct x value to the actual cursor position.
-	x = static_cast<int>(tl.lineForTextPosition(0).cursorToX(qpos));
+	x = static_cast<int>(tl->lineForTextPosition(0).cursorToX(qpos));
 	/* Since QString is UTF-16 and docstring is UCS-4, the offsets may
 	 * not be the same when there are high-plan unicode characters
 	 * (bug #10443).
 	 */
 #if QT_VERSION < 0x040801 || QT_VERSION >= 0x050100
-	return qstring_to_ucs4(tl.text().left(qpos)).length();
+	return qstring_to_ucs4(tl->text().left(qpos)).length();
 #else
 	/* Due to QTBUG-25536 in 4.8.1 <= Qt < 5.1.0, the string returned
-	 * by QString::toUcs4 (used by qstring_to_ucs4)may have wrong
+	 * by QString::toUcs4 (used by qstring_to_ucs4) may have wrong
 	 * length. We work around the problem by trying all docstring
 	 * positions until the right one is found. This is slow only if
 	 * there are many high-plane Unicode characters. It might be
@@ -269,10 +310,10 @@ int GuiFontMetrics::countExpanders(docstring const & str) const
 }
 
 
-bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const force) const
+pair<int, int> *
+GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
+                               bool const rtl, bool const force) const
 {
-	if (s.empty())
-		return false;
 	QTextLayout tl;
 	/* Qt will not break at a leading or trailing space, and we need
 	 * that sometimes, see http://www.lyx.org/trac/ticket/9921.
@@ -280,7 +321,7 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const
 	 * To work around the problem, we enclose the string between
 	 * zero-width characters so that the QTextLayout algorithm will
 	 * agree to break the text at these extremal spaces.
-	*/
+	 */
 	// Unicode character ZERO WIDTH NO-BREAK SPACE
 	QChar const zerow_nbsp(0xfeff);
 	QString qs = zerow_nbsp + toqstr(s) + zerow_nbsp;
@@ -314,8 +355,7 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const
 	tl.createLine();
 	tl.endLayout();
 	if ((force && line.textLength() == offset) || int(line.naturalTextWidth()) > x)
-		return false;
-	x = int(line.naturalTextWidth());
+		return new pair<int, int>(-1, -1);
 	/* Since QString is UTF-16 and docstring is UCS-4, the offsets may
 	 * not be the same when there are high-plan unicode characters
 	 * (bug #10443).
@@ -324,10 +364,10 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const
 	// The ending character zerow_nbsp has to be ignored if the line is complete.
 	int const qlen = line.textLength() - offset - (line.textLength() == qs.length());
 #if QT_VERSION < 0x040801 || QT_VERSION >= 0x050100
-	s = qstring_to_ucs4(qs.mid(offset, qlen));
+	int len = qstring_to_ucs4(qs.mid(offset, qlen)).length();
 #else
 	/* Due to QTBUG-25536 in 4.8.1 <= Qt < 5.1.0, the string returned
-	 * by QString::toUcs4 (used by qstring_to_ucs4)may have wrong
+	 * by QString::toUcs4 (used by qstring_to_ucs4) may have wrong
 	 * length. We work around the problem by trying all docstring
 	 * positions until the right one is found. This is slow only if
 	 * there are many high-plane Unicode characters. It might be
@@ -338,7 +378,36 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const
 	while (len >= 0 && toqstr(s.substr(0, len)).length() != qlen)
 		--len;
 	LASSERT(len > 0 || qlen == 0, /**/);
-	s = s.substr(0, len);
+#endif
+	// The -1 is here to account for the leading zerow_nbsp.
+	return new pair<int, int>(len, int(line.naturalTextWidth()));
+}
+
+
+bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const force) const
+{
+	PROFILE_THIS_BLOCK(breakAt)
+	if (s.empty())
+		return false;
+	pair<int, int> * pp;
+#ifdef CACHE_FONT_METRICS
+	docstring const s_cache = s + convert<docstring>(x) + (rtl ? "r" : "l") + (force ? "f" : "w");
+
+	pp = breakat_cache_[s_cache];
+	if (!pp) {
+		PROFILE_CACHE_MISS(breakAt)
+#endif
+		pp = breakAt_helper(s, x, rtl, force);
+#ifdef CACHE_FONT_METRICS
+		breakat_cache_.insert(s_cache, pp, s_cache.size() * sizeof(char_type));
+	}
+#endif
+	if (pp->first == -1)
+		return false;
+	s = s.substr(0, pp->first);
+	x = pp->second;
+#ifndef CACHE_FONT_METRICS
+	delete pp;
 #endif
 	return true;
 }
@@ -355,7 +424,6 @@ void GuiFontMetrics::rectText(docstring const & str,
 }
 
 
-
 void GuiFontMetrics::buttonText(docstring const & str,
 	int & w, int & ascent, int & descent) const
 {
diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h
index 9b4c2c4..1cdc901 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -12,12 +12,15 @@
 #ifndef GUI_FONT_METRICS_H
 #define GUI_FONT_METRICS_H
 
+#define CACHE_FONT_METRICS
+
 #include "frontends/FontMetrics.h"
 
 #include "support/docstring.h"
 
-#include <QByteArray>
+#ifdef CACHE_FONT_METRICS
 #include <QCache>
+#endif
 #include <QFont>
 #include <QFontMetrics>
 #include <QHash>
@@ -65,11 +68,16 @@ public:
 	///
 	int width(QString const & str) const;
 
+	/// Return a pointer to a cached QTextLayout object
+	QTextLayout const *
+	getTextLayout(docstring const & s, bool const rtl,
+                  double const wordspacing) const;
+
 private:
 
-	QTextLayout const &
-	getTextLayout(docstring const & s, QFont font,
-	              bool const rtl, double const wordspacing) const;
+	std::pair<int, int> *
+	breakAt_helper(docstring const & s, int const x,
+	               bool const rtl, bool const force) const;
 
 	/// The font
 	QFont font_;
@@ -80,8 +88,16 @@ private:
 	/// Cache of char widths
 	mutable QHash<char_type, int> width_cache_;
 
+#ifdef CACHE_FONT_METRICS
 	/// Cache of string widths
-	mutable QCache<QByteArray, int> strwidth_cache_;
+	mutable QCache<docstring, int> strwidth_cache_;
+
+	/// Cache for breakAt
+	mutable QCache<docstring, std::pair<int, int>> breakat_cache_;
+
+	/// Cache for QTextLayout:s
+	mutable QCache<docstring, QTextLayout> qtextlayout_cache_;
+#endif
 
 	struct AscendDescend {
 		int ascent;
@@ -95,13 +111,6 @@ private:
 	/// Cache of char right bearings
 	mutable QHash<char_type, int> rbearing_cache_;
 
-	// A trivial QTextLayout cache
-	mutable QTextLayout tl_cache_;
-	mutable docstring tl_cache_s_;
-	mutable QFont tl_cache_font_;
-	mutable bool tl_cache_rtl_;
-	mutable double tl_cache_wordspacing_;
-
 };
 
 } // namespace frontend
diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp
index 6c89ff5..448b5a4 100644
--- a/src/frontends/qt4/GuiPainter.cpp
+++ b/src/frontends/qt4/GuiPainter.cpp
@@ -478,8 +478,17 @@ void GuiPainter::text(int x, int y, docstring const & s,
 		return;
 	}
 
-	// don't use the pixmap cache,
-	do_drawText(x, y, str, dir, f, ff);
+	// don't use the pixmap cache
+	setQPainterPen(computeColor(f.realColor()));
+	if (dir != Auto) {
+		QTextLayout const * ptl = fm.getTextLayout(s, dir == RtL, wordspacing);
+		ptl->draw(this, QPointF(x, y - fm.maxAscent()));
+	}
+	else {
+		if (font() != ff)
+			setFont(ff);
+		drawText(x, y, str);
+	}
 	//LYXERR(Debug::PAINTING, "draw " << string(str.toUtf8())
 	//	<< " at " << x << "," << y);
 }
diff --git a/src/support/convert.cpp b/src/support/convert.cpp
index 58a5647..076c9d7 100644
--- a/src/support/convert.cpp
+++ b/src/support/convert.cpp
@@ -155,6 +155,13 @@ string convert<string>(double d)
 
 
 template<>
+docstring convert<docstring>(double d)
+{
+	return from_ascii(convert<string>(d));
+}
+
+
+template<>
 int convert<int>(string const s)
 {
 	return strtol(s.c_str(), 0, 10);
-- 
2.9.3

Reply via email to