From 0f4e88f44b24e87a3df4a8aa6f399bb5e6ae95c3 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 7 Oct 2018 00:02:32 -0700 Subject: [PATCH 1/2] fix: overly thin font rendering on mojave --- patches/common/skia/.patches.yaml | 7 + patches/common/skia/fix_font_thickness.patch | 168 +++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 patches/common/skia/fix_font_thickness.patch diff --git a/patches/common/skia/.patches.yaml b/patches/common/skia/.patches.yaml index 5d634838cef41..8599717f065ce 100644 --- a/patches/common/skia/.patches.yaml +++ b/patches/common/skia/.patches.yaml @@ -4,3 +4,10 @@ patches: author: Ales Pergl file: dcheck.patch description: null +- + author: Shelley Vohr + file: fix_font_thickness.patch + description: | + Backports https://skia-review.googlesource.com/c/157566/ to + fix an issue whereby font rendering weight was too thin compared + to other fonts present on MacOS Mojave. diff --git a/patches/common/skia/fix_font_thickness.patch b/patches/common/skia/fix_font_thickness.patch new file mode 100644 index 0000000000000..906cca9557b0d --- /dev/null +++ b/patches/common/skia/fix_font_thickness.patch @@ -0,0 +1,168 @@ +diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp +index 3cc51d43f6..c0b606c004 100644 +--- a/src/ports/SkFontHost_mac.cpp ++++ b/src/ports/SkFontHost_mac.cpp +@@ -377,17 +377,28 @@ static constexpr const uint8_t kSpiderSymbol_ttf[] = { + 0x00, 0x02, 0x00, 0x00 + }; + ++enum class SmoothBehavior { ++ none, // SmoothFonts produces no effect. ++ some, // SmoothFonts produces some effect, but not subpixel coverage. ++ subpixel, // SmoothFonts produces some effect and provides subpixel coverage. ++}; ++ + /** + * There does not appear to be a publicly accessable API for determining if lcd + * font smoothing will be applied if we request it. The main issue is that if + * smoothing is applied a gamma of 2.0 will be used, if not a gamma of 1.0. + */ +-static bool supports_LCD() { +- static bool gSupportsLCD = []{ +- uint32_t bitmap[16][16] = {}; ++static SmoothBehavior smooth_behavior() { ++ static SmoothBehavior gSmoothBehavior = []{ ++ uint32_t noSmoothBitmap[16][16] = {}; ++ uint32_t smoothBitmap[16][16] = {}; + UniqueCFRef colorspace(CGColorSpaceCreateDeviceRGB()); +- UniqueCFRef cgContext( +- CGBitmapContextCreate(&bitmap, 16, 16, 8, 16*4, colorspace.get(), BITMAP_INFO_RGB)); ++ SkUniqueCFRef noSmoothContext( ++ CGBitmapContextCreate(&noSmoothBitmap, 16, 16, 8, 16*4, ++ colorspace.get(), BITMAP_INFO_RGB)); ++ SkUniqueCFRef smoothContext( ++ CGBitmapContextCreate(&smoothBitmap, 16, 16, 8, 16*4, ++ colorspace.get(), BITMAP_INFO_RGB)); + + UniqueCFRef data( + CGDataProviderCreateWithData(nullptr, kSpiderSymbol_ttf, +@@ -398,31 +409,39 @@ static bool supports_LCD() { + CTFontCreateWithGraphicsFont(cgFont.get(), 16, nullptr, nullptr)); + SkASSERT(ctFont); + +- CGContextSetShouldSmoothFonts(cgContext.get(), true); +- CGContextSetShouldAntialias(cgContext.get(), true); +- CGContextSetTextDrawingMode(cgContext.get(), kCGTextFill); +- CGContextSetGrayFillColor(cgContext.get(), 1, 1); ++ CGContextSetShouldSmoothFonts(noSmoothContext.get(), false); ++ CGContextSetShouldAntialias(noSmoothContext.get(), true); ++ CGContextSetTextDrawingMode(noSmoothContext.get(), kCGTextFill); ++ CGContextSetGrayFillColor(noSmoothContext.get(), 1, 1); ++ ++ CGContextSetShouldSmoothFonts(smoothContext.get(), true); ++ CGContextSetShouldAntialias(smoothContext.get(), true); ++ CGContextSetTextDrawingMode(smoothContext.get(), kCGTextFill); ++ CGContextSetGrayFillColor(smoothContext.get(), 1, 1); + CGPoint point = CGPointMake(0, 3); + CGGlyph spiderGlyph = 3; +- CTFontDrawGlyphs(ctFont.get(), &spiderGlyph, &point, 1, cgContext.get()); ++ CTFontDrawGlyphs(ctFont.get(), &spiderGlyph, &point, 1, noSmoothContext.get()); ++ CTFontDrawGlyphs(ctFont.get(), &spiderGlyph, &point, 1, smoothContext.get()); + + // For debugging. +- //UniqueCFRef image(CGBitmapContextCreateImage(cgContext.get())); +- ++ //SkUniqueCFRef image(CGBitmapContextCreateImage(noSmoothContext())); ++ //SkUniqueCFRef image(CGBitmapContextCreateImage(smoothContext())); ++ ++ SmoothBehavior smoothBehavior = SmoothBehavior::none; + for (int x = 0; x < 16; ++x) { + for (int y = 0; y < 16; ++y) { +- uint32_t pixel = bitmap[x][y]; +- uint32_t r = (pixel >> 16) & 0xFF; +- uint32_t g = (pixel >> 8) & 0xFF; +- uint32_t b = (pixel >> 0) & 0xFF; ++ uint32_t smoothPixel = smoothBitmap[x][y]; ++ uint32_t r = (smoothPixel >> 16) & 0xFF; ++ uint32_t g = (smoothPixel >> 8) & 0xFF; ++ uint32_t b = (smoothPixel >> 0) & 0xFF; + if (r != g || r != b) { +- return true; ++ return SmoothBehavior::subpixel; + } + } + } +- return false; ++ return smoothBehavior; + }(); +- return gSupportsLCD; ++ return gSmoothBehavior; + } + + class Offscreen { +@@ -1005,7 +1024,7 @@ CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph& + if (!fRGBSpace) { + //It doesn't appear to matter what color space is specified. + //Regular blends and antialiased text are always (s*a + d*(1-a)) +- //and smoothed text is always g=2.0. ++ //and subpixel antialiased text is always g=2.0. + fRGBSpace.reset(CGColorSpaceCreateDeviceRGB()); + } + +@@ -1260,7 +1279,7 @@ static constexpr uint8_t sk_pow2_table(size_t i) { + * This will invert the gamma applied by CoreGraphics, so we can get linear + * values. + * +- * CoreGraphics obscurely defaults to 2.0 as the smoothing gamma value. ++ * CoreGraphics obscurely defaults to 2.0 as the subpixel coverage gamma value. + * The color space used does not appear to affect this choice. + */ + static constexpr auto gLinearCoverageFromCGLCDValue = SkMakeArray<256>(sk_pow2_table); +@@ -1350,18 +1369,20 @@ void SkScalerContext_Mac::generateImage(const SkGlyph& glyph) { + CGGlyph cgGlyph = SkTo(glyph.getGlyphID()); + + // FIXME: lcd smoothed un-hinted rasterization unsupported. +- bool generateA8FromLCD = fRec.getHinting() != SkPaint::kNo_Hinting; ++ bool requestSmooth = fRec.getHinting() != SkPaint::kNo_Hinting; + + // Draw the glyph + size_t cgRowBytes; +- CGRGBPixel* cgPixels = fOffscreen.getCG(*this, glyph, cgGlyph, &cgRowBytes, generateA8FromLCD); ++ CGRGBPixel* cgPixels = fOffscreen.getCG(*this, glyph, cgGlyph, &cgRowBytes, requestSmooth); + if (cgPixels == nullptr) { + return; + } + + // Fix the glyph + if ((glyph.fMaskFormat == SkMask::kLCD16_Format) || +- (glyph.fMaskFormat == SkMask::kA8_Format && supports_LCD() && generateA8FromLCD)) ++ (glyph.fMaskFormat == SkMask::kA8_Format ++ && requestSmooth ++ && smooth_behavior() == SmoothBehavior::subpixel)) + { + const uint8_t* linear = gLinearCoverageFromCGLCDValue.data(); + +@@ -2257,14 +2278,14 @@ void SkTypeface_Mac::onFilterRec(SkScalerContextRec* rec) const { + + rec->fFlags &= ~flagsWeDontSupport; + +- bool lcdSupport = supports_LCD(); ++ SmoothBehavior smoothBehavior = smooth_behavior(); + + // Only two levels of hinting are supported. + // kNo_Hinting means avoid CoreGraphics outline dilation. + // kNormal_Hinting means CoreGraphics outline dilation is allowed. + // If there is no lcd support, hinting (dilation) cannot be supported. + SkPaint::Hinting hinting = rec->getHinting(); +- if (SkPaint::kSlight_Hinting == hinting || !lcdSupport) { ++ if (SkPaint::kSlight_Hinting == hinting || smoothBehavior == SmoothBehavior::none) { + hinting = SkPaint::kNo_Hinting; + } else if (SkPaint::kFull_Hinting == hinting) { + hinting = SkPaint::kNormal_Hinting; +@@ -2291,12 +2312,15 @@ void SkTypeface_Mac::onFilterRec(SkScalerContextRec* rec) const { + // [LCD][yes-hint]: generate LCD using CoreGraphic's LCD output. + + if (rec->fMaskFormat == SkMask::kLCD16_Format) { +- if (lcdSupport) { ++ if (smoothBehavior == SmoothBehavior::subpixel) { + //CoreGraphics creates 555 masks for smoothed text anyway. + rec->fMaskFormat = SkMask::kLCD16_Format; + rec->setHinting(SkPaint::kNormal_Hinting); + } else { + rec->fMaskFormat = SkMask::kA8_Format; ++ if (smoothBehavior == SmoothBehavior::some) { ++ rec->setHinting(SkPaint::kNormal_Hinting); ++ } + } + } + From 122cf47096a10bb95ac21b53b8e5d05439318d62 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 7 Oct 2018 09:53:43 -0700 Subject: [PATCH 2/2] SkUniqueCFRef => UniqueCFRef --- patches/common/skia/fix_font_thickness.patch | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/patches/common/skia/fix_font_thickness.patch b/patches/common/skia/fix_font_thickness.patch index 906cca9557b0d..858a03747e998 100644 --- a/patches/common/skia/fix_font_thickness.patch +++ b/patches/common/skia/fix_font_thickness.patch @@ -27,10 +27,10 @@ index 3cc51d43f6..c0b606c004 100644 UniqueCFRef colorspace(CGColorSpaceCreateDeviceRGB()); - UniqueCFRef cgContext( - CGBitmapContextCreate(&bitmap, 16, 16, 8, 16*4, colorspace.get(), BITMAP_INFO_RGB)); -+ SkUniqueCFRef noSmoothContext( ++ UniqueCFRef noSmoothContext( + CGBitmapContextCreate(&noSmoothBitmap, 16, 16, 8, 16*4, + colorspace.get(), BITMAP_INFO_RGB)); -+ SkUniqueCFRef smoothContext( ++ UniqueCFRef smoothContext( + CGBitmapContextCreate(&smoothBitmap, 16, 16, 8, 16*4, + colorspace.get(), BITMAP_INFO_RGB)); @@ -62,8 +62,8 @@ index 3cc51d43f6..c0b606c004 100644 // For debugging. - //UniqueCFRef image(CGBitmapContextCreateImage(cgContext.get())); - -+ //SkUniqueCFRef image(CGBitmapContextCreateImage(noSmoothContext())); -+ //SkUniqueCFRef image(CGBitmapContextCreateImage(smoothContext())); ++ //UniqueCFRef image(CGBitmapContextCreateImage(noSmoothContext())); ++ //UniqueCFRef image(CGBitmapContextCreateImage(smoothContext())); + + SmoothBehavior smoothBehavior = SmoothBehavior::none; for (int x = 0; x < 16; ++x) {