Repository navigation
Emoji font: review follow-up from longbridge/gpui-mobile#29 - #11
Conversation
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>
…b-emoji-review # Conflicts: # src/android/jni.rs
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a252a73999
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "loaded bundled CBDT emoji font from assets ({} bytes)", | ||
| bytes.len() | ||
| ); | ||
| if let Err(e) = text_system.add_fonts(vec![std::borrow::Cow::Owned(bytes)]) { |
There was a problem hiding this comment.
When host::start wins the race far enough for a surface to render text before set_host_activity runs, this adds the font directly to the platform CosmicTextSystem. That bypasses GPUI's TextSystem::add_fonts, which invalidates cached font resolution and line layouts, and this UI-thread path also requests no redraw. Consequently, already rendered emoji can remain cached as missing glyphs even though the log reports that the deferred font loaded; route the late installation through the app text system on the render thread and invalidate/redraw the windows.
Useful? React with 👍 / 👎.
1 participant
Merges `android-host-emoji-font` (773e5ad, the review follow-up for longbridge#29) into main. Conflict in the `set_host_activity` doc comment resolved by keeping main's text plus the deferred-load sentence; main's `or_clear` calls are kept.
🤖 Generated with Claude Code