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

Add fallback support to local ideograph font families - #15255

Merged
LukasPaczos merged 1 commit into
masterfrom
tvn-font-fallback
Aug 12, 2019
Merged

LukasPaczos merged 1 commit into
masterfrom
tvn-font-fallback

Conversation

@tobrun

@tobrun tobrun commented Jul 30, 2019 •

Copy link
Copy Markdown
Member

This PR matches logic from #14862 and makes #15225 obsolete. Proposed change allows developers to pass in a array of string for localideographfamily and we will check if we can find a valid font and pass that to core. If not, fallback on sans-serif.

Note: I'm currently not satisfied with taken approach, it uses reflection to lookup valid fonts from TypeFace. Sadly, there isn't an API on the Android SDK to do this cleanly. LMK if you are aware of any.

  • optimize for null values, providing a null should result in downloading the fonts.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Jul 30, 2019
@tobrun tobrun added this to the release-queso milestone Jul 30, 2019
@tobrun tobrun self-assigned this Jul 30, 2019
@chloekraw chloekraw added the needs changelog Indicates PR needs a changelog entry prior to merging. label Jul 30, 2019
@tobrun

tobrun commented Jul 31, 2019

Copy link
Copy Markdown
Member Author

While running on Android Q beta 4, I'm seeing:

W/pboxsdk.testap: Accessing hidden field Landroid/graphics/Typeface;->sSystemFontMap:Ljava/util/Map; (greylist, reflection, allowed)

this means that this reflection call is greylisted as is allowed for the time being. Probably because there isn't any other way of checking font avallability outside of the downloadable fonts in https://developers.google.com/fonts/docs/android which requires min api 16 when using the appcompat variant. Long term we will be switching to that setup instead.

@tobrun
tobrun force-pushed the tvn-font-fallback branch from 5cd9e17 to d5ef1c9 Compare July 31, 2019 11:37
@tobrun tobrun removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Jul 31, 2019
@tobrun
tobrun marked this pull request as ready for review July 31, 2019 11:47
@tobrun
tobrun requested a review from LukasPaczos July 31, 2019 11:47
@tobrun

tobrun commented Jul 31, 2019

Copy link
Copy Markdown
Member Author

On request from @LukasPaczos also verified that this solution works with https://developer.android.com/guide/topics/ui/look-and-feel/downloadable-fonts

@LukasPaczos LukasPaczos 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.

Looks good, thanks for running additional tests 👍

@tobrun
tobrun force-pushed the tvn-font-fallback branch from d5ef1c9 to 719e3f3 Compare August 1, 2019 08:00
@chloekraw chloekraw added the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 1, 2019
@tobrun
tobrun force-pushed the tvn-font-fallback branch from 719e3f3 to 451c98f Compare August 1, 2019 12:57
@tobrun tobrun removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 1, 2019
@tobrun
tobrun force-pushed the tvn-font-fallback branch from 451c98f to 0c7223a Compare August 1, 2019 14:19
@LukasPaczos
LukasPaczos merged commit 58d7734 into master Aug 12, 2019
@LukasPaczos
LukasPaczos deleted the tvn-font-fallback branch August 12, 2019 11:30
@ghost

ghost commented Jan 6, 2020

Copy link
Copy Markdown

Hi! Do we have any update on this? I would like to know if and how I can use local fonts on my symbol layers. Thanks!

@LukasPaczos

Copy link
Copy Markdown
Contributor

Hey @EBiCS, thanks for using Mapbox! This PR references local ideographs generation, which means that the SDK will not download CJK glyphs but rather generate them locally. If you'd like to use a certain font, it has to be declared in your style using Mapbox Studio.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Android Mapbox Maps SDK for Android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants