Repository navigation
android: Load the bundled emoji font on the host-driven path - #29
Conversation
Android 13+ ships a COLRv1 emoji font that swash cannot draw, so the platform loads a bundled CBDT NotoColorEmoji from the APK's assets instead. It read the AssetManager only through AndroidApp, which the host-driven entry point does not have, so emoji rendered as empty boxes there. The first set_host_activity now also takes the application's AssetManager and keeps it, with a global reference to the Java object, for the life of the process. The crate-internal jni::with_asset_manager() lends it out on either path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f205afc to
77126f2
Compare
huacnlee
left a comment
There was a problem hiding this comment.
Thanks for this, the with_asset_manager approach looks right. A few things to address before merging:
1. The emoji font depends on call order and loads only once (src/android/jni.rs, set_host_activity / platform.rs)
HOST_ASSETS is set only inside set_host_activity, but the emoji font is loaded exactly once, in AndroidPlatform::new. If the host calls host::start before set_host_activity (which the host.rs module docs don't forbid), or if getApplicationContext()/getAssets() fails on the first onCreate, emoji stay as empty boxes for the rest of the process, even after a later Activity fills HOST_ASSETS. The only signal is a debug! line. Please do at least one of:
- update the "What the host has to do" section in
host.rsto say thatset_host_activitymust come beforestart; - log at
warn!level, notdebug!, when no AssetManager is available on the host path; - or load the font lazily, or retry it once
HOST_ASSETSbecomes available.
2. JNI local references leak in host_assets
context and assets are local refs that are never deleted. That is fine when the call returns straight to Java, but set_host_activity is a public API and may be called from a native-attached thread or inside a long-lived JNI frame. Wrapping the body in env.with_local_frame(...), or deleting the refs explicitly, would make it safe.
3. Pending exceptions are cleared inconsistently in set_host_activity
The new asset branch clears a pending exception, but the earlier ? returns (new_global_ref, get_java_vm) still return Err with the exception pending. Any further JNI call the host makes before returning to Java is then UB (and CheckJNI aborts). Please clear exceptions on those paths too, so error handling is consistent.
Suggestions (not blocking):
query_night_mode_via_jnistill reads the configuration only throughandroid_app(), so on the host path it always returnsfalse(light appearance). Now thatwith_asset_managerexists, it would be a one-line fix:with_asset_manager(|am| Configuration::from_asset_manager(am)).load_asset_bytescopies the mapped ~10 MB NotoColorEmoji into a newVecwithto_vec(). This predates the PR, but the host path now runs it too. Streaming the asset straight into a singleVecwould avoid doubling peak memory at startup.
Review follow-up for the host-path emoji font: - If host::start runs before set_host_activity, the platform finds no AssetManager. It now leaves its text system pending and logs a warning, and set_host_activity loads the font once it has the AssetManager. The host.rs docs say set_host_activity comes first. - host_assets runs in a JNI local frame, so the context and AssetManager local references are freed even when the caller stays in native code. - set_host_activity clears a pending exception on every error path. - load_asset_bytes reads the asset into one Vec of its size instead of copying the mapped buffer, so the ~10 MB font is not held twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Emoji font: review follow-up from longbridge#29
huacnlee
left a comment
There was a problem hiding this comment.
Thanks, 773e5ade addresses everything I asked for: the deferred emoji load (the locking is sound in both orders), the local frame in host_assets, the exception clearing in set_host_activity, and the single-buffer asset read. LGTM.
Minor follow-ups, none blocking:
- When
startcomes first,load_deferred_emoji_fontreads and parses the ~10 MB font on the UI thread duringonCreate. That's fine for a fallback path, but worth knowing about. - It's worth a quick device test of the
start-before-set_host_activityorder, to confirm text that was laid out before the font arrived picks up the emoji once it does. host.rssaysset_host_activity"must come first" and then says the reverse also works; "should" would be more accurate.query_night_mode_via_jnicould usewith_asset_managertoo, so the host path gets dark mode (can be a separate PR).
On the host-driven entry point (
android::host), emoji render as empty boxes. This PR lets that path read the bundled emoji font.Problem
Android 13+ ships a COLRv1 emoji font that swash cannot draw, so
AndroidPlatformloads the bundled CBDTfonts/NotoColorEmoji.ttffrom the APK's assets instead. It only reaches the AssetManager throughAndroidApp, which the host-driven path does not have, so the font was never loaded there.Change
set_host_activityalso takes the application'sAssetManager(getApplicationContext().getAssets()). It keeps a global reference to the Java object, so the nativeAAssetManagerstays valid for the life of the process.jni::with_asset_manager(|am| …)lends it out on either path.load_asset_bytestakes an&ndk::asset::AssetManagerinstead of anAndroidApp.set_host_activitystill succeeds.set_host_activity's docs now say to call it beforehost::start, since the font is loaded when the platform starts.The
android-activitypath is unchanged.Screenshots
Left: without this change. Right: with it. (OnePlus CPH2581, Android 16, host-driven test app.)
Testing
cargo fmt --all -- --check,cargo clippy --target aarch64-linux-android --lib: no new warnings.fonts/NotoColorEmoji.ttf: the log showssystem emoji font is COLR v1 (no CBDT) — will try bundled fallback, thenloaded bundled CBDT emoji font from assets (10589456 bytes), and emoji render in color.Not covered here: flags (🇮🇹) and
❤️(U+2764 + VS16) still don't render on this device, with or without this change. Flags come from the system'sNotoColorEmojiFlags.ttf, which probably has the same COLRv1 problem. I'd look at that separately.Test app: Bombatomica64/Gpui-android.
🤖 Generated with Claude Code