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

[ios, macos] Default to local rendering of CJK characters, using system font #14862

Merged
merged 51 commits into from
Jul 10, 2019

Conversation

m-stephen
Copy link
Contributor

@m-stephen m-stephen commented Jun 6, 2019

Closes: #14470
Closes: #14866

Fixed an issue that caused the map to ignore the remote fonts for CJK Kanji glyphs.
Added ability to specify an array of fonts for fallbacks for local ideographic font family.

After this PR merged, we will:

  1. Enabled local font rendering for CJK glyph as default for accelerating map’s loading, using Apple system default font.
  2. If developers set MGLIdeographicFontFamilyName to a string value, which is a custom local font family name, e.g. PingFang TC, we will use that for local rendering.
  3. If developers set MGLIdeographicFontFamilyName to an array of local fonts' family name, we will choose the first available font of that array. If there isn't an available font name, fallback to system default font.
    For example, we will use PingFang TC as a local font:
	<key>MGLIdeographicFontFamilyName</key>
	<array>
		<string>Test</string>
		<string>Test2</string>
		<string>PingFang TC</string>
		<string>Arial</string>
	</array>
  1. If developers set MGLIdeographicFontFamilyName to a boolean value of NO, we will use their remote CJK font.

@m-stephen m-stephen requested a review from 1ec5 as a code owner June 6, 2019 10:11
@m-stephen m-stephen requested a review from a team June 6, 2019 10:11
@m-stephen m-stephen added the iOS Mapbox Maps SDK for iOS label Jun 6, 2019
@m-stephen m-stephen requested review from julianrex and removed request for 1ec5 June 6, 2019 10:12
@friedbunny

This comment has been minimized.

@friedbunny

This comment has been minimized.

@chloekraw

This comment has been minimized.

@m-stephen

This comment has been minimized.

@m-stephen

This comment has been minimized.

@m-stephen

This comment has been minimized.

@chloekraw

This comment has been minimized.

@friedbunny

This comment has been minimized.

@fabian-guerra

This comment has been minimized.

@1ec5 1ec5 added localization Human language support and internationalization macOS Mapbox Maps SDK for macOS text rendering labels Jun 10, 2019
@chloekraw

This comment has been minimized.

@fabian-guerra

This comment has been minimized.

@m-stephen

This comment has been minimized.

@m-stephen

This comment has been minimized.

@m-stephen

This comment has been minimized.

@m-stephen
Copy link
Contributor Author

m-stephen commented Jun 11, 2019

FYI, I also found another issue, we cannot render the Hiragana or Katakana using local font, this is the deep reason why we cannot display Japanese characters well:

We use local font in:

bool LocalGlyphRasterizer::canRasterizeGlyph(const FontStack&, GlyphID glyphID) {
return util::i18n::allowsFixedWidthGlyphGeneration(glyphID) && impl->getFont();
}

and in

bool allowsFixedWidthGlyphGeneration(char16_t chr) {
// Mirrors conservative set of characters used in glyph_manager.js/_tinySDF
return isInCJKUnifiedIdeographs(chr) || isInHangulSyllables(chr);
}

we judge characters whether can rasterize or not.

The isInCJKUnifiedIdeographs is defined by:

DEFINE_IS_IN_UNICODE_BLOCK(CJKUnifiedIdeographs, 0x4E00, 0x9FFF)

The unicodes range is from 4E00 to 9FFF, which means we only render Chinese characters using local font. We should add more ranges of CJK glyphs unicodes, and different range of them should use different Apple default font, same as Apple do.

I’ll make a new commit to fix this issue and need GL team help to review.

m-stephen and others added 16 commits July 9, 2019 10:03
@m-stephen
Copy link
Contributor Author

m-stephen commented Jul 9, 2019

@friedbunny I have added some plist value tests, please re-review, thanks!

@m-stephen m-stephen requested a review from friedbunny July 9, 2019 07:19
Copy link
Contributor

@friedbunny friedbunny left a comment

Choose a reason for hiding this comment

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

Thanks for your patience throughout the process, @m-stephen. 🙇

platform/darwin/test/MGLRendererConfigurationTests.mm Outdated Show resolved Hide resolved
platform/darwin/test/MGLRendererConfigurationTests.mm Outdated Show resolved Hide resolved
@friedbunny friedbunny changed the title [iOS, macOS]Use system default font for Kanji characters as default local rendering strategy [ios, macos] Default to local rendering of CJK characters, using system font Jul 9, 2019
@m-stephen m-stephen merged commit 6b59db8 into master Jul 10, 2019
@m-stephen m-stephen deleted the Stephen-CJK branch July 10, 2019 02:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS localization Human language support and internationalization macOS Mapbox Maps SDK for macOS text rendering
Projects
None yet
6 participants