From cf3d75430cc56ce5b53a509a4e581ca09661f91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Kera=CC=88nen?= Date: Fri, 10 Feb 2017 21:06:28 +0200 Subject: [PATCH] libgui|Font: Improved thread-safety Native fonts are now designed to be used only in a single thread, and internally in de::Font there is a separate native font instance per thread. This allows text operations to proceed without blocking in all the background threads. --- .../sdk/libgui/include/de/text/nativefont.h | 3 + .../src/text/coretextnativefont_macx.cpp | 86 +++++-------------- doomsday/sdk/libgui/src/text/font.cpp | 85 ++++++++++++------ doomsday/sdk/libgui/src/text/nativefont.cpp | 5 -- 4 files changed, 84 insertions(+), 95 deletions(-) diff --git a/doomsday/sdk/libgui/include/de/text/nativefont.h b/doomsday/sdk/libgui/include/de/text/nativefont.h index 0964eb269a..357c24d71a 100644 --- a/doomsday/sdk/libgui/include/de/text/nativefont.h +++ b/doomsday/sdk/libgui/include/de/text/nativefont.h @@ -35,6 +35,9 @@ namespace de { * string of text, and draw the text onto an Image. This is an abstract base class for * concrete implementations of native fonts. * + * Each NativeFont is specific to a single thread. This allows text rendering to be + * done in multiple background threads without synchronization. + * * @ingroup gui */ class LIBGUI_PUBLIC NativeFont : public Asset diff --git a/doomsday/sdk/libgui/src/text/coretextnativefont_macx.cpp b/doomsday/sdk/libgui/src/text/coretextnativefont_macx.cpp index d238b16213..6430315359 100644 --- a/doomsday/sdk/libgui/src/text/coretextnativefont_macx.cpp +++ b/doomsday/sdk/libgui/src/text/coretextnativefont_macx.cpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace de { @@ -109,7 +110,7 @@ struct CoreTextFontCache : public Lockable return font; } -#ifdef DENG2_DEBUG +#if 0 float fontSize(CTFontRef font) const { DENG2_FOR_EACH_CONST(Fonts, i, fonts) @@ -137,26 +138,6 @@ struct CoreTextFontCache : public Lockable #endif }; -#ifdef DENG2_DEBUG -struct LineCounter : public Lockable { - int count = 0; - ~LineCounter() { - qDebug() << "[CoreTextNativeFont] Cached line count:" << count; - } - void inc() { - lock(); - ++count; - unlock(); - } - void dec() { - lock(); - --count; - unlock(); - } -}; -static LineCounter ctLineCounter; -#endif - static CoreTextFontCache fontCache; DENG2_PIMPL(CoreTextNativeFont) @@ -167,48 +148,27 @@ DENG2_PIMPL(CoreTextNativeFont) float height; float lineSpacing; - // Note that fonts may be used from multiple threads, so we keep a thread-specific - // cache of the most recently created line. struct CachedLine { String lineText; CTLineRef line = nullptr; + ~CachedLine() + { + release(); + } + void release() { if (line) { CFRelease(line); line = nullptr; -#ifdef DENG2_DEBUG - ctLineCounter.dec(); -#endif } lineText.clear(); } }; - struct Cache : public QHash, public Lockable - { - ~Cache() - { - clear(); - } - - void clear() - { - for (CachedLine &entry : *this) - { - entry.release(); - } - } - - CachedLine &cachedLineForCurrentThread() - { - DENG2_GUARD(this); - return (*this)[QThread::currentThread()]; - } - }; - Cache cache; + CachedLine cache; Impl(Public *i) : Base(i) @@ -252,7 +212,7 @@ DENG2_PIMPL(CoreTextNativeFont) void release() { font = 0; - cache.clear(); + cache.release(); } void updateFontAndMetrics() @@ -271,14 +231,13 @@ DENG2_PIMPL(CoreTextNativeFont) CachedLine &makeLine(String const &text, CGColorRef color = 0) { - auto &cachedLine = cache.cachedLineForCurrentThread(); - if (cachedLine.lineText == text) + if (cache.lineText == text) { - return cachedLine; // Already got it. + return cache; // Already got it. } - cachedLine.release(); - cachedLine.lineText = text; + cache.release(); + cache.lineText = text; void const *keys[] = { kCTFontAttributeName, kCTForegroundColorAttributeName }; void const *values[] = { font, color }; @@ -287,16 +246,12 @@ DENG2_PIMPL(CoreTextNativeFont) CFStringRef textStr = CFStringCreateWithCharacters(nil, (UniChar *) text.data(), text.size()); CFAttributedStringRef as = CFAttributedStringCreate(0, textStr, attribs); - cachedLine.line = CTLineCreateWithAttributedString(as); - -#ifdef DENG2_DEBUG - ctLineCounter.inc(); -#endif + cache.line = CTLineCreateWithAttributedString(as); CFRelease(attribs); CFRelease(textStr); CFRelease(as); - return cachedLine; + return cache; } }; @@ -366,7 +321,7 @@ Rectanglei CoreTextNativeFont::nativeFontMeasure(String const &text) const //CGLineGetImageBounds(d->line, d->gc); // more accurate but slow Rectanglei rect(Vector2i(0, -d->ascent), - Vector2i(roundi(CTLineGetTypographicBounds(d->cache.cachedLineForCurrentThread().line, NULL, NULL, NULL)), + Vector2i(roundi(CTLineGetTypographicBounds(d->cache.line, NULL, NULL, NULL)), d->descent)); return rect; @@ -392,12 +347,11 @@ QImage CoreTextNativeFont::nativeFontRasterize(String const &text, CGColorRef fgColor = CGColorCreate(fontCache.colorspace(), &fg.x); // Ensure the color is used by recreating the attributed line string. - auto &cachedLine = d->cache.cachedLineForCurrentThread(); - cachedLine.release(); + d->cache.release(); d->makeLine(d->applyTransformation(text), fgColor); // Set up the bitmap for drawing into. - Rectanglei const bounds = measure(cachedLine.lineText); + Rectanglei const bounds = measure(d->cache.lineText); QImage backbuffer(QSize(bounds.width(), bounds.height()), QImage::Format_ARGB32); backbuffer.fill(QColor(background.x, background.y, background.z, background.w).rgba()); @@ -409,11 +363,11 @@ QImage CoreTextNativeFont::nativeFontRasterize(String const &text, kCGImageAlphaPremultipliedLast); CGContextSetTextPosition(gc, 0, d->descent); - CTLineDraw(cachedLine.line, gc); + CTLineDraw(d->cache.line, gc); CGColorRelease(fgColor); CGContextRelease(gc); - cachedLine.release(); + d->cache.release(); return backbuffer; } diff --git a/doomsday/sdk/libgui/src/text/font.cpp b/doomsday/sdk/libgui/src/text/font.cpp index 477963e386..f3be88fd82 100644 --- a/doomsday/sdk/libgui/src/text/font.cpp +++ b/doomsday/sdk/libgui/src/text/font.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #if defined(MACOSX) && defined(MACOS_10_7) # include "../src/text/coretextnativefont_macx.h" @@ -71,10 +72,27 @@ namespace internal } } -DENG2_PIMPL(Font), public Lockable +DENG2_PIMPL(Font) { - PlatformFont font; - QHash fontMods; + QFont referenceFont; + struct ThreadFonts + { + PlatformFont font; + QHash fontMods; + + ~ThreadFonts() { + qDeleteAll(fontMods.values()); + } + }; + + /** + * Each thread uses its own independent set of native font instances. This allows + * background threads to freely measure and render text using the native font + * instances without any synchronization. Also note that these background threads are + * pooled, so there is only a fixed total number of threads accessing these objects. + */ + QThreadStorage fontsForThread; + ConstantRule *heightRule; ConstantRule *ascentRule; ConstantRule *descentRule; @@ -86,7 +104,7 @@ DENG2_PIMPL(Font), public Lockable createRules(); } - Impl(Public *i, PlatformFont const &qfont) : Base(i), font(qfont) + Impl(Public *i, QFont const &qfont) : Base(i), referenceFont(qfont) { #if 0 // Development aid: list all available fonts and styles. @@ -104,7 +122,7 @@ DENG2_PIMPL(Font), public Lockable ~Impl() { - qDeleteAll(fontMods.values()); + //qDeleteAll(fontMods.values()); releaseRef(heightRule); releaseRef(ascentRule); @@ -120,30 +138,44 @@ DENG2_PIMPL(Font), public Lockable lineSpacingRule = new ConstantRule(0); } + /** + * Initializes the current thread's platform fonts for this Font. + */ + ThreadFonts &getThreadFonts() + { + if (!fontsForThread.hasLocalData()) + { + fontsForThread.localData().font = PlatformFont(referenceFont); + } + return fontsForThread.localData(); + } + void updateMetrics() { - ascent = font.ascent(); - if (font.weight() != NativeFont::Normal) + auto &plat = getThreadFonts(); + + ascent = plat.font.ascent(); + if (plat.font.weight() != NativeFont::Normal) { // Use the ascent of the normal weight for non-normal weights; // we need to align content to baseline regardless of weight. - PlatformFont normalized(font); + PlatformFont normalized(plat.font); normalized.setWeight(NativeFont::Normal); ascent = normalized.ascent(); } - ascentRule->set(ascent); - descentRule->set(font.descent()); - heightRule->set(font.height()); - lineSpacingRule->set(font.lineSpacing()); + ascentRule ->set(ascent); + descentRule ->set(plat.font.descent()); + heightRule ->set(plat.font.height()); + lineSpacingRule->set(plat.font.lineSpacing()); } PlatformFont &getFontMod(internal::FontParams const ¶ms) { - DENG2_GUARD(this); + auto &plat = getThreadFonts(); - auto found = fontMods.constFind(params); - if (found != fontMods.constEnd()) + auto found = plat.fontMods.constFind(params); + if (found != plat.fontMods.constEnd()) { return *found.value(); } @@ -154,7 +186,7 @@ DENG2_PIMPL(Font), public Lockable mod->setStyle(params.spec.style); mod->setWeight(params.spec.weight); mod->setTransform(params.spec.transform); - fontMods.insert(params, mod); + plat.fontMods.insert(params, mod); return *mod; } @@ -168,9 +200,11 @@ DENG2_PIMPL(Font), public Lockable */ PlatformFont const &alteredFont(RichFormat::Iterator const &rich) { + auto &plat = getThreadFonts(); + if (!rich.isDefault()) { - internal::FontParams modParams(font); + internal::FontParams modParams(plat.font); // Size change. if (!fequal(rich.sizeFactor(), 1.f)) @@ -185,12 +219,12 @@ DENG2_PIMPL(Font), public Lockable break; case RichFormat::Regular: - modParams.family = font.family(); + modParams.family = plat.font.family(); modParams.spec.style = NativeFont::Regular; break; case RichFormat::Italic: - modParams.family = font.family(); + modParams.family = plat.font.family(); modParams.spec.style = NativeFont::Italic; break; @@ -199,7 +233,7 @@ DENG2_PIMPL(Font), public Lockable { if (Font const *altFont = rich.format.format().style().richStyleFont(rich.style())) { - modParams = internal::FontParams(altFont->d->font); + modParams = internal::FontParams(altFont->d->getThreadFonts().font); } } break; @@ -214,14 +248,16 @@ DENG2_PIMPL(Font), public Lockable } return getFontMod(modParams); } - return font; + + // No alterations applied. + return plat.font; } }; Font::Font() : d(new Impl(this)) {} -Font::Font(Font const &other) : d(new Impl(this, other.d->font)) +Font::Font(Font const &other) : d(new Impl(this, other.d->referenceFont)) {} Font::Font(QFont const &font) : d(new Impl(this, font)) @@ -307,8 +343,9 @@ QImage Font::rasterize(String const &textLine, Vector4ub fg = foreground; Vector4ub bg = background; + auto const &plat = d->getThreadFonts(); QImage img(QSize(bounds.width(), - de::max(duint(d->font.height()), bounds.height())), + de::max(duint(plat.font.height()), bounds.height())), QImage::Format_ARGB32); img.fill(bgColor.rgba()); @@ -324,7 +361,7 @@ QImage Font::rasterize(String const &textLine, iter.next(); if (iter.range().isEmpty()) continue; - PlatformFont const *font = &d->font; + PlatformFont const *font = &plat.font; if (iter.isDefault()) { diff --git a/doomsday/sdk/libgui/src/text/nativefont.cpp b/doomsday/sdk/libgui/src/text/nativefont.cpp index 78c0ac681c..f7ce64bf0d 100644 --- a/doomsday/sdk/libgui/src/text/nativefont.cpp +++ b/doomsday/sdk/libgui/src/text/nativefont.cpp @@ -33,8 +33,6 @@ DENG2_PIMPL(NativeFont) Style style; int weight; Transform transform; - - Lockable mutex; QHash measureCache; Impl(Public *i) @@ -188,7 +186,6 @@ Rectanglei NativeFont::measure(String const &text) const if (text.size() < MAX_CACHE_STRING_LENGTH) { - DENG2_GUARD_FOR(d->mutex, mtx); auto foundInCache = d->measureCache.constFind(text); if (foundInCache != d->measureCache.constEnd()) { @@ -199,14 +196,12 @@ Rectanglei NativeFont::measure(String const &text) const Rectanglei const bounds = nativeFontMeasure(text); // Remember this for later. - d->mutex.lock(); if (d->measureCache.size() > MAX_CACHE_STRINGS) { // Too many strings, forget everything. d->measureCache.clear(); } d->measureCache.insert(text, bounds); - d->mutex.unlock(); return bounds; }