From 6a1a5ee5c9bab31b72b1e55f78bd5b1a11ad78bb Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Mon, 27 Nov 2017 12:50:28 -0800 Subject: [PATCH 01/14] POC implementation of a CoreText-based LocalGlyphRasterizer. --- platform/darwin/src/local_glyph_rasterizer.mm | 129 ++++++++++++++++++ platform/ios/config.cmake | 2 +- platform/macos/config.cmake | 2 +- 3 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 platform/darwin/src/local_glyph_rasterizer.mm diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm new file mode 100644 index 00000000000..085774f75b1 --- /dev/null +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -0,0 +1,129 @@ +#include +#include + +#import + +namespace { + +template +struct CFHandle { + CFHandle(T t_): t(t_) {} + ~CFHandle() { Releaser(t); } + T operator*() { return t; } + operator bool() { return t; } +private: + T t; +}; + +} // namespace + + +namespace mbgl { + +/* + Initial implementation of darwin TinySDF support: + Draw any CJK glyphs using a default system font + + Where to take this: + - Configure whether to use local fonts, and which fonts to use, + based on map or even style layer options + - Build heuristics for choosing fonts based on input FontStack + (maybe a globally configurable FontStack -> UIFontDescriptor map would make sense? + - Extract glyph metrics so that this can be used with more than just fixed width glyphs +*/ + +bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { + // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics + // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph + return util::i18n::allowsIdeographicBreaking(glyphID); +} + +using CGContextHandle = CFHandle; +using CGColorSpaceHandle = CFHandle; + +Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { + Glyph fixedMetrics; + fixedMetrics.id = glyphID; + + fixedMetrics.metrics.width = 24; + fixedMetrics.metrics.height = 24; + fixedMetrics.metrics.left = 0; + fixedMetrics.metrics.top = -8; + fixedMetrics.metrics.advance = 24; + + uint32_t width = 30; + uint32_t height = 30; + Size size(width, height); + + fixedMetrics.bitmap = AlphaImage(size); + + NSDictionary *fontAttributes = + [NSDictionary dictionaryWithObjectsAndKeys: + [NSNumber numberWithFloat:24.0], (NSString *)kCTFontSizeAttribute, + nil]; + // Create a descriptor. + CTFontDescriptorRef descriptor = + CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes); + + // Create a font using the descriptor. + CTFontRef font = CTFontCreateWithFontDescriptor(descriptor, 0.0, NULL); + CFRelease(descriptor); + + + CFStringRef string = CFStringCreateWithCharacters(NULL, (UniChar*)&glyphID, 1); + + PremultipliedImage image(size); + + CGColorSpaceHandle colorSpace(CGColorSpaceCreateDeviceRGB()); + // TODO: Is there a way to just draw a single alpha channel instead of copying it out of an RGB image? Doesn't seem like the grayscale colorspace is what I'm looking for... + if (!colorSpace) { + throw std::runtime_error("CGColorSpaceCreateDeviceRGB failed"); + } + + constexpr const size_t bitsPerComponent = 8; + constexpr const size_t bytesPerPixel = 4; + const size_t bytesPerRow = bytesPerPixel * width; + + CGContextHandle context(CGBitmapContextCreate( + image.data.get(), + width, + height, + bitsPerComponent, + bytesPerRow, + *colorSpace, + kCGBitmapByteOrderDefault | kCGImageAlphaPremultipliedLast)); + if (!context) { + throw std::runtime_error("CGBitmapContextCreate failed"); + } + + CFStringRef keys[] = { kCTFontAttributeName }; + CFTypeRef values[] = { font }; + + CFDictionaryRef attributes = + CFDictionaryCreate(kCFAllocatorDefault, (const void**)&keys, + (const void**)&values, sizeof(keys) / sizeof(keys[0]), + &kCFTypeDictionaryKeyCallBacks, + &kCFTypeDictionaryValueCallBacks); + + CFAttributedStringRef attrString = + CFAttributedStringCreate(kCFAllocatorDefault, string, attributes); + CFRelease(string); + CFRelease(attributes); + + CTLineRef line = CTLineCreateWithAttributedString(attrString); // TODO: Get glyph runs (for metric extraction) and use showGlyphs API instead? + + // Set text position and draw the line into the graphics context + CGContextSetTextPosition(*context, 0.0, 0.0); + CTLineDraw(line, *context); + CFRelease(line); + CFRelease(font); + CFRelease(string); // TODO: Surely leaking something here, wrap these up! + + for (uint32_t i = 0; i < width * height; i++) { + fixedMetrics.bitmap.data[i] = image.data[4 * i + 3]; // alpha value + } + + return fixedMetrics; +} + +} // namespace mbgl diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake index 3b99211299c..2cc16bbbc2c 100644 --- a/platform/ios/config.cmake +++ b/platform/ios/config.cmake @@ -28,11 +28,11 @@ macro(mbgl_platform_core) # Misc PRIVATE platform/darwin/mbgl/storage/reachability.h PRIVATE platform/darwin/mbgl/storage/reachability.m + PRIVATE platform/darwin/src/local_glyph_rasterizer.mm PRIVATE platform/darwin/src/logging_nslog.mm PRIVATE platform/darwin/src/nsthread.mm PRIVATE platform/darwin/src/string_nsstring.mm PRIVATE platform/default/bidi.cpp - PRIVATE platform/default/local_glyph_rasterizer.cpp PRIVATE platform/default/thread_local.cpp PRIVATE platform/default/utf.cpp diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake index 677c2c5fd53..21663885d4e 100644 --- a/platform/macos/config.cmake +++ b/platform/macos/config.cmake @@ -14,11 +14,11 @@ macro(mbgl_platform_core) # Misc PRIVATE platform/darwin/mbgl/storage/reachability.h PRIVATE platform/darwin/mbgl/storage/reachability.m + PRIVATE platform/darwin/src/local_glyph_rasterizer.mm PRIVATE platform/darwin/src/logging_nslog.mm PRIVATE platform/darwin/src/nsthread.mm PRIVATE platform/darwin/src/string_nsstring.mm PRIVATE platform/default/bidi.cpp - PRIVATE platform/default/local_glyph_rasterizer.cpp PRIVATE platform/default/thread_local.cpp PRIVATE platform/default/utf.cpp From c43dd5782f71bfc4648a3b778ed708089cfb0b05 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Mon, 27 Nov 2017 13:33:53 -0800 Subject: [PATCH 02/14] Fix build for iOS. --- platform/darwin/src/local_glyph_rasterizer.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 085774f75b1..f7304b18f40 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -2,6 +2,8 @@ #include #import +#import +#import namespace { From 629565f06cec87baa16cefd76ff0a40bb8a484cc Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Mon, 27 Nov 2017 16:00:17 -0800 Subject: [PATCH 03/14] Adjust drawing positions and glyph metrics to fit entire glyph within drawn area, better align to baseline. Includes code to extract glyph metrics, but the results don't match anything I'd expect so I'm not using them. --- platform/darwin/src/local_glyph_rasterizer.mm | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index f7304b18f40..043b91fdd53 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -47,14 +47,15 @@ Glyph fixedMetrics; fixedMetrics.id = glyphID; - fixedMetrics.metrics.width = 24; - fixedMetrics.metrics.height = 24; - fixedMetrics.metrics.left = 0; - fixedMetrics.metrics.top = -8; + uint32_t width = 40; + uint32_t height = 40; + + fixedMetrics.metrics.width = width; + fixedMetrics.metrics.height = height; + fixedMetrics.metrics.left = 3; + fixedMetrics.metrics.top = -1; fixedMetrics.metrics.advance = 24; - uint32_t width = 30; - uint32_t height = 30; Size size(width, height); fixedMetrics.bitmap = AlphaImage(size); @@ -114,12 +115,32 @@ CGContextHandle context(CGBitmapContextCreate( CTLineRef line = CTLineCreateWithAttributedString(attrString); // TODO: Get glyph runs (for metric extraction) and use showGlyphs API instead? + CFArrayRef glyphRuns = CTLineGetGlyphRuns(line); + CFIndex runCount = CFArrayGetCount(glyphRuns); + assert(runCount == 1); + CTRunRef run = (CTRunRef)CFArrayGetValueAtIndex(glyphRuns, 0); + CFIndex glyphCount = CTRunGetGlyphCount(run); + assert(glyphCount == 1); + const CGGlyph *glyphs = CTRunGetGlyphsPtr(run); + + CGRect boundingRects[1]; + boundingRects[0] = CGRectMake(0, 0, 0, 0); + CGSize advances[1]; + advances[0] = CGSizeMake(0,0); + CGRect totalBoundingRect = CTFontGetBoundingRectsForGlyphs(font, kCTFontOrientationDefault, glyphs, boundingRects, 1); + double totalAdvance = CTFontGetAdvancesForGlyphs(font, kCTFontOrientationDefault, glyphs, advances, 1); + + // Break in the debugger to see these values: translating from "user coordinates" to bitmap pixel coordinates + // should be OK, but a lot of glyphs seem to have empty bounding boxes...? + (void)totalBoundingRect; + (void)totalAdvance; + // Set text position and draw the line into the graphics context - CGContextSetTextPosition(*context, 0.0, 0.0); + CGContextSetTextPosition(*context, 0.0, 10.0); CTLineDraw(line, *context); + CFRelease(line); CFRelease(font); - CFRelease(string); // TODO: Surely leaking something here, wrap these up! for (uint32_t i = 0; i < width * height; i++) { fixedMetrics.bitmap.data[i] = image.data[4 * i + 3]; // alpha value From ae6dd46bf5af60592947011344277bb6cabbc310 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 11:45:36 -0800 Subject: [PATCH 04/14] Reduce padding on CJK glyphs (should make rendering slightly faster) --- platform/darwin/src/local_glyph_rasterizer.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 043b91fdd53..978bda0cb5b 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -47,8 +47,8 @@ Glyph fixedMetrics; fixedMetrics.id = glyphID; - uint32_t width = 40; - uint32_t height = 40; + uint32_t width = 35; + uint32_t height = 35; fixedMetrics.metrics.width = width; fixedMetrics.metrics.height = height; @@ -136,7 +136,7 @@ CGContextHandle context(CGBitmapContextCreate( (void)totalAdvance; // Set text position and draw the line into the graphics context - CGContextSetTextPosition(*context, 0.0, 10.0); + CGContextSetTextPosition(*context, 0.0, 5.0); CTLineDraw(line, *context); CFRelease(line); From 8e5542812ec9335f160401a5f2123383b3823b4c Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 13:25:15 -0800 Subject: [PATCH 05/14] Moving towards configurable darwin implementation of LocalGlyphRasterizer. Thin C++ wrappers on C calls. --- platform/darwin/src/local_glyph_rasterizer.mm | 157 ++++++++++-------- platform/default/local_glyph_rasterizer.cpp | 9 + src/mbgl/text/glyph_manager.hpp | 2 +- src/mbgl/text/local_glyph_rasterizer.hpp | 6 +- test/text/glyph_manager.test.cpp | 4 + 5 files changed, 109 insertions(+), 69 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 978bda0cb5b..fa5d1a93021 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -22,6 +22,15 @@ namespace mbgl { +using CGContextHandle = CFHandle; +using CGColorSpaceHandle = CFHandle; +using CTFontDescriptorRefHandle = CFHandle; +using CTFontRefHandle = CFHandle; +using CFStringRefHandle = CFHandle; +using CFAttributedStringRefHandle = CFHandle; +using CTLineRefHandle = CFHandle; +using CFDictionaryRefHandle = CFHandle; + /* Initial implementation of darwin TinySDF support: Draw any CJK glyphs using a default system font @@ -34,48 +43,66 @@ - Extract glyph metrics so that this can be used with more than just fixed width glyphs */ -bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { - // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics - // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph - return util::i18n::allowsIdeographicBreaking(glyphID); -} - -using CGContextHandle = CFHandle; -using CGColorSpaceHandle = CFHandle; +class LocalGlyphRasterizer::Impl { +public: + Impl(CTFontRef fontHandle) + : font(fontHandle) + {} -Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { - Glyph fixedMetrics; - fixedMetrics.id = glyphID; - - uint32_t width = 35; - uint32_t height = 35; - - fixedMetrics.metrics.width = width; - fixedMetrics.metrics.height = height; - fixedMetrics.metrics.left = 3; - fixedMetrics.metrics.top = -1; - fixedMetrics.metrics.advance = 24; - - Size size(width, height); + CTFontRefHandle font; +}; - fixedMetrics.bitmap = AlphaImage(size); - +LocalGlyphRasterizer::LocalGlyphRasterizer(void*) +{ NSDictionary *fontAttributes = [NSDictionary dictionaryWithObjectsAndKeys: [NSNumber numberWithFloat:24.0], (NSString *)kCTFontSizeAttribute, nil]; - // Create a descriptor. - CTFontDescriptorRef descriptor = - CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes); - // Create a font using the descriptor. - CTFontRef font = CTFontCreateWithFontDescriptor(descriptor, 0.0, NULL); - CFRelease(descriptor); + CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); + + impl = std::make_unique(CTFontCreateWithFontDescriptor(*descriptor, 0.0, NULL)); +} + +LocalGlyphRasterizer::~LocalGlyphRasterizer() +{} + +bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { + // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics + // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph + return util::i18n::allowsIdeographicBreaking(glyphID); +} +// TODO: In theory we should be able to transform user-coordinate bounding box and advance +// values into pixel glyph metrics. This would remove the need to use fixed glyph metrics +// (which will be slightly off depending on the font), and allow us to return non CJK glyphs +// (which will have variable "advance" values). +void extractGlyphMetrics(CTFontRef font, CTLineRef line) { + CFArrayRef glyphRuns = CTLineGetGlyphRuns(line); + CFIndex runCount = CFArrayGetCount(glyphRuns); + assert(runCount == 1); + CTRunRef run = (CTRunRef)CFArrayGetValueAtIndex(glyphRuns, 0); + CFIndex glyphCount = CTRunGetGlyphCount(run); + assert(glyphCount == 1); + const CGGlyph *glyphs = CTRunGetGlyphsPtr(run); - CFStringRef string = CFStringCreateWithCharacters(NULL, (UniChar*)&glyphID, 1); + CGRect boundingRects[1]; + boundingRects[0] = CGRectMake(0, 0, 0, 0); + CGSize advances[1]; + advances[0] = CGSizeMake(0,0); + CGRect totalBoundingRect = CTFontGetBoundingRectsForGlyphs(font, kCTFontOrientationDefault, glyphs, boundingRects, 1); + double totalAdvance = CTFontGetAdvancesForGlyphs(font, kCTFontOrientationDefault, glyphs, advances, 1); + + // Break in the debugger to see these values: translating from "user coordinates" to bitmap pixel coordinates + // should be OK, but a lot of glyphs seem to have empty bounding boxes...? + (void)totalBoundingRect; + (void)totalAdvance; +} - PremultipliedImage image(size); +PremultipliedImage drawGlyphBitmap(GlyphID glyphID, CTFontRef font, Size size) { + PremultipliedImage rgbaBitmap(size); + + CFStringRefHandle string(CFStringCreateWithCharacters(NULL, reinterpret_cast(&glyphID), 1)); CGColorSpaceHandle colorSpace(CGColorSpaceCreateDeviceRGB()); // TODO: Is there a way to just draw a single alpha channel instead of copying it out of an RGB image? Doesn't seem like the grayscale colorspace is what I'm looking for... @@ -85,12 +112,12 @@ constexpr const size_t bitsPerComponent = 8; constexpr const size_t bytesPerPixel = 4; - const size_t bytesPerRow = bytesPerPixel * width; + const size_t bytesPerRow = bytesPerPixel * size.width; CGContextHandle context(CGBitmapContextCreate( - image.data.get(), - width, - height, + rgbaBitmap.data.get(), + size.width, + size.height, bitsPerComponent, bytesPerRow, *colorSpace, @@ -102,48 +129,44 @@ CGContextHandle context(CGBitmapContextCreate( CFStringRef keys[] = { kCTFontAttributeName }; CFTypeRef values[] = { font }; - CFDictionaryRef attributes = + CFDictionaryRefHandle attributes( CFDictionaryCreate(kCFAllocatorDefault, (const void**)&keys, (const void**)&values, sizeof(keys) / sizeof(keys[0]), &kCFTypeDictionaryKeyCallBacks, - &kCFTypeDictionaryValueCallBacks); + &kCFTypeDictionaryValueCallBacks)); - CFAttributedStringRef attrString = - CFAttributedStringCreate(kCFAllocatorDefault, string, attributes); - CFRelease(string); - CFRelease(attributes); + CFAttributedStringRefHandle attrString(CFAttributedStringCreate(kCFAllocatorDefault, *string, *attributes)); - CTLineRef line = CTLineCreateWithAttributedString(attrString); // TODO: Get glyph runs (for metric extraction) and use showGlyphs API instead? - - CFArrayRef glyphRuns = CTLineGetGlyphRuns(line); - CFIndex runCount = CFArrayGetCount(glyphRuns); - assert(runCount == 1); - CTRunRef run = (CTRunRef)CFArrayGetValueAtIndex(glyphRuns, 0); - CFIndex glyphCount = CTRunGetGlyphCount(run); - assert(glyphCount == 1); - const CGGlyph *glyphs = CTRunGetGlyphsPtr(run); - - CGRect boundingRects[1]; - boundingRects[0] = CGRectMake(0, 0, 0, 0); - CGSize advances[1]; - advances[0] = CGSizeMake(0,0); - CGRect totalBoundingRect = CTFontGetBoundingRectsForGlyphs(font, kCTFontOrientationDefault, glyphs, boundingRects, 1); - double totalAdvance = CTFontGetAdvancesForGlyphs(font, kCTFontOrientationDefault, glyphs, advances, 1); + CTLineRefHandle line(CTLineCreateWithAttributedString(*attrString)); - // Break in the debugger to see these values: translating from "user coordinates" to bitmap pixel coordinates - // should be OK, but a lot of glyphs seem to have empty bounding boxes...? - (void)totalBoundingRect; - (void)totalAdvance; + // For debugging only, doesn't get useful metrics yet + extractGlyphMetrics(font, *line); // Set text position and draw the line into the graphics context CGContextSetTextPosition(*context, 0.0, 5.0); - CTLineDraw(line, *context); + CTLineDraw(*line, *context); - CFRelease(line); - CFRelease(font); + return rgbaBitmap; +} + +Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { + Glyph fixedMetrics; + fixedMetrics.id = glyphID; + + Size size(35, 35); - for (uint32_t i = 0; i < width * height; i++) { - fixedMetrics.bitmap.data[i] = image.data[4 * i + 3]; // alpha value + fixedMetrics.metrics.width = size.width; + fixedMetrics.metrics.height = size.height; + fixedMetrics.metrics.left = 3; + fixedMetrics.metrics.top = -1; + fixedMetrics.metrics.advance = 24; + + PremultipliedImage rgbaBitmap = drawGlyphBitmap(glyphID, *(impl->font), size); + + // Copy alpha values from RGBA bitmap into the AlphaImage output + fixedMetrics.bitmap = AlphaImage(size); + for (uint32_t i = 0; i < size.width * size.height; i++) { + fixedMetrics.bitmap.data[i] = rgbaBitmap.data[4 * i + 3]; } return fixedMetrics; diff --git a/platform/default/local_glyph_rasterizer.cpp b/platform/default/local_glyph_rasterizer.cpp index 7ace6cbfb13..514b6a23cb5 100644 --- a/platform/default/local_glyph_rasterizer.cpp +++ b/platform/default/local_glyph_rasterizer.cpp @@ -2,6 +2,15 @@ namespace mbgl { +class LocalGlyphRasterizer::Impl { +}; + +LocalGlyphRasterizer::LocalGlyphRasterizer(void*) +{} + +LocalGlyphRasterizer::~LocalGlyphRasterizer() +{} + bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID) { return false; } diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp index 194f503ff18..d9a4e64c1d3 100644 --- a/src/mbgl/text/glyph_manager.hpp +++ b/src/mbgl/text/glyph_manager.hpp @@ -25,7 +25,7 @@ class GlyphRequestor { class GlyphManager : public util::noncopyable { public: - GlyphManager(FileSource&, std::unique_ptr = std::make_unique()); + GlyphManager(FileSource&, std::unique_ptr = std::make_unique((void*)NULL)); ~GlyphManager(); // Workers send a `getGlyphs` message to the main thread once they have determined diff --git a/src/mbgl/text/local_glyph_rasterizer.hpp b/src/mbgl/text/local_glyph_rasterizer.hpp index c2bdbd28402..753ebe372a7 100644 --- a/src/mbgl/text/local_glyph_rasterizer.hpp +++ b/src/mbgl/text/local_glyph_rasterizer.hpp @@ -32,11 +32,15 @@ namespace mbgl { class LocalGlyphRasterizer { public: - virtual ~LocalGlyphRasterizer() = default; + virtual ~LocalGlyphRasterizer(); + LocalGlyphRasterizer(void* configuration); // virtual so that test harness can override platform-specific behavior virtual bool canRasterizeGlyph(const FontStack&, GlyphID); virtual Glyph rasterizeGlyph(const FontStack&, GlyphID); +private: + class Impl; + std::unique_ptr impl; }; } // namespace mbgl diff --git a/test/text/glyph_manager.test.cpp b/test/text/glyph_manager.test.cpp index a96e1b970c5..a50e42236f8 100644 --- a/test/text/glyph_manager.test.cpp +++ b/test/text/glyph_manager.test.cpp @@ -19,6 +19,10 @@ static constexpr const size_t stubBitmapLength = 900; class StubLocalGlyphRasterizer : public LocalGlyphRasterizer { public: + StubLocalGlyphRasterizer() + : LocalGlyphRasterizer(0) + {} + bool canRasterizeGlyph(const FontStack&, GlyphID glyphID) { return util::i18n::allowsIdeographicBreaking(glyphID); } From e5c4d9653dd2d34c4c7809c7ca3e429adb151ed4 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 13:42:43 -0800 Subject: [PATCH 06/14] Share CFHandle code between image.mm and local_glyph_rasterizer.mm --- platform/darwin/src/CFHandle.hpp | 27 ++++++++++++++++++ platform/darwin/src/image.mm | 21 +------------- platform/darwin/src/local_glyph_rasterizer.mm | 28 ++++--------------- platform/ios/config.cmake | 1 + platform/macos/config.cmake | 1 + 5 files changed, 35 insertions(+), 43 deletions(-) create mode 100644 platform/darwin/src/CFHandle.hpp diff --git a/platform/darwin/src/CFHandle.hpp b/platform/darwin/src/CFHandle.hpp new file mode 100644 index 00000000000..b87b8c696fd --- /dev/null +++ b/platform/darwin/src/CFHandle.hpp @@ -0,0 +1,27 @@ + +namespace { + +template +struct CFHandle { + CFHandle(T t_): t(t_) {} + ~CFHandle() { Releaser(t); } + T operator*() { return t; } + operator bool() { return t; } +private: + T t; +}; + +using CGImageHandle = CFHandle; +using CFDataHandle = CFHandle; +using CGImageSourceHandle = CFHandle; +using CGDataProviderHandle = CFHandle; +using CGColorSpaceHandle = CFHandle; +using CGContextHandle = CFHandle; + +using CFStringRefHandle = CFHandle; +using CFAttributedStringRefHandle = CFHandle; +using CFDictionaryRefHandle = CFHandle; + + +} // namespace + diff --git a/platform/darwin/src/image.mm b/platform/darwin/src/image.mm index 57b680fbdbb..70655457638 100644 --- a/platform/darwin/src/image.mm +++ b/platform/darwin/src/image.mm @@ -2,26 +2,7 @@ #import -namespace { - -template -struct CFHandle { - CFHandle(T t_): t(t_) {} - ~CFHandle() { Releaser(t); } - T operator*() { return t; } - operator bool() { return t; } -private: - T t; -}; - -} // namespace - -using CGImageHandle = CFHandle; -using CFDataHandle = CFHandle; -using CGImageSourceHandle = CFHandle; -using CGDataProviderHandle = CFHandle; -using CGColorSpaceHandle = CFHandle; -using CGContextHandle = CFHandle; +#import "CFHandle.hpp" CGImageRef CGImageFromMGLPremultipliedImage(mbgl::PremultipliedImage&& src) { // We're converting the PremultipliedImage's backing store to a CGDataProvider, and are taking diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index fa5d1a93021..944fca48c3e 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -5,32 +5,10 @@ #import #import -namespace { - -template -struct CFHandle { - CFHandle(T t_): t(t_) {} - ~CFHandle() { Releaser(t); } - T operator*() { return t; } - operator bool() { return t; } -private: - T t; -}; - -} // namespace - +#import "CFHandle.hpp" namespace mbgl { -using CGContextHandle = CFHandle; -using CGColorSpaceHandle = CFHandle; -using CTFontDescriptorRefHandle = CFHandle; -using CTFontRefHandle = CFHandle; -using CFStringRefHandle = CFHandle; -using CFAttributedStringRefHandle = CFHandle; -using CTLineRefHandle = CFHandle; -using CFDictionaryRefHandle = CFHandle; - /* Initial implementation of darwin TinySDF support: Draw any CJK glyphs using a default system font @@ -43,6 +21,10 @@ - Extract glyph metrics so that this can be used with more than just fixed width glyphs */ +using CTFontDescriptorRefHandle = CFHandle; +using CTFontRefHandle = CFHandle; +using CTLineRefHandle = CFHandle; + class LocalGlyphRasterizer::Impl { public: Impl(CTFontRef fontHandle) diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake index 2cc16bbbc2c..0cfc86d548c 100644 --- a/platform/ios/config.cmake +++ b/platform/ios/config.cmake @@ -28,6 +28,7 @@ macro(mbgl_platform_core) # Misc PRIVATE platform/darwin/mbgl/storage/reachability.h PRIVATE platform/darwin/mbgl/storage/reachability.m + PRIVATE platform/darwin/src/CFHandle.hpp PRIVATE platform/darwin/src/local_glyph_rasterizer.mm PRIVATE platform/darwin/src/logging_nslog.mm PRIVATE platform/darwin/src/nsthread.mm diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake index 21663885d4e..8646a79071c 100644 --- a/platform/macos/config.cmake +++ b/platform/macos/config.cmake @@ -14,6 +14,7 @@ macro(mbgl_platform_core) # Misc PRIVATE platform/darwin/mbgl/storage/reachability.h PRIVATE platform/darwin/mbgl/storage/reachability.m + PRIVATE platform/darwin/src/CFHandle.hpp PRIVATE platform/darwin/src/local_glyph_rasterizer.mm PRIVATE platform/darwin/src/logging_nslog.mm PRIVATE platform/darwin/src/nsthread.mm From 3114947224fd42013954da6c4cef28d31b1238e7 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 14:03:28 -0800 Subject: [PATCH 07/14] LocalGlyphRasterizer takes a void* configuration input. If nothing is set, it doesn't rasterize glyphs locally. --- platform/darwin/src/local_glyph_rasterizer.mm | 22 ++++++++++++------- src/mbgl/text/local_glyph_rasterizer.hpp | 2 +- test/text/glyph_manager.test.cpp | 4 ---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 944fca48c3e..44658b75040 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -34,16 +34,18 @@ CTFontRefHandle font; }; -LocalGlyphRasterizer::LocalGlyphRasterizer(void*) +LocalGlyphRasterizer::LocalGlyphRasterizer(void* configuration) { - NSDictionary *fontAttributes = - [NSDictionary dictionaryWithObjectsAndKeys: - [NSNumber numberWithFloat:24.0], (NSString *)kCTFontSizeAttribute, - nil]; + if (configuration) { + NSMutableDictionary *fontAttributes = CFBridgingRelease((CFDictionaryRef)configuration); + fontAttributes[(NSString *)kCTFontSizeAttribute] = [NSNumber numberWithFloat:24.0]; - CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); + CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); - impl = std::make_unique(CTFontCreateWithFontDescriptor(*descriptor, 0.0, NULL)); + impl = std::make_unique(CTFontCreateWithFontDescriptor(*descriptor, 0.0, NULL)); + } else { + impl = std::make_unique((CTFontRef)NULL); + } } LocalGlyphRasterizer::~LocalGlyphRasterizer() @@ -52,7 +54,7 @@ bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph - return util::i18n::allowsIdeographicBreaking(glyphID); + return *(impl->font) && util::i18n::allowsIdeographicBreaking(glyphID); } // TODO: In theory we should be able to transform user-coordinate bounding box and advance @@ -133,6 +135,10 @@ CFDictionaryRefHandle attributes( Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { Glyph fixedMetrics; + if (!*(impl->font)) { + return fixedMetrics; + } + fixedMetrics.id = glyphID; Size size(35, 35); diff --git a/src/mbgl/text/local_glyph_rasterizer.hpp b/src/mbgl/text/local_glyph_rasterizer.hpp index 753ebe372a7..f43676dfa4f 100644 --- a/src/mbgl/text/local_glyph_rasterizer.hpp +++ b/src/mbgl/text/local_glyph_rasterizer.hpp @@ -33,7 +33,7 @@ namespace mbgl { class LocalGlyphRasterizer { public: virtual ~LocalGlyphRasterizer(); - LocalGlyphRasterizer(void* configuration); + LocalGlyphRasterizer(void* configuration = nullptr); // virtual so that test harness can override platform-specific behavior virtual bool canRasterizeGlyph(const FontStack&, GlyphID); diff --git a/test/text/glyph_manager.test.cpp b/test/text/glyph_manager.test.cpp index a50e42236f8..a96e1b970c5 100644 --- a/test/text/glyph_manager.test.cpp +++ b/test/text/glyph_manager.test.cpp @@ -19,10 +19,6 @@ static constexpr const size_t stubBitmapLength = 900; class StubLocalGlyphRasterizer : public LocalGlyphRasterizer { public: - StubLocalGlyphRasterizer() - : LocalGlyphRasterizer(0) - {} - bool canRasterizeGlyph(const FontStack&, GlyphID glyphID) { return util::i18n::allowsIdeographicBreaking(glyphID); } From bfefc93a3b1f9309f3b403d8a3bed36d62cc416d Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 15:21:40 -0800 Subject: [PATCH 08/14] Change LocalGlyphRasterizer configuration to just a plain "font family" string, plumb it out as far as the Renderer constructor. --- include/mbgl/renderer/renderer.hpp | 3 ++- platform/darwin/src/local_glyph_rasterizer.mm | 10 ++++++---- platform/macos/src/MGLMapView.mm | 12 ++++++------ src/mbgl/renderer/renderer.cpp | 5 +++-- src/mbgl/renderer/renderer_impl.cpp | 5 +++-- src/mbgl/renderer/renderer_impl.hpp | 2 +- src/mbgl/text/glyph_manager.hpp | 2 +- src/mbgl/text/local_glyph_rasterizer.hpp | 2 +- 8 files changed, 23 insertions(+), 18 deletions(-) diff --git a/include/mbgl/renderer/renderer.hpp b/include/mbgl/renderer/renderer.hpp index 0937e9334b8..db28ee92fc4 100644 --- a/include/mbgl/renderer/renderer.hpp +++ b/include/mbgl/renderer/renderer.hpp @@ -25,7 +25,8 @@ class Renderer { public: Renderer(RendererBackend&, float pixelRatio_, FileSource&, Scheduler&, GLContextMode = GLContextMode::Unique, - const optional programCacheDir = {}); + const optional programCacheDir = {}, + const optional localFontFamily = {}); ~Renderer(); void markContextLost(); diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 44658b75040..f12be78daa7 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -34,11 +34,13 @@ CTFontRefHandle font; }; -LocalGlyphRasterizer::LocalGlyphRasterizer(void* configuration) +LocalGlyphRasterizer::LocalGlyphRasterizer(const optional fontFamily) { - if (configuration) { - NSMutableDictionary *fontAttributes = CFBridgingRelease((CFDictionaryRef)configuration); - fontAttributes[(NSString *)kCTFontSizeAttribute] = [NSNumber numberWithFloat:24.0]; + if (fontFamily) { + NSDictionary *fontAttributes = @{ + (NSString *)kCTFontSizeAttribute: [NSNumber numberWithFloat:24.0], + (NSString *)kCTFontFamilyNameAttribute: [[NSString alloc] initWithCString:fontFamily->c_str() encoding:NSUTF8StringEncoding] + }; CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index 8df6f4545d3..b41b7ab0d29 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -217,15 +217,15 @@ + (void)initialize { - (instancetype)initWithFrame:(NSRect)frameRect { if (self = [super initWithFrame:frameRect]) { - [self commonInit]; + [self commonInit:nil]; self.styleURL = nil; } return self; } -- (instancetype)initWithFrame:(NSRect)frame styleURL:(nullable NSURL *)styleURL { +- (instancetype)initWithFrame:(NSRect)frame styleURL:(nullable NSURL *)styleURL localIdeographFontFamily:(nullable NSString *)fontFamily { if (self = [super initWithFrame:frame]) { - [self commonInit]; + [self commonInit:fontFamily]; self.styleURL = styleURL; } return self; @@ -233,7 +233,7 @@ - (instancetype)initWithFrame:(NSRect)frame styleURL:(nullable NSURL *)styleURL - (instancetype)initWithCoder:(nonnull NSCoder *)decoder { if (self = [super initWithCoder:decoder]) { - [self commonInit]; + [self commonInit:nil]; } return self; } @@ -252,7 +252,7 @@ + (NSArray *)restorableStateKeyPaths { return @[@"camera", @"debugMask"]; } -- (void)commonInit { +- (void)commonInit:(nullable NSString*)fontFamily { _isTargetingInterfaceBuilder = NSProcessInfo.processInfo.mgl_isInterfaceBuilderDesignablesAgent; // Set up cross-platform controllers and resources. @@ -274,7 +274,7 @@ - (void)commonInit { _mbglThreadPool = mbgl::sharedThreadPool(); - auto renderer = std::make_unique(*_mbglView, [NSScreen mainScreen].backingScaleFactor, *mbglFileSource, *_mbglThreadPool, mbgl::GLContextMode::Unique); + auto renderer = std::make_unique(*_mbglView, [NSScreen mainScreen].backingScaleFactor, *mbglFileSource, *_mbglThreadPool, mbgl::GLContextMode::Unique, mbgl::optional(), fontFamily ? std::string([fontFamily UTF8String]) : mbgl::optional()); _rendererFrontend = std::make_unique(std::move(renderer), self, *_mbglView, true); _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, self.size, [NSScreen mainScreen].backingScaleFactor, *mbglFileSource, *_mbglThreadPool, mbgl::MapMode::Continuous, mbgl::ConstrainMode::None, mbgl::ViewportMode::Default); diff --git a/src/mbgl/renderer/renderer.cpp b/src/mbgl/renderer/renderer.cpp index 8953b419f7f..6d086c70b13 100644 --- a/src/mbgl/renderer/renderer.cpp +++ b/src/mbgl/renderer/renderer.cpp @@ -10,9 +10,10 @@ Renderer::Renderer(RendererBackend& backend, FileSource& fileSource_, Scheduler& scheduler_, GLContextMode contextMode_, - const optional programCacheDir_) + const optional programCacheDir_, + const optional localFontFamily_) : impl(std::make_unique(backend, pixelRatio_, fileSource_, scheduler_, - contextMode_, std::move(programCacheDir_))) { + contextMode_, std::move(programCacheDir_), std::move(localFontFamily_))) { } Renderer::~Renderer() { diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp index aa138df6628..4bed0e251b1 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -42,7 +42,8 @@ Renderer::Impl::Impl(RendererBackend& backend_, FileSource& fileSource_, Scheduler& scheduler_, GLContextMode contextMode_, - const optional programCacheDir_) + const optional programCacheDir_, + const optional localFontFamily_) : backend(backend_) , scheduler(scheduler_) , fileSource(fileSource_) @@ -50,7 +51,7 @@ Renderer::Impl::Impl(RendererBackend& backend_, , contextMode(contextMode_) , pixelRatio(pixelRatio_) , programCacheDir(programCacheDir_) - , glyphManager(std::make_unique(fileSource)) + , glyphManager(std::make_unique(fileSource, std::make_unique(localFontFamily_))) , imageManager(std::make_unique()) , lineAtlas(std::make_unique(Size{ 256, 512 })) , imageImpls(makeMutable>>()) diff --git a/src/mbgl/renderer/renderer_impl.hpp b/src/mbgl/renderer/renderer_impl.hpp index 4f8139791c6..5d0200a5df7 100644 --- a/src/mbgl/renderer/renderer_impl.hpp +++ b/src/mbgl/renderer/renderer_impl.hpp @@ -38,7 +38,7 @@ class Renderer::Impl : public GlyphManagerObserver, public RenderSourceObserver{ public: Impl(RendererBackend&, float pixelRatio_, FileSource&, Scheduler&, GLContextMode, - const optional programCacheDir); + const optional programCacheDir, const optional localFontFamily); ~Impl() final; void markContextLost() { diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp index d9a4e64c1d3..d34ec2a61ed 100644 --- a/src/mbgl/text/glyph_manager.hpp +++ b/src/mbgl/text/glyph_manager.hpp @@ -25,7 +25,7 @@ class GlyphRequestor { class GlyphManager : public util::noncopyable { public: - GlyphManager(FileSource&, std::unique_ptr = std::make_unique((void*)NULL)); + GlyphManager(FileSource&, std::unique_ptr); ~GlyphManager(); // Workers send a `getGlyphs` message to the main thread once they have determined diff --git a/src/mbgl/text/local_glyph_rasterizer.hpp b/src/mbgl/text/local_glyph_rasterizer.hpp index f43676dfa4f..82b16b534d0 100644 --- a/src/mbgl/text/local_glyph_rasterizer.hpp +++ b/src/mbgl/text/local_glyph_rasterizer.hpp @@ -33,7 +33,7 @@ namespace mbgl { class LocalGlyphRasterizer { public: virtual ~LocalGlyphRasterizer(); - LocalGlyphRasterizer(void* configuration = nullptr); + LocalGlyphRasterizer(const optional fontFamily = optional()); // virtual so that test harness can override platform-specific behavior virtual bool canRasterizeGlyph(const FontStack&, GlyphID); From 3d7669bafd97cf0838d226eab8fa72bb977537cc Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 15:33:19 -0800 Subject: [PATCH 09/14] Fix unit tests. --- platform/darwin/src/local_glyph_rasterizer.mm | 19 ++++++++++++------- platform/ios/config.cmake | 1 + platform/macos/config.cmake | 1 + src/mbgl/text/glyph_manager.hpp | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index f12be78daa7..a5edecb097d 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -22,16 +22,21 @@ */ using CTFontDescriptorRefHandle = CFHandle; -using CTFontRefHandle = CFHandle; using CTLineRefHandle = CFHandle; class LocalGlyphRasterizer::Impl { public: - Impl(CTFontRef fontHandle) - : font(fontHandle) + Impl(CTFontRef font_) + : font(font_) {} + + ~Impl() { + if (font) { + CFRelease(font); + } + } - CTFontRefHandle font; + CTFontRef font; }; LocalGlyphRasterizer::LocalGlyphRasterizer(const optional fontFamily) @@ -56,7 +61,7 @@ bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph - return *(impl->font) && util::i18n::allowsIdeographicBreaking(glyphID); + return impl->font && util::i18n::allowsIdeographicBreaking(glyphID); } // TODO: In theory we should be able to transform user-coordinate bounding box and advance @@ -137,7 +142,7 @@ CFDictionaryRefHandle attributes( Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { Glyph fixedMetrics; - if (!*(impl->font)) { + if (!impl->font) { return fixedMetrics; } @@ -151,7 +156,7 @@ CFDictionaryRefHandle attributes( fixedMetrics.metrics.top = -1; fixedMetrics.metrics.advance = 24; - PremultipliedImage rgbaBitmap = drawGlyphBitmap(glyphID, *(impl->font), size); + PremultipliedImage rgbaBitmap = drawGlyphBitmap(glyphID, impl->font, size); // Copy alpha values from RGBA bitmap into the AlphaImage output fixedMetrics.bitmap = AlphaImage(size); diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake index 0cfc86d548c..55de8bc398c 100644 --- a/platform/ios/config.cmake +++ b/platform/ios/config.cmake @@ -85,6 +85,7 @@ macro(mbgl_platform_core) target_link_libraries(mbgl-core PUBLIC "-lz" PUBLIC "-framework Foundation" + PUBLIC "-framework CoreText" PUBLIC "-framework CoreGraphics" PUBLIC "-framework OpenGLES" PUBLIC "-framework ImageIO" diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake index 8646a79071c..a91719b9d4b 100644 --- a/platform/macos/config.cmake +++ b/platform/macos/config.cmake @@ -65,6 +65,7 @@ macro(mbgl_platform_core) target_link_libraries(mbgl-core PUBLIC "-lz" PUBLIC "-framework Foundation" + PUBLIC "-framework CoreText" PUBLIC "-framework CoreGraphics" PUBLIC "-framework OpenGL" PUBLIC "-framework ImageIO" diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp index d34ec2a61ed..84db2c4be50 100644 --- a/src/mbgl/text/glyph_manager.hpp +++ b/src/mbgl/text/glyph_manager.hpp @@ -25,7 +25,7 @@ class GlyphRequestor { class GlyphManager : public util::noncopyable { public: - GlyphManager(FileSource&, std::unique_ptr); + GlyphManager(FileSource&, std::unique_ptr = std::make_unique(optional())); ~GlyphManager(); // Workers send a `getGlyphs` message to the main thread once they have determined From 8a9009c175830a8cc6ad2fcbbd7e6121258be440 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 28 Nov 2017 15:40:58 -0800 Subject: [PATCH 10/14] Fix non ios/macos builds. --- platform/default/local_glyph_rasterizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/default/local_glyph_rasterizer.cpp b/platform/default/local_glyph_rasterizer.cpp index 514b6a23cb5..7866f294205 100644 --- a/platform/default/local_glyph_rasterizer.cpp +++ b/platform/default/local_glyph_rasterizer.cpp @@ -5,7 +5,7 @@ namespace mbgl { class LocalGlyphRasterizer::Impl { }; -LocalGlyphRasterizer::LocalGlyphRasterizer(void*) +LocalGlyphRasterizer::LocalGlyphRasterizer(const optional) {} LocalGlyphRasterizer::~LocalGlyphRasterizer() From 6451d4a7a368058defec98d96aa1234fe1f9b6b4 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 29 Nov 2017 09:57:47 -0800 Subject: [PATCH 11/14] Document CFHandle Add pragma once Move instantiations of CFHandle down to where they're used (although a few are duplicated). --- platform/darwin/src/CFHandle.hpp | 40 ++++++++++--------- platform/darwin/src/image.mm | 7 ++++ platform/darwin/src/local_glyph_rasterizer.mm | 5 +++ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/platform/darwin/src/CFHandle.hpp b/platform/darwin/src/CFHandle.hpp index b87b8c696fd..edcc9aafdff 100644 --- a/platform/darwin/src/CFHandle.hpp +++ b/platform/darwin/src/CFHandle.hpp @@ -1,27 +1,31 @@ +#pragma once + +/* + CFHandle is a minimal wrapper designed to hold and release CoreFoundation-style handles + It is non-transferrable: wrap it in something like a unique_ptr if you need to pass it around, + or just use unique_ptr with a custom deleter. + CFHandle has no special treatment for null handles -- be careful not to let it hold a null + handle if the behavior of the Releaser isn't defined for null. + + ex: + using CFDataHandle = CFHandle; + + CFDataHandle data(CFDataCreateWithBytesNoCopy( + kCFAllocatorDefault, reinterpret_cast(source.data()), source.size(), + kCFAllocatorNull)); +*/ namespace { -template +template struct CFHandle { - CFHandle(T t_): t(t_) {} - ~CFHandle() { Releaser(t); } - T operator*() { return t; } - operator bool() { return t; } + CFHandle(HandleType handle_): handle(handle_) {} + ~CFHandle() { Releaser(handle); } + HandleType operator*() { return handle; } + operator bool() { return handle; } private: - T t; + HandleType handle; }; -using CGImageHandle = CFHandle; -using CFDataHandle = CFHandle; -using CGImageSourceHandle = CFHandle; -using CGDataProviderHandle = CFHandle; -using CGColorSpaceHandle = CFHandle; -using CGContextHandle = CFHandle; - -using CFStringRefHandle = CFHandle; -using CFAttributedStringRefHandle = CFHandle; -using CFDictionaryRefHandle = CFHandle; - - } // namespace diff --git a/platform/darwin/src/image.mm b/platform/darwin/src/image.mm index 70655457638..8c0d5fa484d 100644 --- a/platform/darwin/src/image.mm +++ b/platform/darwin/src/image.mm @@ -4,6 +4,13 @@ #import "CFHandle.hpp" +using CGImageHandle = CFHandle; +using CFDataHandle = CFHandle; +using CGImageSourceHandle = CFHandle; +using CGDataProviderHandle = CFHandle; +using CGColorSpaceHandle = CFHandle; +using CGContextHandle = CFHandle; + CGImageRef CGImageFromMGLPremultipliedImage(mbgl::PremultipliedImage&& src) { // We're converting the PremultipliedImage's backing store to a CGDataProvider, and are taking // over ownership of the memory. diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index a5edecb097d..9e05235a8e3 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -21,6 +21,11 @@ - Extract glyph metrics so that this can be used with more than just fixed width glyphs */ +using CGColorSpaceHandle = CFHandle; +using CGContextHandle = CFHandle; +using CFStringRefHandle = CFHandle; +using CFAttributedStringRefHandle = CFHandle; +using CFDictionaryRefHandle = CFHandle; using CTFontDescriptorRefHandle = CFHandle; using CTLineRefHandle = CFHandle; From bf53bf42c10663268833e5f809e1f785f3a45019 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 29 Nov 2017 10:53:48 -0800 Subject: [PATCH 12/14] Load fonts per-fontstack (but still no heuristics for choosing font weight) --- platform/darwin/src/local_glyph_rasterizer.mm | 100 +++++++++++------- src/mbgl/util/i18n.cpp | 5 + src/mbgl/util/i18n.hpp | 4 + 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 9e05235a8e3..3d30bdae487 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -1,6 +1,8 @@ #include #include +#include + #import #import #import @@ -10,15 +12,27 @@ namespace mbgl { /* - Initial implementation of darwin TinySDF support: - Draw any CJK glyphs using a default system font + Darwin implementation of LocalGlyphRasterizer: + Draws CJK glyphs using locally available fonts. + + Mirrors GL JS implementation in that: + - Only CJK glyphs are drawn locally (because we can guess their metrics effectively) + * Render size/metrics determined experimentally by rendering a few different fonts + - Configuration is done at map creation time by setting a "font family" + * JS uses a CSS font-family, this uses kCTFontFamilyNameAttribute which has + somewhat different behavior. + - We use heuristics to extract a font-weight based on the incoming font stack - Where to take this: - - Configure whether to use local fonts, and which fonts to use, - based on map or even style layer options - - Build heuristics for choosing fonts based on input FontStack - (maybe a globally configurable FontStack -> UIFontDescriptor map would make sense? - - Extract glyph metrics so that this can be used with more than just fixed width glyphs + Further improvements are possible: + - If we could reliably extract glyph metrics, we wouldn't be limited to CJK glyphs + - We could push the font configuration down to individual style layers, which would + allow any current style to be reproducible using local fonts. + - Instead of just exposing "font family" as a configuration, we could expose a richer + CTFontDescriptor configuration option (although we'd have to override font size to + make sure it stayed at 24pt). + - Because Apple exposes glyph paths via `CTFontCreatePathForGlyph` we could potentially + render directly to SDF instead of going through TinySDF -- although it's not clear + how much of an improvement it would be. */ using CGColorSpaceHandle = CFHandle; @@ -31,44 +45,56 @@ class LocalGlyphRasterizer::Impl { public: - Impl(CTFontRef font_) - : font(font_) + Impl(const optional fontFamily_) + : fontFamily(fontFamily_) {} ~Impl() { - if (font) { - CFRelease(font); + for (auto& pair : fontHandles) { + CFRelease(pair.second); } } - CTFontRef font; + + CTFontRef getFont(const FontStack& fontStack) { + if (!fontFamily) { + return NULL; + } + + if (fontHandles.find(fontStack) == fontHandles.end()) { + NSDictionary *fontAttributes = @{ + (NSString *)kCTFontSizeAttribute: [NSNumber numberWithFloat:24.0], + (NSString *)kCTFontFamilyNameAttribute: [[NSString alloc] initWithCString:fontFamily->c_str() encoding:NSUTF8StringEncoding] + }; + + CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); + CTFontRef font = CTFontCreateWithFontDescriptor(*descriptor, 0.0, NULL); + if (!font) { + throw std::runtime_error("CTFontCreateWithFontDescriptor failed"); + } + + fontHandles[fontStack] = font; + } + return fontHandles[fontStack]; + } + +private: + std::unordered_map fontHandles; + optional fontFamily; }; LocalGlyphRasterizer::LocalGlyphRasterizer(const optional fontFamily) -{ - if (fontFamily) { - NSDictionary *fontAttributes = @{ - (NSString *)kCTFontSizeAttribute: [NSNumber numberWithFloat:24.0], - (NSString *)kCTFontFamilyNameAttribute: [[NSString alloc] initWithCString:fontFamily->c_str() encoding:NSUTF8StringEncoding] - }; - - CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); - - impl = std::make_unique(CTFontCreateWithFontDescriptor(*descriptor, 0.0, NULL)); - } else { - impl = std::make_unique((CTFontRef)NULL); - } -} + : impl(std::make_unique(fontFamily)) +{} LocalGlyphRasterizer::~LocalGlyphRasterizer() {} -bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) { - // TODO: This is a rough approximation of the set of glyphs that will work with fixed glyph metrics - // Either narrow this down to be conservative, or actually extract glyph metrics in rasterizeGlyph - return impl->font && util::i18n::allowsIdeographicBreaking(glyphID); +bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack& fontStack, GlyphID glyphID) { + return util::i18n::allowsFixedWidthGlyphGeneration(glyphID) && impl->getFont(fontStack); } +/* // TODO: In theory we should be able to transform user-coordinate bounding box and advance // values into pixel glyph metrics. This would remove the need to use fixed glyph metrics // (which will be slightly off depending on the font), and allow us to return non CJK glyphs @@ -94,6 +120,7 @@ void extractGlyphMetrics(CTFontRef font, CTLineRef line) { (void)totalBoundingRect; (void)totalAdvance; } +*/ PremultipliedImage drawGlyphBitmap(GlyphID glyphID, CTFontRef font, Size size) { PremultipliedImage rgbaBitmap(size); @@ -136,18 +163,19 @@ CFDictionaryRefHandle attributes( CTLineRefHandle line(CTLineCreateWithAttributedString(*attrString)); // For debugging only, doesn't get useful metrics yet - extractGlyphMetrics(font, *line); + //extractGlyphMetrics(font, *line); - // Set text position and draw the line into the graphics context + // Start drawing a little bit below the top of the bitmap CGContextSetTextPosition(*context, 0.0, 5.0); CTLineDraw(*line, *context); return rgbaBitmap; } -Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack&, GlyphID glyphID) { +Glyph LocalGlyphRasterizer::rasterizeGlyph(const FontStack& fontStack, GlyphID glyphID) { Glyph fixedMetrics; - if (!impl->font) { + CTFontRef font = impl->getFont(fontStack); + if (!font) { return fixedMetrics; } @@ -161,7 +189,7 @@ CFDictionaryRefHandle attributes( fixedMetrics.metrics.top = -1; fixedMetrics.metrics.advance = 24; - PremultipliedImage rgbaBitmap = drawGlyphBitmap(glyphID, impl->font, size); + PremultipliedImage rgbaBitmap = drawGlyphBitmap(glyphID, font, size); // Copy alpha values from RGBA bitmap into the AlphaImage output fixedMetrics.bitmap = AlphaImage(size); diff --git a/src/mbgl/util/i18n.cpp b/src/mbgl/util/i18n.cpp index 3e3a68e2484..1fc13bfb7d8 100644 --- a/src/mbgl/util/i18n.cpp +++ b/src/mbgl/util/i18n.cpp @@ -392,6 +392,11 @@ bool allowsIdeographicBreaking(char16_t chr) { // || isInCJKCompatibilityIdeographsSupplement(chr)); } +bool allowsFixedWidthGlyphGeneration(char16_t chr) { + // Mirrors conservative set of characters used in glyph_manager.js/_tinySDF + return isInCJKUnifiedIdeographs(chr) || isInHangulSyllables(chr); +} + bool allowsVerticalWritingMode(const std::u16string& string) { for (char32_t chr : string) { if (hasUprightVerticalOrientation(chr)) { diff --git a/src/mbgl/util/i18n.hpp b/src/mbgl/util/i18n.hpp index 61c5a1ea962..b3960c743ca 100644 --- a/src/mbgl/util/i18n.hpp +++ b/src/mbgl/util/i18n.hpp @@ -23,6 +23,10 @@ bool allowsIdeographicBreaking(const std::u16string& string); by the given Unicode codepoint due to ideographic breaking. */ bool allowsIdeographicBreaking(char16_t chr); +/** Conservative set of characters expected to have relatively fixed sizes and + advances */ +bool allowsFixedWidthGlyphGeneration(char16_t chr); + /** Returns whether any substring of the given string can be drawn as vertical text with upright glyphs. */ bool allowsVerticalWritingMode(const std::u16string& string); From 0845907fad7d644cfcd5c03ef7ef8acae222e1be Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 29 Nov 2017 12:54:58 -0800 Subject: [PATCH 13/14] Implement font stack-based heuristics for font loading. In my tests so far, I have yet to get CoreText to render anything differently based on the font weight changes. --- platform/darwin/src/local_glyph_rasterizer.mm | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/platform/darwin/src/local_glyph_rasterizer.mm b/platform/darwin/src/local_glyph_rasterizer.mm index 3d30bdae487..fde0193b863 100644 --- a/platform/darwin/src/local_glyph_rasterizer.mm +++ b/platform/darwin/src/local_glyph_rasterizer.mm @@ -1,5 +1,6 @@ #include #include +#include #include @@ -62,9 +63,14 @@ CTFontRef getFont(const FontStack& fontStack) { } if (fontHandles.find(fontStack) == fontHandles.end()) { + + NSDictionary* fontTraits = @{ (NSString *)kCTFontSymbolicTrait: [NSNumber numberWithFloat:getFontWeight(fontStack)] }; + NSDictionary *fontAttributes = @{ (NSString *)kCTFontSizeAttribute: [NSNumber numberWithFloat:24.0], - (NSString *)kCTFontFamilyNameAttribute: [[NSString alloc] initWithCString:fontFamily->c_str() encoding:NSUTF8StringEncoding] + (NSString *)kCTFontFamilyNameAttribute: [[NSString alloc] initWithCString:fontFamily->c_str() encoding:NSUTF8StringEncoding], + (NSString *)kCTFontTraitsAttribute: fontTraits + //(NSString *)kCTFontStyleNameAttribute: (getFontWeight(fontStack) > .3) ? @"Bold" : @"Regular" }; CTFontDescriptorRefHandle descriptor(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)fontAttributes)); @@ -79,6 +85,30 @@ CTFontRef getFont(const FontStack& fontStack) { } private: + float getFontWeight(const FontStack& fontStack) { + // Analog to logic in glyph_manager.js + // From NSFontDescriptor.h (macOS 10.11+) NSFontWeight*: + constexpr float light = -.4; + constexpr float regular = 0.0; + constexpr float medium = .23; + constexpr float bold = .4; + + float fontWeight = regular; + for (auto font : fontStack) { + // Last font in the fontstack "wins" + std::string lowercaseFont = mbgl::platform::lowercase(font); + if (lowercaseFont.find("bold") != std::string::npos) { + fontWeight = bold; + } else if (lowercaseFont.find("medium") != std::string::npos) { + fontWeight = medium; + } else if (lowercaseFont.find("light") != std::string::npos) { + fontWeight = light; + } + } + + return fontWeight; + } + std::unordered_map fontHandles; optional fontFamily; }; From 34e7ae26f47699babc5b01a58616ca464c8010d6 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Wed, 29 Nov 2017 12:56:11 -0800 Subject: [PATCH 14/14] Remove entry point for setting local ideographic font family, so that tests pass. --- platform/macos/src/MGLMapView.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index b41b7ab0d29..484679bbbd1 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -223,9 +223,9 @@ - (instancetype)initWithFrame:(NSRect)frameRect { return self; } -- (instancetype)initWithFrame:(NSRect)frame styleURL:(nullable NSURL *)styleURL localIdeographFontFamily:(nullable NSString *)fontFamily { +- (instancetype)initWithFrame:(NSRect)frame styleURL:(nullable NSURL *)styleURL { if (self = [super initWithFrame:frame]) { - [self commonInit:fontFamily]; + [self commonInit:nil]; self.styleURL = styleURL; } return self;