From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Fri, 11 Dec 2015 15:33:34 +0000 (+0100)
Subject: Do not use a static variable as QTextLayout cache
X-Git-Tag: 2.2.0beta1~358
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=91b385166de84677197f326702f0e95a5d649091;p=features.git

Do not use a static variable as QTextLayout cache

It is a bad idea to have a QObject variable that oulives the main QApplication object. See for example:
https://www.ics.com/designpatterns/book/globals.html

Here the QTextLayout object was static to the anonymous namespace getTextLayout function, and got destroyed after the freetype renderer had been disposed of by QApplication.

This causes segmentation faults when quitting LyX on some systems.

This patch moves the cache together with other GuiFontMetrics caches. It means that one will have one such QTextLayout per font type, but this will not change much.
---

diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp
index 7cf841f873..c58be2aee3 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -23,8 +23,6 @@
 
 #include "support/lassert.h"
 
-#include <QTextLayout>
-
 using namespace std;
 using namespace lyx::support;
 
@@ -56,7 +54,8 @@ inline QChar const ucs4_to_qchar(char_type const ucs4)
 // Limit strwidth_cache_ size to 512kB of string data
 GuiFontMetrics::GuiFontMetrics(QFont const & font)
 	: font_(font), metrics_(font, 0),
-	  strwidth_cache_(1 << 19)
+	  strwidth_cache_(1 << 19),
+	  tl_cache_rtl_(false), tl_cache_wordspacing_(-1.0)
 {
 }
 
@@ -170,36 +169,27 @@ int GuiFontMetrics::signedWidth(docstring const & s) const
 		return width(s);
 }
 
-namespace {
 
-QTextLayout const & getTextLayout(docstring const & s, QFont font,
-                                  bool const rtl, double const wordspacing)
+QTextLayout const &
+GuiFontMetrics::getTextLayout(docstring const & s, QFont font,
+                              bool const rtl, double const wordspacing) const
 {
-	static docstring old_s;
-	static QFont old_font;
-	static bool old_rtl = false;
-	// this invalid value is to make sure that it is reset on first try.
-	static double old_wordspacing = -1.0;
-	// This one is our trivial cache
-	static QTextLayout tl;
-	if (s != old_s || font != old_font || rtl != old_rtl
-	    || wordspacing != old_wordspacing) {
-		tl.setText(toqstr(s));
+	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.setFont(font);
+		tl_cache_.setFont(font);
 		// Note that both setFlags and the enums are undocumented
-		tl.setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
-		tl.beginLayout();
-		tl.createLine();
-		tl.endLayout();
-		old_s = s;
-		old_font = font;
-		old_rtl = rtl;
-		old_wordspacing = wordspacing;
+		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;
 	}
-	return tl;
-}
-
+	return tl_cache_;
 }
 
 
diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h
index 1e813f6af1..8c8daab5cd 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -21,6 +21,7 @@
 #include <QFont>
 #include <QFontMetrics>
 #include <QHash>
+#include <QTextLayout>
 
 namespace lyx {
 namespace frontend {
@@ -63,6 +64,11 @@ public:
 	int width(QString const & str) const;
 
 private:
+
+	QTextLayout const &
+	getTextLayout(docstring const & s, QFont font,
+	              bool const rtl, double const wordspacing) const;
+
 	/// The font
 	QFont font_;
 
@@ -86,6 +92,14 @@ 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