Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Cherry picks client-side rendering of CJK glyphs into release-agua - #10817

Merged
fabian-guerra merged 10 commits into
release-aguafrom
fabian-cp-10522
Jan 3, 2018
Merged

Cherry picks client-side rendering of CJK glyphs into release-agua#10817
fabian-guerra merged 10 commits into
release-aguafrom
fabian-cp-10522

Conversation

@fabian-guerra

@fabian-guerra fabian-guerra commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

Cherry picks the iOS/macOS code from #10522 into release-agua

ChrisLoer and others added 8 commits January 2, 2018 12:06
@ChrisLoer @fabian-guerra
[ios,macos] Darwin implementation of a CoreText-based LocalGlyphRaste…
24c6cf6 
…rizer.

 - Changing font weight does not currently appear to be working.
 - Glyph metric extraction code not working; currently unused.
Andrew Kitchen @fabian-guerra
[ios, macos] Adds support for specifying an ideographic font family name
89bb627 
Adding a MGLIdeographicFontFamilyName to the containing app's Info.plist
will result in CJK glyphs being rasterized on demand (#10522)
Andrew Kitchen @fabian-guerra
[darwin, ios, macos] Introduces an MGLRendererConfiguration class
2113a2c 
Instructions for enabling client-side rendering of CJK glyphs live in
this header, and this class provides the rest of the values needed for
instantiating the renderer on iOS and macOS.
Andrew Kitchen @fabian-guerra
[darwin, macos] Rename Info.plist key for consistency
bf2ecf6 
Also removes related dead code in macos MGLMapView.mm
Andrew Kitchen @fabian-guerra
[macos, ios] Adds documentation for the MGLIdeographFontFamilyName key
b1c0939
@ChrisLoer @fabian-guerra
[ios,macos] Remove dead/non-functioning font-weight and glyph metric …
f746e8f 
…code.

Add local font family to default iosapp configuration.
@ChrisLoer @fabian-guerra
[ios,macos] Update docs to use Apple-friendly "PingFang" font example.
9b5aa61
@ChrisLoer @fabian-guerra
[ios,macos] Revert ideographic->ideograph name change.
31d20aa 
Original GL JS name was meant to represent "font family to use for locally generating ideographs", but "ideographic font family" communicates a similar intent more concisely.
@fabian-guerra fabian-guerra added this to the ios-v3.7.3 milestone Jan 2, 2018
@fabian-guerra fabian-guerra self-assigned this Jan 2, 2018
@fabian-guerra fabian-guerra added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Jan 2, 2018
@akitchen

akitchen commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

Seems straightforward enough. I'm testing the changes locally now.

Note that we'll also want to merge in #10712 in order for this functionality to be enabled by default.

@akitchen

akitchen commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

The build failures are real:

In file included from /Users/bob/workspace/mapbox-gl-native/platform/ios/src/MGLMapView.mm:45:
/Users/bob/workspace/mapbox-gl-native/platform/darwin/src/MGLRendererConfiguration.h:3:9: fatal error: 'mbgl/renderer/mode.hpp' file not found
#import <mbgl/renderer/mode.hpp>
        ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

@ChrisLoer

ChrisLoer commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

After locally changing the mode.hpp include path, it's working locally for me in the ios app.

Note that we'll also want to merge in #10712 in order for this functionality to be enabled by default.

Agree we should port that change, but just to be clear -- it still shouldn't be enabled by default in the SDK, we just want the documentation and iosapp example to be clearer

@akitchen

akitchen commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

it still shouldn't be enabled by default in the SDK, we just want the documentation and iosapp example to be clearer

yes, this simply provides an example of a working configuration for our sample app. The SDK functionality remains disabled by default.

@fabian-guerra

fabian-guerra commented Jan 2, 2018

Copy link
Copy Markdown
Contributor Author

@akitchen @ChrisLoer with the last change this branch is compiling.

@akitchen akitchen changed the title Cherry picks 10522 into release-agua Cherry picks client-side rendering of CJK glyphs into release-agua Jan 3, 2018
@akitchen

akitchen commented Jan 3, 2018

Copy link
Copy Markdown
Contributor

OK, #10712 has been merged into master. @fabian-guerra if you could add that commit I think we'll be good to go.

Andrew Kitchen @fabian-guerra
[ios, macos] Updates documentation with default CJK font recommendations
80f40c5 
Also updates the font to use for rendering CJK ideographs in our sample
apps to `PingFang TC`, as simply specifying `PingFang` was always
triggering iOS's font fallback behavior.

[Fixes #10675]
@fabian-guerra

fabian-guerra commented Jan 3, 2018

Copy link
Copy Markdown
Contributor Author

@akitchen @ChrisLoer added #10712 into this PR

ChrisLoer
ChrisLoer approved these changes Jan 3, 2018
View reviewed changes

@ChrisLoer ChrisLoer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Coveralls failure looks spurious to me.

@fabian-guerra
fabian-guerra merged commit 760ef23 into release-agua Jan 3, 2018
@fabian-guerra
fabian-guerra deleted the fabian-cp-10522 branch January 3, 2018 17:38
@1ec5 1ec5 mentioned this pull request Jan 5, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akitchen akitchen Awaiting requested review from akitchen
1 more reviewer
@ChrisLoer ChrisLoer ChrisLoer approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@fabian-guerra fabian-guerra

Labels

iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS

Projects

None yet

Milestone

ios-v3.7.3

Development

Successfully merging this pull request may close these issues.

3 participants

@fabian-guerra @akitchen @ChrisLoer