From: Guillaume Munch Date: Mon, 20 Feb 2017 22:59:24 +0000 (+0100) Subject: Introduce support/Cache.h X-Git-Tag: 2.2.3~59 X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=998c3e7c8ef8400633ae45216233fb0962bc97be;p=features.git Introduce support/Cache.h 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) --- diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 5bb99aafc1..80ff8195ec 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -24,14 +24,11 @@ #include "support/convert.h" #include "support/lassert.h" -#ifdef CACHE_SOME_METRICS #include -#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 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(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(wordspacing); + if (shared_ptr ptl = qtextlayout_cache_[s_cache]) + return ptl; + shared_ptr const ptl = make_shared(); + 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 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 tl = getTextLayout(s, rtl, wordspacing); int const qpos = tl->lineForTextPosition(0).xToCursor(x); // correct x value to the actual cursor position. x = static_cast(tl->lineForTextPosition(0).cursorToX(qpos)); @@ -301,7 +293,7 @@ int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl, -pair * +pair 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(-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(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 * pp; -#ifdef CACHE_METRICS_BREAKAT - docstring const s_cache = s + convert(x) + (rtl ? "r" : "l") + (force ? "f" : "w"); - pp = breakat_cache_[s_cache]; - if (!pp) { -#endif + docstring const s_cache = + s + convert(x) + (rtl ? "r" : "l") + (force ? "f" : "w"); + pair 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; } diff --git a/src/frontends/qt4/GuiFontMetrics.h b/src/frontends/qt4/GuiFontMetrics.h index 438ebb4c65..ffbbe20bcc 100644 --- a/src/frontends/qt4/GuiFontMetrics.h +++ b/src/frontends/qt4/GuiFontMetrics.h @@ -14,30 +14,16 @@ #include "frontends/FontMetrics.h" +#include "support/Cache.h" #include "support/docstring.h" +#include "support/shared_ptr.h" #include #include #include #include -// 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 -#endif +#include 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 getTextLayout(docstring const & s, bool const rtl, - double const wordspacing) const; + double const wordspacing) const; private: - std::pair * - breakAt_helper(docstring const & s, int const x, - bool const rtl, bool const force) const; + std::pair 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 width_cache_; - -#ifdef CACHE_METRICS_WIDTH /// Cache of string widths - mutable QCache strwidth_cache_; -#endif - -#ifdef CACHE_METRICS_BREAKAT + mutable Cache strwidth_cache_; /// Cache for breakAt - mutable QCache > breakat_cache_; -#endif - -#ifdef CACHE_METRICS_QTEXTLAYOUT - /// Cache for QTextLayout:s - mutable QCache qtextlayout_cache_; -#endif + mutable Cache > breakat_cache_; + /// Cache for QTextLayout + mutable Cache > qtextlayout_cache_; struct AscendDescend { int ascent; diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp index 8607454d8d..f87c26699f 100644 --- a/src/frontends/qt4/GuiPainter.cpp +++ b/src/frontends/qt4/GuiPainter.cpp @@ -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 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 index 0000000000..6ff452fc3c --- /dev/null +++ b/src/support/Cache.h @@ -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 + +#include +#ifdef LYX_USE_CXX11 +#include +#endif + +namespace lyx { + +/** + * Cache implements a cache where objects are stored by copy. + * + * This is a wrapper for QCache. See the documentation of QCache: + * . + * + * 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 Cache : private QCache { +#if defined(LYX_USE_CXX11) && !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)) + static_assert(std::is_copy_constructible::value, + "lyx::Cache only stores copyable objects!"); + static_assert(std::is_default_constructible::value, + "lyx::Cache only stores default-constructible objects!"); + using Q = QCache; +#else + typedef QCache 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 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 diff --git a/src/support/Makefile.am b/src/support/Makefile.am index ed6af579d1..a1dc3c2f69 100644 --- a/src/support/Makefile.am +++ b/src/support/Makefile.am @@ -33,6 +33,7 @@ liblyxsupport_a_SOURCES = \ FileMonitor.cpp \ RandomAccessList.h \ bind.h \ + Cache.h \ ConsoleApplication.cpp \ ConsoleApplication.h \ ConsoleApplicationPrivate.h \