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

[android] Agua cherry-pick of TinySDF functionality #10706

Merged
merged 5 commits into from
Dec 18, 2017

Conversation

ChrisLoer
Copy link
Contributor

This PR cherry picks core/android TinySDF functionality from master. It also contains the initial ios/macos implementation because that commit made significant changes to core, but does not include all the follow-up commits that actually enable the functionality on ios/macos. It includes the core test for TinySDF, but does not include the unit tests that actually do local glyph generation (they only ran on macOS).

I tested by building the test app and running the "Local CJK glyphs" test activity in a Pixel emulator.

In general I tried to keep the commits as minimally modified as possible, hoping that will reduce pain during the eventual merge back to master. If it helps, I can re-write the ios/macos commit to strip out the darwin-specific parts of the code.

/cc @zugaldia @chriswu42 @lilykaiser @Guardiola31337 @ivovandongen

@zugaldia
Copy link
Member

cc: @mapbox/maps-android

 - Platform-specific LocalGlyphRasterizer is responsible for deciding which glyphs to rasterize locally and for implementing the rasterization.
 - Default platform implementation doesn't locally generate any glyphs -> no behavior change
 - Unit test uses StubLocalGlyphRasterizer, which returns a single fixed bitmap for all CJK glyphs
 - Rename glyph_loader.test to glyph_manager.test
 - Draws bold version of glyph if font stack contains string "bold"
 - Not hooked up to global configuration yet
@ChrisLoer ChrisLoer force-pushed the android-agua-tinysdf branch from 6c0bd52 to 218dcf2 Compare December 15, 2017 18:16
@lilykaiser lilykaiser added this to the android-v5.2.2 milestone Dec 15, 2017
@ChrisLoer
Copy link
Contributor Author

I ended up stripping the PR back a little further:

  • I removed the core unit test that exercises TinySDF (glyph_manager.test.cpp) to avoid digging into merge conflicts coming from modifying/renaming that test from the previous glyph_loader.test.cpp. The core unit test is relevant to Android, but I'm not too worried about not having it run because the test just touches the TinySDF algorithm itself, which is really unlikely to be modified on the release branch. For testing the Android functionality, the important part is till the "Local GJK Glyphs" test activity.
  • Since I was stripping changes out of the commit anyway, I went ahead and stripped out the unused darwin implementation.

@kkaefer kkaefer added Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Dec 18, 2017
Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

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

👍

@ChrisLoer ChrisLoer merged commit f2b42af into release-agua Dec 18, 2017
@tobrun tobrun deleted the android-agua-tinysdf branch December 19, 2017 07:48
@tobrun tobrun mentioned this pull request Dec 20, 2017
22 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants