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

Don't force downloading of Open Sans fonts #11055

Merged
merged 1 commit into from
Jan 26, 2018
Merged

Don't force downloading of Open Sans fonts #11055

merged 1 commit into from
Jan 26, 2018

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Jan 25, 2018

When a SymbolLayer doesn't have a text-font defined, we automatically add Open Sans/Arial Unicode MS. However, when the SymbolLayer is only used for rendering icons, it doesn't have text-field defined either. In those cases, we still force downloading Open Sans/Arial Unicode MS during offline pack creation. If the user doesn't use this font, this change should save ~15MB and a few seconds in download time.

@kkaefer kkaefer added offline Core The cross-platform C++ core, aka mbgl labels Jan 25, 2018
@kkaefer kkaefer added the needs backport Indicates PR needs to be cherrypicked into a previous release branch. label Jan 25, 2018
When a SymbolLayer doesn't have a text-font defined, we automatically add Open Sans/Arial Unicode MS. However, when the SymbolLayer is only used for rendering icons, it doesn't have text-field defined either. In those cases, we still force downloading Open Sans/Arial Unicode MS during offline pack creation. If the user doesn't use this font, this change should save ~15MB and a few seconds in download time.
Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

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

This could break apps that have styles without font-stack and font-text property but use the runtime APIs to add symbol layers.

Would be good to add a note to the [Offline Maps] (https://www.mapbox.com/help/mobile-offline/) documentation.

@kkaefer
Copy link
Member Author

kkaefer commented Jan 26, 2018

This could break apps that have styles without font-stack and font-text property but use the runtime APIs to add symbol layers.

This is correct, however, using runtime-styling with offline downloads is a slipperly slope anyway; e.g. you can also use any other kind of font that isn't available offline. However, this PR does change the behavior to the default font not being available offline by default.

@ChrisLoer
Copy link
Contributor

This could break apps that have styles without font-stack and font-text property but use the runtime APIs to add symbol layers.

This ended up being part of the confusion in: #12167 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl needs backport Indicates PR needs to be cherrypicked into a previous release branch. offline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants