From 05a729ff90f75ac013873742ee0e4ae7e5e6e415 Mon Sep 17 00:00:00 2001 From: "bungeman@google.com" Date: Thu, 20 Jun 2013 15:29:16 +0000 Subject: [PATCH] Fix limit on size of glyph paths. Some web fonts have been observed to contain glyphs which require a buffer greater than 20KB. In these rare cases we should allocate space on the heap. Most glyphs require less than 8KB, and Windows has a 1MB stack, so in the common case just use the stack. This change also removes the gFTMutex which was both poorly named and used. The constructor does not appear to have any need for it, and with this change the path generation code does not need it. The mask generating code does need memory barriers for correctness, but these bariers are no-ops on x86. We will need another change to clean up this sort of static initialization code. R=caryclark@google.com Review URL: https://codereview.chromium.org/17435003 git-svn-id: http://skia.googlecode.com/svn/trunk@9700 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/ports/SkFontHost_win.cpp | 105 +++++++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 34 deletions(-) diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp index 44f9530..3a7f518 100755 --- a/src/ports/SkFontHost_win.cpp +++ b/src/ports/SkFontHost_win.cpp @@ -19,6 +19,7 @@ #include "SkPath.h" #include "SkStream.h" #include "SkString.h" +#include "SkTemplates.h" #include "SkThread.h" #include "SkTypeface_win.h" #include "SkTypefaceCache.h" @@ -46,18 +47,11 @@ static void call_ensure_accessible(const LOGFONT& lf) { // always packed xxRRGGBB typedef uint32_t SkGdiRGB; -template T* SkTAddByteOffset(T* ptr, size_t byteOffset) { - return (T*)((char*)ptr + byteOffset); -} - // define this in your Makefile or .gyp to enforce AA requests // which GDI ignores at small sizes. This flag guarantees AA // for rotated text, regardless of GDI's notions. //#define SK_ENFORCE_ROTATED_TEXT_AA_ON_WINDOWS -// client3d has to undefine this for now -#define CAN_USE_LOGFONT_NAME - static bool isLCD(const SkScalerContext::Rec& rec) { return SkMask::kLCD16_Format == rec.fMaskFormat || SkMask::kLCD32_Format == rec.fMaskFormat; @@ -91,9 +85,6 @@ static bool needToRenderWithSkia(const SkScalerContext::Rec& rec) { using namespace skia_advanced_typeface_metrics_utils; -static const uint16_t BUFFERSIZE = (16384 - 32); -static uint8_t glyphbuf[BUFFERSIZE]; - /** * Since LOGFONT wants its textsize as an int, and we support fractional sizes, * and since we have a cache of LOGFONTs for our tyepfaces, we always set the @@ -554,8 +545,6 @@ static FIXED float2FIXED(float x) { return SkFixedToFIXED(SkFloatToFixed(x)); } -SK_DECLARE_STATIC_MUTEX(gFTMutex); - #define HIRES_TEXTSIZE 2048 #define HIRES_SHIFT 11 static inline SkFixed HiResToFixed(int value) { @@ -589,9 +578,8 @@ SkScalerContext_Windows::SkScalerContext_Windows(SkTypeface* rawTypeface, , fFont(0) , fSavefont(0) , fSC(0) - , fGlyphCount(-1) { - SkAutoMutexAcquire ac(gFTMutex); - + , fGlyphCount(-1) +{ LogFontTypeface* typeface = reinterpret_cast(rawTypeface); fDDC = ::CreateCompatibleDC(NULL); @@ -924,10 +912,22 @@ static void build_power_table(uint8_t table[], float ee) { * that shifting into other color spaces is imprecise. */ static const uint8_t* getInverseGammaTableGDI() { - static bool gInited; + // Since build_power_table is idempotent, many threads can build gTableGdi + // simultaneously. + + // Microsoft Specific: + // Making gInited volatile provides read-aquire and write-release in vc++. + // In VS2012, see compiler option /volatile:(ms|iso). + // Replace with C++11 atomics when possible. + static volatile bool gInited; static uint8_t gTableGdi[256]; - if (!gInited) { + if (gInited) { + // Need a L/L (read) barrier (full acquire not needed). If gInited is observed + // true then gTableGdi is observable, but it must be requested. + } else { build_power_table(gTableGdi, 2.3f); + // Need a S/S (write) barrier (full release not needed) here so that this + // write to gInited becomes observable after gTableGdi. gInited = true; } return gTableGdi; @@ -941,15 +941,27 @@ static const uint8_t* getInverseGammaTableGDI() { * If this value is not specified, the default is a gamma of 1.4. */ static const uint8_t* getInverseGammaTableClearType() { - static bool gInited; + // We don't expect SPI_GETFONTSMOOTHINGCONTRAST to ever change, so building + // gTableClearType with build_power_table is effectively idempotent. + + // Microsoft Specific: + // Making gInited volatile provides read-aquire and write-release in vc++. + // In VS2012, see compiler option /volatile:(ms|iso). + // Replace with C++11 atomics when possible. + static volatile bool gInited; static uint8_t gTableClearType[256]; - if (!gInited) { + if (gInited) { + // Need a L/L (read) barrier (acquire not needed). If gInited is observed + // true then gTableClearType is observable, but it must be requested. + } else { UINT level = 0; if (!SystemParametersInfo(SPI_GETFONTSMOOTHINGCONTRAST, 0, &level, 0) || !level) { // can't get the data, so use a default level = 1400; } build_power_table(gTableClearType, level / 1000.0f); + // Need a S/S (write) barrier (release not needed) here so that this + // write to gInited becomes observable after gTableClearType. gInited = true; } return gTableClearType; @@ -1005,7 +1017,7 @@ static bool is_rgb_really_bw(const SkGdiRGB* src, int width, int height, int src return false; } } - src = SkTAddByteOffset(src, srcRB); + src = SkTAddOffset(src, srcRB); } return true; } @@ -1050,7 +1062,7 @@ static void rgb_to_bw(const SkGdiRGB* SK_RESTRICT src, size_t srcRB, } dst[byteCount] = byte; } - src = SkTAddByteOffset(src, srcRB); + src = SkTAddOffset(src, srcRB); dst -= dstRB; } } @@ -1066,7 +1078,7 @@ static void rgb_to_a8(const SkGdiRGB* SK_RESTRICT src, size_t srcRB, for (int i = 0; i < width; i++) { dst[i] = rgb_to_a8(src[i], table8); } - src = SkTAddByteOffset(src, srcRB); + src = SkTAddOffset(src, srcRB); dst -= dstRB; } } @@ -1082,7 +1094,7 @@ static void rgb_to_lcd16(const SkGdiRGB* SK_RESTRICT src, size_t srcRB, const Sk for (int i = 0; i < width; i++) { dst[i] = rgb_to_lcd16(src[i], tableR, tableG, tableB); } - src = SkTAddByteOffset(src, srcRB); + src = SkTAddOffset(src, srcRB); dst = (uint16_t*)((char*)dst - dstRB); } } @@ -1098,7 +1110,7 @@ static void rgb_to_lcd32(const SkGdiRGB* SK_RESTRICT src, size_t srcRB, const Sk for (int i = 0; i < width; i++) { dst[i] = rgb_to_lcd32(src[i], tableR, tableG, tableB); } - src = SkTAddByteOffset(src, srcRB); + src = SkTAddOffset(src, srcRB); dst = (uint32_t*)((char*)dst - dstRB); } } @@ -1109,7 +1121,6 @@ static inline unsigned clamp255(unsigned x) { } void SkScalerContext_Windows::generateImage(const SkGlyph& glyph) { - SkAutoMutexAcquire ac(gFTMutex); SkASSERT(fDDC); const bool isBW = SkMask::kBW_Format == fRec.fMaskFormat; @@ -1148,7 +1159,7 @@ void SkScalerContext_Windows::generateImage(const SkGlyph& glyph) { int b = (addr[x] >> 0) & 0xFF; addr[x] = (table[r] << 16) | (table[g] << 8) | table[b]; } - addr = SkTAddByteOffset(addr, srcRB); + addr = SkTAddOffset(addr, srcRB); } } @@ -1200,22 +1211,48 @@ void SkScalerContext_Windows::generateImage(const SkGlyph& glyph) { } void SkScalerContext_Windows::generatePath(const SkGlyph& glyph, SkPath* path) { - - SkAutoMutexAcquire ac(gFTMutex); - SkASSERT(&glyph && path); SkASSERT(fDDC); path->reset(); GLYPHMETRICS gm; - uint32_t total_size = GetGlyphOutlineW(fDDC, glyph.fID, GGO_NATIVE | GGO_GLYPH_INDEX, &gm, BUFFERSIZE, glyphbuf, &fMat22); + + // Out of all the fonts on a typical Windows box, + // 25% of glyphs require more than 2KB. + // 1% of glyphs require more than 4KB. + // 0.01% of glyphs require more than 8KB. + // 8KB is less than 1% of the normal 1MB stack on Windows. + // Note that some web fonts glyphs require more than 20KB. + static const uint16_t BUFFERSIZE = (1 << 13); + SkAutoSTMalloc glyphbuf(BUFFERSIZE); + + const UINT flags = GGO_NATIVE | GGO_GLYPH_INDEX; + DWORD total_size = GetGlyphOutlineW(fDDC, glyph.fID, flags, &gm, BUFFERSIZE, glyphbuf, &fMat22); if (GDI_ERROR == total_size) { - LogFontTypeface::EnsureAccessible(this->getTypeface()); - total_size = GetGlyphOutlineW(fDDC, glyph.fID, GGO_NATIVE | GGO_GLYPH_INDEX, &gm, BUFFERSIZE, glyphbuf, &fMat22); + // GDI_ERROR because the BUFFERSIZE was too small, or because the data was not accessible. + // When the data is not accessable GetGlyphOutlineW fails rather quickly, + // so just try to get the size. If that fails then ensure the data is accessible. + total_size = GetGlyphOutlineW(fDDC, glyph.fID, flags, &gm, 0, NULL, &fMat22); if (GDI_ERROR == total_size) { - SkASSERT(false); - return; + LogFontTypeface::EnsureAccessible(this->getTypeface()); + total_size = GetGlyphOutlineW(fDDC, glyph.fID, flags, &gm, 0, NULL, &fMat22); + if (GDI_ERROR == total_size) { + SkASSERT(false); + return; + } + } + + glyphbuf.reset(total_size); + + DWORD ret = GetGlyphOutlineW(fDDC, glyph.fID, flags, &gm, total_size, glyphbuf, &fMat22); + if (GDI_ERROR == ret) { + LogFontTypeface::EnsureAccessible(this->getTypeface()); + ret = GetGlyphOutlineW(fDDC, glyph.fID, flags, &gm, total_size, glyphbuf, &fMat22); + if (GDI_ERROR == ret) { + SkASSERT(false); + return; + } } } -- 2.7.4