]> git.lyx.org Git - features.git/commitdiff
Introduce support/Cache.h
authorGuillaume Munch <gm@lyx.org>
Mon, 20 Feb 2017 22:59:24 +0000 (23:59 +0100)
committerJean-Marc Lasgouttes <lasgouttes@lyx.org>
Thu, 23 Feb 2017 17:07:30 +0000 (18:07 +0100)
Useful to cache copies of objects, including shared_ptrs. No risks of dangling
pointer, and avoid naked pointers in the source.

Fix memory leak when compiling with Qt5.

As part as the backport to stable, this code has been change to work
with C++98.

(cherry picked from commit 33b696c8acf2e64b44d449180781de6dbc203709)
(cherry picked from commit e04079aa528ecbf4a8e39ed2b19c3cb50174e151)
(cherry picked from commit 5211ca52cac2ad7a6669d15c39f2cee172d18323)
(cherry picked from commit 8353a53cc38fe364bee516e86a08251e4ae974fc)

src/frontends/qt4/GuiFontMetrics.cpp
src/frontends/qt4/GuiFontMetrics.h
src/frontends/qt4/GuiPainter.cpp
src/support/Cache.h [new file with mode: 0644]
src/support/Makefile.am

index 5bb99aafc1b4c65d3295e6fba710130d7419cd6a..80ff8195ec0837e0989d87ba2cb79083f1c8ca4e 100644 (file)
 #include "support/convert.h"
 #include "support/lassert.h"
 
-#ifdef CACHE_SOME_METRICS
 #include <QByteArray>
-#endif
 
 using namespace std;
 using namespace lyx::support;
 
-#ifdef CACHE_SOME_METRICS
 namespace std {
 
 /*
@@ -46,11 +43,28 @@ uint qHash(lyx::docstring const & s)
 }
 
 }
-#endif
 
 namespace lyx {
 namespace frontend {
 
+
+/*
+ * 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.
+ * Also, setting size to 0 is tantamount to disabling the cache.
+ */
+int cache_metrics_width_size = 1 << 19;
+int cache_metrics_breakat_size = 1 << 19;
+// Qt 5.x already has its own caching of QTextLayout objects
+#if (QT_VERSION < 0x050000)
+int cache_metrics_qtextlayout_size = 500;
+#else
+int cache_metrics_qtextlayout_size = 0;
+#endif
+
+
 namespace {
 /**
  * Convert a UCS4 character into a QChar.
@@ -73,23 +87,11 @@ inline QChar const ucs4_to_qchar(char_type const ucs4)
 } // anon namespace
 
 
-/*
- * 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)
-#ifdef CACHE_METRICS_WIDTH
-       , strwidth_cache_(1 << 19)
-#endif
-#ifdef CACHE_METRICS_BREAKAT
-       , breakat_cache_(1 << 19)
-#endif
-#ifdef CACHE_METRICS_QTEXTLAYOUT
-       ,  qtextlayout_cache_(500)
-#endif
+       : font_(font), metrics_(font, 0),
+         strwidth_cache_(cache_metrics_width_size),
+         breakat_cache_(cache_metrics_breakat_size),
+         qtextlayout_cache_(cache_metrics_qtextlayout_size)
 {
 }
 
@@ -174,11 +176,8 @@ int GuiFontMetrics::rbearing(char_type c) const
 
 int GuiFontMetrics::width(docstring const & s) const
 {
-#ifdef CACHE_METRICS_WIDTH
-       int * pw = strwidth_cache_[s];
-       if (pw)
-               return *pw;
-#endif
+       if (strwidth_cache_.contains(s))
+               return strwidth_cache_[s];
        /* For some reason QMetrics::width returns a wrong value with Qt5
         * with some arabic text. OTOH, QTextLayout is broken for single
         * characters with null width (like \not in mathed). Also, as a
@@ -200,9 +199,7 @@ int GuiFontMetrics::width(docstring const & s) const
                tl.endLayout();
                w = int(line.naturalTextWidth());
        }
-#ifdef CACHE_METRICS_WIDTH
-       strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type));
-#endif
+       strwidth_cache_.insert(s, w, s.size() * sizeof(char_type));
        return w;
 }
 
@@ -225,31 +222,26 @@ int GuiFontMetrics::signedWidth(docstring const & s) const
 }
 
 
-QTextLayout const *
+shared_ptr<QTextLayout const>
 GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl,
                               double const wordspacing) const
 {
-       QTextLayout * ptl;
-#ifdef CACHE_METRICS_QTEXTLAYOUT
-       docstring const s_cache = s + (rtl ? "r" : "l") + convert<docstring>(wordspacing);
-       ptl = qtextlayout_cache_[s_cache];
-       if (!ptl) {
-#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
-               ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
-               ptl->beginLayout();
-               ptl->createLine();
-               ptl->endLayout();
-#ifdef CACHE_METRICS_QTEXTLAYOUT
-               qtextlayout_cache_.insert(s_cache, ptl);
-       }
-#endif
+       docstring const s_cache =
+               s + (rtl ? "r" : "l") + convert<docstring>(wordspacing);
+       if (shared_ptr<QTextLayout const> ptl = qtextlayout_cache_[s_cache])
+               return ptl;
+       shared_ptr<QTextLayout> const ptl = make_shared<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
+       ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
+       ptl->beginLayout();
+       ptl->createLine();
+       ptl->endLayout();
+       qtextlayout_cache_.insert(s_cache, ptl);
        return ptl;
 }
 
@@ -259,7 +251,7 @@ int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl,
 {
        if (pos <= 0)
                pos = 0;
-       QTextLayout const * tl = getTextLayout(s, rtl, wordspacing);
+       shared_ptr<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).
@@ -272,7 +264,7 @@ int GuiFontMetrics::pos2x(docstring const & s, int pos, bool const rtl,
 int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
                           double const wordspacing) const
 {
-       QTextLayout const * tl = getTextLayout(s, rtl, wordspacing);
+       shared_ptr<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));
@@ -301,7 +293,7 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl,
 
 
 
-pair<int, int> *
+pair<int, int>
 GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
                                bool const rtl, bool const force) const
 {
@@ -346,7 +338,7 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
        tl.createLine();
        tl.endLayout();
        if ((force && line.textLength() == offset) || int(line.naturalTextWidth()) > x)
-               return new pair<int, int>(-1, -1);
+               return make_pair(-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).
@@ -371,7 +363,7 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
        LASSERT(len > 0 || qlen == 0, /**/);
 #endif
        // The -1 is here to account for the leading zerow_nbsp.
-       return new pair<int, int>(len, int(line.naturalTextWidth()));
+       return make_pair(len, int(line.naturalTextWidth()));
 }
 
 
