Skip to content
This repository has been 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
merged 10 commits into from
Jan 3, 2018
Merged

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

merged 10 commits into from
Jan 3, 2018

Conversation

fabian-guerra
Copy link
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.
@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)
@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.
@fabian-guerra
[darwin, macos] Rename Info.plist key for consistency
bf2ecf6 
Also removes related dead code in macos MGLMapView.mm
@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
Copy link
Contributor

akitchen commented Jan 2, 2018

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
Copy link
Contributor

akitchen commented Jan 2, 2018

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
Copy link
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
Copy link
Contributor

akitchen commented Jan 2, 2018

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
Copy link
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
Copy link
Contributor

akitchen commented Jan 3, 2018

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

@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
Copy link
Contributor Author

@akitchen @ChrisLoer added #10712 into this PR

ChrisLoer
ChrisLoer approved these changes Jan 3, 2018
View reviewed changes
Copy link
Contributor

@ChrisLoer ChrisLoer left a comment

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

@ChrisLoer ChrisLoer ChrisLoer approved these changes

@akitchen akitchen Awaiting requested review from akitchen

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