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

Fallback support for CJK glyph generation #14915

Closed

Description

@tobrun

Local glyph generation should include support for fallback fonts. A developer should be able to pass in an array of fonts should they choose to set the font(s) for local glyph generation.

Activity

  1. added this to the release-p milestone on Jun 13, 2019
  2. chloekraw commented on Jun 17, 2019

    @chloekraw
    Contributor

    In #10522 we added support in core for using locally-generated glyphs. Per @tobrun, the core implementation accepts a single string and needs to be modified to accept an array of fonts in order to support fallbacks. cc/ @tmpsantos

  3. added
    CoreThe cross-platform C++ core, aka mbgl
    iOSMapbox Maps SDK for iOS
    on Jun 17, 2019
  4. tobrun commented on Jun 24, 2019

    @tobrun
    MemberAuthor

    I noticed that iOS is handling this differently, they check the font array on the binding side and pass only 1 value to core. I believe that handling this on core side would align it more with the other APIs we have that use fonts (eg text-font)

  5. chloekraw commented on Jun 25, 2019

    @chloekraw
    Contributor

    @tobrun, I believe the way iOS handles fallback fonts for localIdeographFontFamily matches the implementation they use for passing in fonts for all text, including text-font.

    Theoretically speaking, is a comparable approach possible on Android, if we wanted to refactor the way that Android passes fonts to core? I'm not sure whether it'd be desirable or whether we've previously had a conversation about the pros/cons of both approaches. cc @1ec5

  6. chloekraw commented on Jul 10, 2019

    @chloekraw
    Contributor

    @tmpsantos, how big is the change needed in core to support this? it would be extremely valuable to get this into release-picklejuice so that Android gets to parity with iOS

  7. tmpsantos commented on Jul 19, 2019

    @tmpsantos
    Contributor

    @tmpsantos, how big is the change needed in core to support this?

    Not big, most of the work here is writing render/unit tests.

  8. tobrun commented on Jul 30, 2019

    @tobrun
    MemberAuthor

    Capturing from chat between @tmpsantos, @alexshalamov and me. We are going to add same logic as iOS. Note however that on Android this kind of a hack as we on one end don't have a clear Android API to retrieve system fonts. We can't try/catch creating fonts from names as these factory methods always return non-null even if the font is unusable.

  9. chloekraw commented on Jul 30, 2019

    @chloekraw
    Contributor

    @tobrun thank you for the update! Could you explain why this decision was made? Is there a technical blocker with implementing this in core?

    I want to make sure we're fully considering the implications of this decision and alternatives before proceeding.

    cc @zugaldia

  10. tobrun commented on Jul 31, 2019

    @tobrun
    MemberAuthor

    I initially thought it would be a bit more easy for going with the core heavy implementation but that would result in going back and forth between core and the SDK. Having #15255 is much simpler and matches iOS setup.

  11. removed
    CoreThe cross-platform C++ core, aka mbgl
    iOSMapbox Maps SDK for iOS
    on Jul 31, 2019
  12. chloekraw commented on Jul 31, 2019

    @chloekraw
    Contributor

    @tobrun ok, sounds good!

  13. chloekraw commented on Aug 12, 2019

    @chloekraw
    Contributor

    Closed by #15255

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

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions