Skip to content

[codex] Port remaining Android XR compatibility fixes#182

Open
matthargett wants to merge 1 commit into
BabylonJS:mainfrom
rebeckerspecialties:codex/android-xr-compat-remaining
Open

[codex] Port remaining Android XR compatibility fixes#182
matthargett wants to merge 1 commit into
BabylonJS:mainfrom
rebeckerspecialties:codex/android-xr-compat-remaining

Conversation

@matthargett

@matthargett matthargett commented Jun 4, 2026

Copy link
Copy Markdown

Summary

This PR now contains only the Android/runtime-host work that is still missing from main after rebasing onto 8cd1429:

  • build and run the Android unit-test app against API level 35;
  • align the app's local default with the NDK r28c version already used by CI, while retaining the -PndkVersion override;
  • accept comma-separated Gradle ABI filters and choose sensible arm64/x86_64 defaults for local and injected builds;
  • send native debug-trace output to a dedicated JsRuntimeHost logcat tag; and
  • replace the inspector's char16_t/uint16_t type-punning buffer with a correctly typed std::vector<uint16_t>.

The Android setup instructions now name the SDK and NDK versions required by this test configuration.

Work already superseded or removed

  • #118 (70ba1e5) already made the project build with NDK r28c and moved Android CI to it. #159 (49e2980) migrated CI to GitHub Actions, and #164 (7239f91) restored the connected-emulator test. This PR therefore only advances that emulator from API 33 to API 35 and aligns the local Gradle default.
  • #133 (06d4420) aligned the supported Android and Windows prebuilts on V8 11. The old three-argument/four-argument inspector connect split is no longer applicable and has been removed from this PR.
  • The proposed android-jsc 250231.0.0 -> 294992.0.0 bump has been removed. 294992 is published under a different Maven path than this repository consumes, and its arm64 binary still has 4 KiB ELF load alignment, so it would not solve Android 15/Android XR 16 KiB page compatibility. #206 handles the replacement separately by consuming Bun's pinned Android WebKit/JSC archives for arm64-v8a and x86_64, relinking them as libjsc.so, and raising only the JSC path to API 28.
  • The rebase conflict with #176 (f07d991) was resolved while preserving its Hermes and host-compiler CMake arguments.

Validation

  • npm ci in Tests.
  • Fresh API 35 / NDK 28.2.13676358 arm64 debug builds with both V8 and JavaScriptCore.
  • Connected Android tests on an API 35 arm64 emulator with both engines: 1/1 test passed for V8 and 1/1 for JavaScriptCore.
  • ELF program-header audit: the newly built JNI library, libc++_shared.so, and the supported V8 prebuilt all use 16 KiB load alignment. The legacy JSC prebuilt validated by this branch remains 4 KiB aligned; its intentionally separate replacement in #206 produces 16 KiB-aligned libjsc.so binaries for arm64-v8a and x86_64.
  • git diff --check.

Scope

This is Android API/toolchain and runtime-host preparation; it is not by itself complete BabylonNative Android XR support. OpenXR loader/manifest wiring, lifecycle and input integration, arm64 Android CI/ELF auditing, and validation on Galaxy XR/XREAL hardware remain separate follow-up work (principally BabylonNative#1766).

@matthargett matthargett force-pushed the codex/android-xr-compat-remaining branch from 91cf1ad to 625af30 Compare June 4, 2026 19:24
@matthargett matthargett marked this pull request as ready for review June 4, 2026 22:44
Copilot AI review requested due to automatic review settings June 4, 2026 22:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the Android unit test harness and related dependencies to newer Android tooling (API 35 / newer NDK), improves Android-native logging, and adds compatibility for different V8 Inspector connect() signatures.

Changes:

  • Bumped Android compile/target SDK to 35, updated NDK version handling, and improved ABI filter selection logic.
  • Switched native test trace output to Logcat and updated Android emulator CI to API level 35.
  • Updated JSC Android dependency and adjusted V8 Inspector agent for V8 version API differences.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Tests/UnitTests/Android/app/src/main/cpp/JNI.cpp Routes debug trace output to Android Logcat instead of stdout.
Tests/UnitTests/Android/app/build.gradle Updates SDK/NDK configuration and refines ABI filter behavior.
README.md Updates Android setup instructions to match new SDK/NDK/API level expectations.
Core/Node-API/package-jsc.json Bumps jsc-android dependency version.
Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp Adds V8 version-based connect() handling and adjusts string buffer creation.
.github/workflows/build-android.yml Moves CI emulator image to API level 35.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp Outdated
Comment thread Tests/UnitTests/Android/app/src/main/cpp/JNI.cpp
Comment thread Tests/UnitTests/Android/app/build.gradle
@matthargett matthargett requested a review from Copilot June 5, 2026 06:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp
Comment thread Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp Outdated
Comment thread Tests/UnitTests/Android/app/build.gradle
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.

2 participants