From: Guillaume Munch Date: Mon, 20 Feb 2017 22:59:24 +0000 (+0100) Subject: Introduce support/Cache.h X-Git-Tag: 2.3.0alpha1~347 X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=33b696c8acf2e64b44d449180781de6dbc203709;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. --- diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 512e8771b6..31c45481ae 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -27,14 +27,11 @@ #define DISABLE_PMPROF #include "support/pmprof.h" -#ifdef CACHE_SOME_METRICS #include -#endif using namespace std; using namespace lyx::support; -#ifdef CACHE_SOME_METRICS namespace std { /* @@ -49,11 +46,21 @@ uint qHash(lyx::docstring const & s) } } -#endif namespace lyx { namespace frontend { + +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. @@ -83,16 +90,10 @@ inline QChar const ucs4_to_qchar(char_type const ucs4) * 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) { } @@ -177,13 +178,10 @@ int GuiFontMetrics::rbearing(char_type c) const int GuiFontMetrics::width(docstring const & s) const { - PROFILE_THIS_BLOCK(width) -#ifdef CACHE_METRICS_WIDTH - int * pw = strwidth_cache_[s]; - if (pw) - return *pw; - PROFILE_CACHE_MISS(width) -#endif + PROFILE_THIS_BLOCK(width); + if (strwidth_cache_.contains(s)) + return strwidth_cache_[s]; + PROFILE_CACHE_MISS(width); /* 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 @@ -205,9 +203,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; } @@ -230,33 +226,28 @@ int GuiFontMetrics::signedWidth(docstring const & s) const } -QTextLayout const * +shared_ptr GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, double const wordspacing) const { - PROFILE_THIS_BLOCK(getTextLayout) - QTextLayout * ptl; -#ifdef CACHE_METRICS_QTEXTLAYOUT - docstring const s_cache = s + (rtl ? "r" : "l") + convert(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 - ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); - ptl->beginLayout(); - ptl->createLine(); - ptl->endLayout(); -#ifdef CACHE_METRICS_QTEXTLAYOUT - qtextlayout_cache_.insert(s_cache, ptl); - } -#endif + PROFILE_THIS_BLOCK(getTextLayout); + docstring const s_cache = + s + (rtl ? "r" : "l") + convert(wordspacing); + if (auto ptl = qtextlayout_cache_[s_cache]) + return ptl; + PROFILE_CACHE_MISS(getTextLayout); + auto 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; } @@ -266,7 +257,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). @@ -279,7 +270,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)); @@ -328,7 +319,7 @@ int GuiFontMetrics::countExpanders(docstring const & str) const } -pair * +pair GuiFontMetrics::breakAt_helper(docstring const & s, int const x, bool const rtl, bool const force) const { @@ -373,7 +364,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 {-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). @@ -398,35 +389,31 @@ 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 {len, int(line.naturalTextWidth())}; } bool GuiFontMetrics::breakAt(docstring & s, int & x, bool const rtl, bool const force) const { - PROFILE_THIS_BLOCK(breakAt) + PROFILE_THIS_BLOCK(breakAt); 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) { - PROFILE_CACHE_MISS(breakAt) -#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 { + PROFILE_CACHE_MISS(breakAt); 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 de567ef8fb..35f66fc502 100644 --- a/src/frontends/qt4/GuiFontMetrics.h +++ b/src/frontends/qt4/GuiFontMetrics.h @@ -14,6 +14,7 @@ #include "frontends/FontMetrics.h" +#include "support/Cache.h" #include "support/docstring.h" #include @@ -21,23 +22,7 @@ #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 { @@ -82,15 +67,14 @@ public: int width(QString const & str) const; /// Return a pointer to a cached QTextLayout object - QTextLayout const * + std::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_; @@ -100,21 +84,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 448b5a44b2..a7401092c9 100644 --- a/src/frontends/qt4/GuiPainter.cpp +++ b/src/frontends/qt4/GuiPainter.cpp @@ -481,7 +481,8 @@ void 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..75427809b4 --- /dev/null +++ b/src/support/Cache.h @@ -0,0 +1,75 @@ +// -*- 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 +#include + +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 { + 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; + +public: + /// + Cache(int max_cost = 100) : Q(max_cost) {} + /// + bool insert(Key const & key, Val object, int cost = 1) + { + return Q::insert(key, new Val(std::move(object)), cost); + } + // Returns the default value (e.g. null pointer) before using the result. If + // this is not convenient for your type, check if it exists beforehand with + // Cache::contains. + Val object(Key const & key) const + { + if (Val * obj = Q::object(key)) + return *obj; + return Val(); + } + /// 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(); } + Val operator[](Key const & key) const { return object(key); } +}; + +} // namespace lyx + +#endif diff --git a/src/support/Makefile.am b/src/support/Makefile.am index 08f1c7180d..a63bb89681 100644 --- a/src/support/Makefile.am +++ b/src/support/Makefile.am @@ -35,6 +35,7 @@ liblyxsupport_a_SOURCES = \ FileMonitor.cpp \ RandomAccessList.h \ bind.h \ + Cache.h \ Changer.h \ ConsoleApplication.cpp \ ConsoleApplication.h \