@@ -379,25 +371,21 @@ bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const
 {
        if (s.empty())
                return false;
-       pair<int, int> * pp;
-#ifdef CACHE_METRICS_BREAKAT
-       docstring const s_cache = s + convert<docstring>(x) + (rtl ? "r" : "l") + (force ? "f" : "w");
 
-       pp = breakat_cache_[s_cache];
-       if (!pp) {
-#endif
+       docstring const s_cache =
+               s + convert<docstring>(x) + (rtl ? "r" : "l") + (force ? "f" : "w");
+       pair<int, int> pp;
+
+       if (breakat_cache_.contains(s_cache))
+               pp = breakat_cache_[s_cache];
+       else {
                pp = breakAt_helper(s, x, rtl, force);
-#ifdef CACHE_METRICS_BREAKAT
                breakat_cache_.insert(s_cache, pp, s_cache.size() * sizeof(char_type));
        }
-#endif
-       if (pp->first == -1)
+       if (pp.first == -1)
                return false;
-       s = s.substr(0, pp->first);
-       x = pp->second;
-#ifndef CACHE_METRICS_BREAKAT
-       delete pp;
-#endif
+       s = s.substr(0, pp.first);
+       x = pp.second;
        return true;
 }
 
index 438ebb4c65638b15b388cd24b0620664064ae394..ffbbe20bcc0c09bc347126594d2150249ba12ad3 100644 (file)
 
 #include "frontends/FontMetrics.h"
 
+#include "support/Cache.h"
 #include "support/docstring.h"
+#include "support/shared_ptr.h"
 
 #include <QFont>
 #include <QFontMetrics>
 #include <QHash>
 #include <QTextLayout>
 
-// Declare which font metrics elements have to be cached
-
-#define CACHE_METRICS_WIDTH
-#define CACHE_METRICS_BREAKAT
-// Qt 5.x already has its own caching of QTextLayout objects
-#if (QT_VERSION < 0x050000)
-#define CACHE_METRICS_QTEXTLAYOUT
-#endif
-
-#if defined(CACHE_METRICS_WIDTH) || defined(CACHE_METRICS_BREAKAT) \
-  || defined(CACHE_METRICS_QTEXTLAYOUT)
-#define CACHE_SOME_METRICS
-#endif
-
-#ifdef CACHE_SOME_METRICS
-#include <QCache>
-#endif
+#include <memory>
 
 namespace lyx {
 namespace frontend {
@@ -80,15 +66,14 @@ public:
        int width(QString const & str) const;
 
        /// Return a pointer to a cached QTextLayout object
-       QTextLayout const *
+       shared_ptr<QTextLayout const>
        getTextLayout(docstring const & s, bool const rtl,
-                  double const wordspacing) const;
+                     double const wordspacing) const;
 
 private:
 
-       std::pair<int, int> *
-       breakAt_helper(docstring const & s, int const x,
-                      bool const rtl, bool const force) const;
+       std::pair<int, int> breakAt_helper(docstring const & s, int const x,
+                                          bool const rtl, bool const force) const;
 
        /// The font
        QFont font_;
@@ -98,21 +83,12 @@ private:
 
        /// Cache of char widths
        mutable QHash<char_type, int> width_cache_;
-
-#ifdef CACHE_METRICS_WIDTH
        /// Cache of string widths
-       mutable QCache<docstring, int> strwidth_cache_;
-#endif
-
-#ifdef CACHE_METRICS_BREAKAT
+       mutable Cache<docstring, int> strwidth_cache_;
        /// Cache for breakAt
-       mutable QCache<docstring, std::pair<int, int> > breakat_cache_;
-#endif
-
-#ifdef CACHE_METRICS_QTEXTLAYOUT
-       /// Cache for QTextLayout:s
-       mutable QCache<docstring, QTextLayout> qtextlayout_cache_;
-#endif
+       mutable Cache<docstring, std::pair<int, int> > breakat_cache_;
+       /// Cache for QTextLayout
+       mutable Cache<docstring, shared_ptr<QTextLayout> > qtextlayout_cache_;
 
        struct AscendDescend {
                int ascent;
index 8607454d8d1f2f37bda4726450e6eddfc62d2a78..f87c26699f724241d15cc89cdd344025aee09726 100644 (file)
@@ -473,7 +473,8 @@ int GuiPainter::text(int x, int y, docstring const & s,
        // don't use the pixmap cache
        setQPainterPen(computeColor(f.realColor()));
        if (dir != Auto) {
-               QTextLayout const * ptl = fm.getTextLayout(s, dir == RtL, wordspacing);
+               shared_ptr<QTextLayout const> ptl =
+                       fm.getTextLayout(s, dir == RtL, wordspacing);
                ptl->draw(this, QPointF(x, y - fm.maxAscent()));
        }
        else {
diff --git a/src/support/Cache.h b/src/support/Cache.h
new file mode 100644 (file)
index 0000000..6ff452f
--- /dev/null
@@ -0,0 +1,89 @@
+// -*- C++ -*-
+/**
+ * \file Cache.h
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Guillaume Munch
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#ifndef CACHE_H
+#define CACHE_H
+
+#include <QCache>
+
+#include <list>
+#ifdef LYX_USE_CXX11
+#include <type_traits>
+#endif
+
+namespace lyx {
+
+/**
+ * Cache<Key, T> implements a cache where objects are stored by copy.
+ *
+ * This is a wrapper for QCache. See the documentation of QCache:
+ * <https://doc.qt.io/qt-5/qcache.html>.
+ *
+ * It is especially useful for storing shared pointers. This turns QCache into a
+ * shared-ownership cache with no risks of dangling pointer. It is also useful
+ * for small copyable objects.
+ *
+ * Use this rather than QCache directly, to avoid naked pointers.
+ */
+template <class Key, class Val>
+class Cache : private QCache<Key, Val> {
+#if defined(LYX_USE_CXX11) && !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6))
+       static_assert(std::is_copy_constructible<Val>::value,
+                     "lyx::Cache only stores copyable objects!");
+       static_assert(std::is_default_constructible<Val>::value,
+                     "lyx::Cache only stores default-constructible objects!");
+       using Q = QCache<Key, Val>;
+#else
+       typedef QCache<Key, Val> Q;
+#endif
+
+public:
+       ///
+       Cache(int max_cost = 100) : Q(max_cost) {}
+       ///
+#ifdef LYX_USE_CXX11
+       bool insert(Key const & key, Val object, int cost = 1)
+       {
+               return Q::insert(key, new Val(std::move(object)), cost);
+       }
+#else
+       bool insert(Key const & key, Val const & object, int cost = 1)
+       {
+               return Q::insert(key, new Val(object), cost);
+       }
+#endif
+       // Returns the default value (e.g. null pointer) if not found in the
+       // cache. If this is not convenient for your class Val, check if it exists
+       // beforehand with Cache::contains.
+       Val object(Key const & key) const
+       {
+               if (Val * obj = Q::object(key))
+                       return *obj;
+               return Val();
+       }
+       /// Synonymous for object, same remark as above.
+       Val operator[](Key const & key) const { return object(key); }
+       /// Everything from QCache except QCache::take.
+       using Q::clear;
+       using Q::contains;
+       using Q::count;
+       using Q::remove;
+       using Q::size;
+       bool empty() const { return Q::isEmpty(); }
+       std::list<Key> keys() { return Q::keys().toStdList(); }
+       int max_cost() const { return Q::maxCost(); }
+       void set_max_cost(int cost) { Q::setMaxCost(cost); }
+       int total_cost() const { return Q::totalCost(); }
+};
+
+} // namespace lyx
+
+#endif
index ed6af579d18f2204306fce362fcd49db24517652..a1dc3c2f695c844777f67181024a2522eac79195 100644 (file)
@@ -33,6 +33,7 @@ liblyxsupport_a_SOURCES = \
        FileMonitor.cpp \
        RandomAccessList.h \
        bind.h \
+       Cache.h \
        ConsoleApplication.cpp \
        ConsoleApplication.h \
        ConsoleApplicationPrivate.h \