Incremental steps to support Android XR #115
Conversation
This change updates the Azure Pipelines CI for Android to use NDK version 28.2.13676358 and SDK API level 35. The previous configuration used an outdated NDK and SDK, causing the build to fail with an "NDK is not installed" error. This commit addresses the issue by: - Updating the `ndkVersion` variable in `azure-pipelines.yml`. - Modernizing the Android job in `jobs/android.yml` to: - Use the `macos-latest` VM image. - Install the correct NDK and SDK versions. - Create an AVD with the new system image. - Use JDK 17 for the Gradle build.
Fix: Update Android CI to use NDK 28 and API 35
This change updates the Azure Pipelines CI for Android to use NDK version 28.2.13676358 and SDK API level 35. The previous configuration used an outdated NDK and SDK, causing the build to fail. This commit addresses the issue by: - Updating the `ndkVersion` variable in `azure-pipelines.yml`. - Modernizing the Android job in `jobs/android.yml` to: - Use the `macos-latest` VM image. - Install the correct NDK and SDK versions. - Create an AVD with the new system image. - Use JDK 17 for the Gradle build. - Add `_JAVA_OPTIONS` to fix `NoClassDefFoundError` with `sdkmanager` on newer JDKs.
Updated required tools and minimum Android version in README. Added link to Android OS measured usages globally. Note specifically Android-base XR device coverage that Android 10 and up includes. This *does* exclude Oculus Go (which was left on Android 7 before EOL), which we can discuss further if coverage of that device is deemed critical.
…it explicitly now. Update README to be specific about minimums and why.
…ent crashes on shutdown
… size cost to downstream users, as this JSC includes full intl build. It might still be worth it based on the JIT and GC performance improvements that shoudl give noticeable uplift to downstream users.
… which should also allow some of the explicit VM flags to be eliminated
…lding everything else back. It should be discussed why Chakra support is still needed in the OSS repo.
…g out in CI. this worked okay on my local macbook before, so I'm assuming swap was the problem. this makes it faster on my local macbook, hopefully fixes it in CI.
…simulator. TypedArray test has some endianness specifics, skipping it temporarily. use the globalThis polyfill more consistently.
|
test failing on Android is |
… apps that are testing deployments of different JS engines can include which runtime is being used in their JS-based telemetry (which is preferable over native telemetry because it can be updated OTA and while native app container is backgrounded), but it's proven to be too volatile. I could delete it, but preferring to leave it skipped here as a placeholder.
We will have to check with our partners, but Env().Global().Set("globalThis", Env().Global());Is there an issue with this? |
In BabylonNative, we only bring in the binaries for testing the apps folder. Do you mean you are going to add code that depends on new features of JSC or V8? |
I added new tests and made some gross assumptions about 2019-era JS features being available, and had to iterate a lot to make the Server 2019 version of Chakra (which is a different version/update than consumer/desktop Windows 10) work. I appreciate its only for local tests and maybe doesn't affect downstream app builders, but it's was a friction in this contribution. I did polyfill it in JS, in the meantime (which is what took a lot of iteration). |
If the JS code I can idiomatically write in my own BabylonJS app can be aligned with the JS code I contribute in the testing apps, it would lower the cognitive load for myself (and hopefully) other users to contribute. |
I guess I'm a bit confused or I'm missing something. All of our pipelines that I can find run on the |
We are in the process of converting all the app JavaScript code to use TypeScript. We will likely need use babel to translate to something that Chakra can understand. Then you should be able to contribute code without thinking about the downlevel support in Chakra. |
I assume this was the case, because Chakra has had globalThis feature flag enabled since the ~2020 Windows 10 release build I mentioned. Since you mentioned that the TypeScript setup will transpile down to whatever the Chakra runtime used will support, that solves the problem also. |
|
Integration tested in BabylonNative PR BabylonJS/BabylonNative#1552 tested macOS native build and both Android variants in a Pixel 2 simulator running Android 10, unit tests pass and test app works as expected in both places. |
I'm running Chakra locally on my Windows 11 box with Babylon Native and I haven't looked into what babel does with |
chakra-core/ChakraCore@3517001 It's totally possible that my colleagues at BlueJeans (RIP) enabled it in the local build (we had to ship on Windows 7 and built our own profile-optimized DLL), and it was never enabled in Windows.
Regardless, I've workaround it in the new unit tests and me and my team are technical enough to get into micro details on bundling our app so it's optimized for whichever modern VM we deploy. I'm cool if you're cool :) |
ChakraCore is not Chakra. ChakraCore is the open-source version of Chakra. Chakra ships with the OS. Sometimes changes to ChakraCore will be ported back to Chakra but I'm guessing it's not likely nowadays. Babylon Native only supports Chakra right now. We could in theory also support ChakraCore but that's not currently available. |
Sorry, I was confused about which had which feature enabled. I'm not asking to change, and if your internal customers are okay with it, eliminating Chakra would help simplify things a bit. It's not a big deal, so if there's any complications that make keeping it around a business necessity, that's okay. |
|
@CedricGuillemet any more comments on these changes or requests for additional testing before approving/merging? |
We can't remove support for Chakra. This is used by a number of partners. Once we have babel in the apps build process, there should be minimal difference. |
| static constexpr int compare(const char_type* s1, const char_type* s2, size_t n) noexcept { | ||
| for (size_t i = 0; i < n; ++i) { | ||
| if (s1[i] < s2[i]) return -1; | ||
| if (s1[i] > s2[i]) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| static constexpr const char_type* find(const char_type* s, size_t n, const char_type& c) noexcept { | ||
| for (size_t i = 0; i < n; ++i) { | ||
| if (s[i] == c) return s + i; | ||
| } | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
I only see length being used. Where are these two functions used?
There was a problem hiding this comment.
I thought I needed the statics to complete the template specialization, but maybe not?
There was a problem hiding this comment.
This code is adding a template specialization, but it is not a specialization of std::char_traits. This is just a custom struct with some static functions. You can probably just not use templates at all since it's just a simple function call to get the length.
| // Temporarily disable StdoutLogger due to fdsan issue with NDK 28 | ||
| // android::StdoutLogger::Start(); |
There was a problem hiding this comment.
Temporary in what way? Will this come back?
There was a problem hiding this comment.
This will require a fix in AndroidExtensions repo to avoid the fd sanitizer issues (which were likely causing intermittent instability during app shutdown). Happy to do it.
| }); | ||
| }); | ||
|
|
||
| describe("N-API Compatibility", function () { |
There was a problem hiding this comment.
How are these tests related to N-API? I don't see any code here that is related to N-API. Can you explain?
| }); | ||
| }); | ||
|
|
||
| describe("Unicode and String Encoding", function () { |
There was a problem hiding this comment.
Why are we testing JavaScript features?
There was a problem hiding this comment.
this is meant to cover some aspects of integration that other JS runtime proxies have had issues with in the past. after the N-API crash bug in this repo's code that only occurred when JavaScriptCore was used, I wanted to add some small regression tests to get more coverage when running the local tests under the sanitizers. I think these will be useful when testing integrations against newer version of v8, JSC, etc as well. Still, if you think they're orthogonal, I can remove them.
There was a problem hiding this comment.
Maybe I need more context on the sanitizers. What sanitizers are we talking about?
Also, I think you mean something different when you say N-API (or Node-API). Node-API is the contract between C++ and JavaScript, as well as the implementation of Node-API against a JavaScript engine.
If we want to test Node-API, it should be using something in the Node-API contract (i.e., something needs to cross the native to js or js to native bridge). There is actually a good set of tests for Node-API in Hermes when Node-API was introduced there. It would be great to reuse those tests here, but that should be a completely separate PR and is somewhat of a big lift.
Unless I'm missing something, these tests are strictly testing different JavaScript engines which is not necessarily bad in of itself, but they have nothing to do with Node-API. There are already conformance tests for ECMAScript that do this kind of test, such as https://compat-table.github.io/compat-table/es6.
There was a problem hiding this comment.
clang (and gcc) include sanitizers, which use compile-time code injection to do runtime checking. There's Address Sanitizer (asan), which looks for out-of-bounds memory accesses; Thread Sanitizer (tsan) which looks for bugs relating to threading and mutex misuse; and File Descriptor Sanitizer (fdsan), which looks for (you guessed it) file descript misuse (use after close, etc). There are more sanitizers, but these are the ones I have the most personal experience with. I was a big user of Purify back in the day, and a very early user of valgrind and similar tools.
the N-API reference I'm making is to the work I started in this other PR #116 . I already re-used hermes-windows tests for that PR, and @vmoroz is in the loop. We've been working together in a small group to bring Node Addons ABI into React Native for the past ~9 months. bringing his clean work over wasn't too big of a lift :) , but that's a discussion for PR #116 .
when I found the mutex use-after-free in the AndroidExtensions code while developing this PR, and then the issues in the other PR that sanitizers reported (that are logical once they're highlighted), I thought it prudent to test some of the string serialization and other challenges from JSC integration in bun, and Hermes integration into React Native PlayStation. This could be done further upstream in BabylonNative, or left as an exercise to the app developer (eg me and @hmallen99), just let me know which you'd prefer.
There was a problem hiding this comment.
clang (and gcc) include sanitizers, which use compile-time code injection to do runtime checking. There's Address Sanitizer (asan), which looks for out-of-bounds memory accesses; Thread Sanitizer (tsan) which looks for bugs relating to threading and mutex misuse; and File Descriptor Sanitizer (fdsan), which looks for (you guessed it) file descript misuse (use after close, etc). There are more sanitizers, but these are the ones I have the most personal experience with. I was a big user of Purify back in the day, and a very early user of valgrind and similar tools.
Ok, that's what I thought you meant, but just wanted to make sure. I still don't understand how these sanitizers have to do with testing JavaScript itself though.
the N-API reference I'm making is to the work I started in this other PR #116.
Great! I missed this PR. This will be an awesome addition to this repo. Thanks.
when I found the mutex use-after-free in the AndroidExtensions code while developing this PR
Can you explain this? Is this the PR you opened or something else?
the issues in the other PR that sanitizers reported (that are logical once they're highlighted)
Again, I don't have context for this. Can you elaborate?
I thought it prudent to test some of the string serialization and other challenges from JSC integration in bun, and Hermes integration into React Native PlayStation.
I'm not familiar with either of these and/or how they relate to this repo. Can you elaborate?
This could be done further upstream in BabylonNative, or left as an exercise to the app developer (eg me and @hmallen99), just let me know which you'd prefer.
I'm just trying to understand the motivation.
Tests/UnitTests/Shared/Shared.cpp
Outdated
| #ifdef __ANDROID__ | ||
| // Global flag to track if StdoutLogger has been initialized | ||
| static bool s_stdoutLoggerInitialized = false; | ||
|
|
||
| void EnsureStdoutLoggerStarted() | ||
| { | ||
| if (!s_stdoutLoggerInitialized) | ||
| { | ||
| android::StdoutLogger::Start(); | ||
| s_stdoutLoggerInitialized = true; | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
I don't understand this. It looks like this logic was moved from JNI.cpp to here. Why? This is a shared file. It is not intended to have platform specific logic.
There was a problem hiding this comment.
sorry -- yes, you're correct. I was trying to figure out how to avoid the fdsan crash without adding code into AndroidExtensions repo, which doesn't seem to have its own tests, CI, or other feedback loops. I'll bite the bullet and prepare a PR for AndroidExtensions to fix the root problem where it acutally lives.
There was a problem hiding this comment.
draft PR is up in AndroidExtensions. iniital integration into this PR branch looks good. BabylonJS/AndroidExtensions#15
Co-authored-by: Gary Hsu <[email protected]>
Co-authored-by: Gary Hsu <[email protected]>
| static constexpr int compare(const char_type* s1, const char_type* s2, size_t n) noexcept { | ||
| for (size_t i = 0; i < n; ++i) { | ||
| if (s1[i] < s2[i]) return -1; | ||
| if (s1[i] > s2[i]) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| static constexpr const char_type* find(const char_type* s, size_t n, const char_type& c) noexcept { | ||
| for (size_t i = 0; i < n; ++i) { | ||
| if (s[i] == c) return s + i; | ||
| } | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
This code is adding a template specialization, but it is not a specialization of std::char_traits. This is just a custom struct with some static functions. You can probably just not use templates at all since it's just a simple function call to get the length.
.gitignore
Outdated
| @@ -1 +1,2 @@ | |||
| /Build | |||
| /Tests/UnitTests/dist/ | |||
There was a problem hiding this comment.
I don't know if this is from your PR, but we typically avoid generating files in the source tree. Generated files should go in the CMake binary folder which we typically put in Build.
| }); | ||
| }); | ||
|
|
||
| describe("Unicode and String Encoding", function () { |
There was a problem hiding this comment.
Maybe I need more context on the sanitizers. What sanitizers are we talking about?
Also, I think you mean something different when you say N-API (or Node-API). Node-API is the contract between C++ and JavaScript, as well as the implementation of Node-API against a JavaScript engine.
If we want to test Node-API, it should be using something in the Node-API contract (i.e., something needs to cross the native to js or js to native bridge). There is actually a good set of tests for Node-API in Hermes when Node-API was introduced there. It would be great to reuse those tests here, but that should be a completely separate PR and is somewhat of a big lift.
Unless I'm missing something, these tests are strictly testing different JavaScript engines which is not necessarily bad in of itself, but they have nothing to do with Node-API. There are already conformance tests for ECMAScript that do this kind of test, such as https://compat-table.github.io/compat-table/es6.
…ator. For now, we only have fdsan, ubsan, and tsan, which is still tons better coverage and insight than we had previously.
…gs, but I'll save it for a separate PR.
… like the rest of the repo does. Transfer globalThis test into C++ and don't run on Windows (Chakra). delete the JS version of the compat tests. Update webpack to be more resilient to retargeted build output directory, which I was doing to run AddressSanitizer (asan) builds in parallel with the regular sanitizers, since ASan is mutually exclusive with the others.
… due to an unclean shutdown of the AppRuntime's Work Queue. I'm trying not to fix all the sanitizer issues in this PR, but want the test to be a golden example of micro-embedding that doesn't have immediate bugs.
…ecause it stores wrapped objects in an intermediary void*, the vtpr sanitizer reasonably complains. this macro allows us to sidestep the issue only for those specific problematic calls.
There was a problem hiding this comment.
Sorry @matthargett for all the comments. We are getting closer though.
| #if defined(__clang__) || defined(__GNUC__) | ||
| #define NAPI_NO_SANITIZE_VPTR __attribute__((no_sanitize("vptr"))) | ||
| #else | ||
| #define NAPI_NO_SANITIZE_VPTR | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Where did these changes come from? We try to ensure these headers match the original unless it is documented as a change. Search for // [BABYLON-NATIVE-ADDITION]
There was a problem hiding this comment.
In the commit message, I noted that this should be upstreamed. When they cast the T down to a void* l, it loses RTTI and that trips a sanitizer. I'll add the local comment and submit a PR upstream as well.
| namespace { | ||
| // Template specialization to provide char_traits functionality for JSChar (unsigned short) | ||
| // at compile time, mimicking std::char_traits interface | ||
| // Minimal char_traits-like helper for JSChar (unsigned short) to compute string length at compile time |
There was a problem hiding this comment.
This should just be a simple function. There is no reason for the struct or template.
| set(_jsruntimehost_runtime_source "") | ||
| set(_jsruntimehost_runtime_target "") | ||
|
|
||
| if(ANDROID) | ||
| set(_jsruntimehost_sanitizers "") | ||
| if(DEFINED ENV{JSRUNTIMEHOST_NATIVE_SANITIZERS}) | ||
| set(_jsruntimehost_sanitizers "$ENV{JSRUNTIMEHOST_NATIVE_SANITIZERS}") | ||
| endif() | ||
| if(DEFINED ENV{JSRUNTIMEHOST_ENABLE_ASAN}) | ||
| if(_jsruntimehost_sanitizers STREQUAL "") | ||
| set(_jsruntimehost_sanitizers "address") | ||
| else() | ||
| set(_jsruntimehost_sanitizers "${_jsruntimehost_sanitizers},address") | ||
| endif() | ||
| endif() | ||
| if(NOT _jsruntimehost_sanitizers STREQUAL "") | ||
| string(REGEX REPLACE "[ \t\r\n]" "" _jsruntimehost_sanitizers "${_jsruntimehost_sanitizers}") | ||
| string(REGEX REPLACE ",+" "," _jsruntimehost_sanitizers "${_jsruntimehost_sanitizers}") | ||
| string(REGEX REPLACE "^,|,$" "" _jsruntimehost_sanitizers "${_jsruntimehost_sanitizers}") | ||
| if(NOT _jsruntimehost_sanitizers STREQUAL "") | ||
| message(STATUS "Enabling sanitizers: ${_jsruntimehost_sanitizers}") | ||
| add_compile_options("-fsanitize=${_jsruntimehost_sanitizers}" "-fno-omit-frame-pointer") | ||
| add_link_options("-fsanitize=${_jsruntimehost_sanitizers}") | ||
| set(_jsruntimehost_sanitizers_list "${_jsruntimehost_sanitizers}") | ||
| string(REPLACE "," ";" _jsruntimehost_sanitizers_list "${_jsruntimehost_sanitizers_list}") | ||
| list(FIND _jsruntimehost_sanitizers_list "undefined" _jsruntimehost_has_ubsan) | ||
| if(_jsruntimehost_has_ubsan GREATER -1) | ||
| if(ANDROID_ABI STREQUAL "arm64-v8a") | ||
| set(_jsruntimehost_san_arch "aarch64") | ||
| elseif(ANDROID_ABI STREQUAL "armeabi-v7a") | ||
| set(_jsruntimehost_san_arch "arm") | ||
| elseif(ANDROID_ABI STREQUAL "x86") | ||
| set(_jsruntimehost_san_arch "i686") | ||
| elseif(ANDROID_ABI STREQUAL "x86_64") | ||
| set(_jsruntimehost_san_arch "x86_64") | ||
| else() | ||
| set(_jsruntimehost_san_arch "") | ||
| endif() | ||
| if(_jsruntimehost_san_arch) | ||
| get_filename_component(_jsruntimehost_toolchain_dir "${CMAKE_C_COMPILER}" DIRECTORY) | ||
| get_filename_component(_jsruntimehost_toolchain_root "${_jsruntimehost_toolchain_dir}" DIRECTORY) | ||
| file(GLOB _jsruntimehost_ubsan_runtime | ||
| "${_jsruntimehost_toolchain_root}/lib/clang/*/lib/linux/libclang_rt.ubsan_standalone-${_jsruntimehost_san_arch}-android.so") | ||
| list(LENGTH _jsruntimehost_ubsan_runtime _jsruntimehost_ubsan_runtime_len) | ||
| if(_jsruntimehost_ubsan_runtime_len GREATER 0) | ||
| list(GET _jsruntimehost_ubsan_runtime 0 _jsruntimehost_ubsan_runtime_path) | ||
| set(_jsruntimehost_runtime_source "${_jsruntimehost_ubsan_runtime_path}") | ||
| set(_jsruntimehost_runtime_target "libclang_rt.ubsan_standalone-${_jsruntimehost_san_arch}-android.so") | ||
| else() | ||
| message(WARNING "UBSan runtime not found for ABI ${ANDROID_ABI}") | ||
| endif() | ||
| endif() | ||
| endif() | ||
| endif() | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
I don't know what I'm looking at. Why is it so complex?
There was a problem hiding this comment.
Also, is the intent of this PR to add the sanitizer options?
| ndk { | ||
| def abiFiltersProp = project.findProperty("abiFilters")?.toString() | ||
| if (abiFiltersProp) { | ||
| def propFilters = abiFiltersProp.split(',').collect { it.trim() }.findAll { !it.isEmpty() } | ||
| if (!propFilters.isEmpty()) { | ||
| abiFilters(*propFilters) | ||
| } | ||
| } else { | ||
| // Prefer injected ABI hints and fall back to a host-aware default | ||
| def requestedAbi = project.findProperty("android.injected.build.abi") ?: System.getenv("ANDROID_ABI") | ||
| def defaultAbis = [] | ||
| if (requestedAbi) { | ||
| defaultAbis = requestedAbi.split(',').collect { it.trim() }.findAll { !it.isEmpty() } | ||
| } | ||
| if (defaultAbis.isEmpty()) { | ||
| def hostArch = (System.getProperty("os.arch") ?: "").toLowerCase() | ||
| if (hostArch.contains("aarch64") || hostArch.contains("arm64")) { | ||
| defaultAbis = ['arm64-v8a'] | ||
| } else { | ||
| defaultAbis = ['arm64-v8a', 'x86_64'] | ||
| } | ||
| } | ||
| abiFilters(*defaultAbis) |
There was a problem hiding this comment.
What is the purpose of this change?
| }); | ||
|
|
||
| function runTests() { | ||
| // Import the engine compatibility tests after Mocha is set up |
There was a problem hiding this comment.
Should this comment still be here?
| set(JSRUNTIMEHOST_OUTPUT_DIR "${CMAKE_BINARY_DIR}/Tests/UnitTests/dist") | ||
| set(JSRUNTIMEHOST_OUTPUT_DIR "${JSRUNTIMEHOST_OUTPUT_DIR}" CACHE INTERNAL "Output directory for bundled unit test scripts") |
There was a problem hiding this comment.
Why do we need to set both the cache and local variable?
| set(JSRUNTIMEHOST_OUTPUT_DIR "${CMAKE_BINARY_DIR}/Tests/UnitTests/dist") | ||
| set(JSRUNTIMEHOST_OUTPUT_DIR "${JSRUNTIMEHOST_OUTPUT_DIR}" CACHE INTERNAL "Output directory for bundled unit test scripts") | ||
| file(MAKE_DIRECTORY "${JSRUNTIMEHOST_OUTPUT_DIR}") | ||
| file(REMOVE_RECURSE "${CMAKE_CURRENT_SOURCE_DIR}/UnitTests/dist") |
There was a problem hiding this comment.
Is this a good idea? Seems dangerous to remove a folder in the source folder when running CMake.
| file(MAKE_DIRECTORY "${JSRUNTIMEHOST_OUTPUT_DIR}") | ||
| file(REMOVE_RECURSE "${CMAKE_CURRENT_SOURCE_DIR}/UnitTests/dist") | ||
|
|
||
| set(ENV{JSRUNTIMEHOST_BUNDLE_OUTPUT} "${JSRUNTIMEHOST_OUTPUT_DIR}") |
There was a problem hiding this comment.
Sorry, looking at this more. I didn't realize what was happening here. Looks like dist\tests.js is committed to source control, so my original comment about generated files being in the CMake binary folder does not make sense. Can you please revert the changes for this? Also, don't add the dist folder to .gitignore like you had originally. It actually needs to be included in the PR if the ts file changes.
| GIT_REPOSITORY https://github.com/matthargett/AndroidExtensions.git | ||
| GIT_TAG fix-stale-JNI-ref) |
There was a problem hiding this comment.
Just noting that this needs to be updated once AndroidExtensions PR is merged
| if(APPLE) | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fobjc-arc") | ||
| if(NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "Debug") | ||
| if(NOT DEFINED JSRUNTIMEHOST_NATIVE_SANITIZERS) | ||
| set(JSRUNTIMEHOST_NATIVE_SANITIZERS "address,undefined") | ||
| endif() | ||
| if(JSRUNTIMEHOST_NATIVE_SANITIZERS) | ||
| message(STATUS "macOS sanitizers enabled: ${JSRUNTIMEHOST_NATIVE_SANITIZERS}") | ||
| add_compile_options("-fsanitize=${JSRUNTIMEHOST_NATIVE_SANITIZERS}" "-fno-omit-frame-pointer") | ||
| add_link_options("-fsanitize=${JSRUNTIMEHOST_NATIVE_SANITIZERS}") | ||
| endif() | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
Are sanitizers supposed to be part of this PR?
There was a problem hiding this comment.
To some degree, yes, because NDK 28 turns some of them on by default and the crash that occurred is what got me started on that sidequest. We could override the NDK 28 default to turn off default sanitizers, and put the fixes into a separate PR. Up to you :)
3 participants
Bump the NDK just below unstable r29 and SDK API level 36 that appear to be necessary for building for Android XR devices. Note that targeting API Level 35 is mandatory for publishing new apps to Google Play since August 2025, so this is a bit overdue for those reasons as well.
The C++20 incompatibility highlighted by newer clang in NDK 28 was highlighted by a similar downstream PR in BabylonNative: BabylonJS/BabylonNative#1552
I tried to also use the Android system-installed v8 library, rather than building from a separate source, so that profile-optimized system version that received security updates keeps apps performant. The performance aspect is key for 3D/XR workloads, and the security aspect is key when BabylonJS is processing user-supplied data by opening and rendering splats, behavior graphs, WGSL shaders, etc. It was not straightforward, and so I'm leaving that as a separate step in the future PR.