Skip to content

android: Use the system clipboard, and only tear down the surface a destroy names - #24

Merged
madcodelife merged 2 commits into
longbridge:mainfrom
Bombatomica64:fix/android
Oct 9, 2026
Merged

madcodelife merged 2 commits into
longbridge:mainfrom
Bombatomica64:fix/android

Conversation

@Bombatomica64

@Bombatomica64 Bombatomica64 commented Sep 29, 2026 •

Copy link
Copy Markdown

Thank you for gpui-mobile. The host-driven entry point made it possible to build a full on-device test gallery for GPUI Kit 0.7.0. While running it, I found two small Android problems and am proposing fixes for them here (the wider report is longbridge/gpui-kit#3310). Each is a separate commit, and I'm happy to split them, rework them, or drop either one if you'd prefer a different approach.

1. Back the platform clipboard with ClipboardManager

Problem

AndroidClipboard stored copied text in process memory only. Text copied in a GPUI app (for example Kit's input or TextView "Copy") could not be pasted into other apps, and paste inside the app never saw what other apps had copied. The clipboard package (packages::clipboard, backed by GpuiClipboard.java and ClipboardManager) already existed but the platform clipboard did not use it.

Change

With the clipboard feature (on by default), AndroidClipboard::read / write go through packages::clipboard::{get_text, set_text}. The in-process copy is kept and used as a fallback, with a log::warn!, when the JNI call fails (for example if a host does not package GpuiClipboard.java). Without the feature, behaviour is unchanged.

Testing

  • cargo fmt --all, cargo check, cargo ndk -t arm64-v8a check with and without --features clipboard, and cargo test --lib (46 passed).
  • cargo test doc tests: platform_view and platform_view_element fail identically on main, so that failure is not from this change.
  • aarch64-apple-ios was not checked (Linux machine); the change is Android-only code.
  • On device: used by an Android test app for GPUI Kit 0.7.0 (host entry point, Android 13 emulator via Redroid). Kit's Copy shows Android 13's clipboard overlay, and the copied text reads back on paste.

2. Only tear down the surface a destroy names

Problem

On the host-driven entry point (android::host), recreating the Activity in the same process (e.g. FLAG_ACTIVITY_CLEAR_TASK) could leave a black screen with a working app underneath. The new Activity's surfaceCreated can arrive before the old Activity's surfaceDestroyed. SurfaceDestroyed carried no identity, so the render thread terminated the window that was already attached to the new surface, and nothing re-attached it.

The wait in surface_destroyed() had a related problem: it waited on one shared SURFACE_RELEASED flag that a later surfaceCreated could reset, so overlapping destroy/create pairs could time out or return early.

Change

  • Command::SurfaceDestroyed(Option<usize>) carries the ANativeWindow being destroyed. The render thread calls term_window only if it matches CURRENT_SURFACE; otherwise it logs and ignores the stale destroy.
  • New pub fn surface_destroyed_for(window: &NativeWindow) for hosts that know which surface went away. surface_destroyed() keeps its signature and old meaning (it tears down whatever is attached), and its docs now point to the new function. The module docs recommend surface_destroyed_for.
  • The blocking wait uses request/handled counters (DESTROYS_REQUESTED / DESTROYS_HANDLED), so each caller waits for its own request. The 2 s bound is unchanged.

The android-activity path is untouched.

Testing

  • cargo fmt --all, cargo check, cargo ndk -t arm64-v8a check --all-features, and cargo test --lib (46 passed).
  • cargo test doc tests: platform_view and platform_view_element fail identically on main, so that failure is not from this change.
  • aarch64-apple-ios was not checked (Linux machine); the change is Android-only code.
  • On device: an Android test app on the host path (Android 13 emulator via Redroid) calls surface_destroyed_for from surfaceDestroyed. Before the change, an in-process recreation turned the screen black. After it, 5 consecutive in-process recreations all had their late destroys ignored, and rendering, touch and GPUI state stayed intact.

Playground used for the on-device testing: Bombatomica64/Gpui-android (prebuilt APK), in case you want to try these changes on Android.

Thanks for taking the time to review this.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings September 29, 2026 07:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Bombatomica64 and others added 2 commits October 9, 2026 14:05
AndroidClipboard kept copied text in process memory only, so copy/paste
did not reach other apps and paste never saw the system clipboard. With
the clipboard feature enabled, read/write now go through
packages::clipboard (ClipboardManager via JNI), keeping the in-memory
copy as a fallback when the JNI call fails.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`surface_destroyed` waited on a single `SURFACE_RELEASED` flag that the
render thread set on `SurfaceDestroyed` and cleared on `SurfaceCreated`.
The flag can still read "released" from an earlier destroy while a
`SurfaceCreated` posted before the new destroy sits unhandled in the
queue, so the wait returned before the render thread had processed
either command, and the `Surface` could go away just as the render
thread attached to it and drew.

Number each destroy request instead and wait until the render thread has
handled that many. Commands are handled in order, so everything posted
before the destroy has been handled too. `on_surface_created` no longer
touches the state.

Co-authored-by: lorenzomicheli <lorenzo.micheli@instal.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@madcodelife

Copy link
Copy Markdown
Member

Thanks for the PR!

I rebased it onto main and pushed here. The clipboard commit is unchanged. The surface one I had to rework, since #26 landed in the meantime and android::host now tracks each Activity by HostId. A new Activity instance gets its own id, so a late surfaceDestroyed from the old one only tears down its own window. That already covers the black screen you hit, so I dropped surface_destroyed_for and the pointer check.

Your counters were still worth keeping, though. With the single SURFACE_RELEASED flag, surface_destroyed could see a stale "released" from an earlier destroy while a SurfaceCreated was still sitting in the queue, and return too early, so I kept them, adapted to the new API.

@madcodelife
madcodelife merged commit f9fe5a7 into longbridge:main Oct 9, 2026
5 checks passed
Bombatomica64 added a commit to Bombatomica64/Gpui-android that referenced this pull request Oct 9, 2026
…18)

* Build against gpui-mobile main after longbridge/gpui-mobile#24, #26 and #27

Upstream reworked #24 on top of #26, which gives each host Activity its own
window keyed by a HostId. LabActivity keeps its HostId in its saved state, so
a recreated instance gets its window back and a fresh launch opens a new one;
onDestroy closes the window when the Activity finishes. The catalog window
now opens from host::on_open_window, and back sends IME event 5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* CI: build the release APK on GitHub Actions

Builds arm64-v8a + x86_64 with ./build.sh on pushes to main, PRs and v* tags,
uploads the APK as an artifact, and attaches it to the release for a v* tag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants