Resurrect HelloAndroid and HelloAndroidCamera2 - #9286
Open
alexreinking wants to merge 3 commits into
Open
Conversation
apps/HelloAndroid had no working build: build.gradle pinned Gradle 2.2 /
AGP 1.2.2 and resolved from jcenter(), which shut down in 2021;
jni/Application.mk's `APP_STL := gnustl_static` is a hard error on
current NDKs; and the src/res/AndroidManifest.xml-at-the-root layout
predates Android Studio.
Replace it with the standard app/src/main/{java,res,cpp} module layout,
current Gradle/AGP, and a single app/src/main/cpp/CMakeLists.txt that
Gradle's externalNativeBuild drives directly. Following the "Use
add_halide_generator" pattern in doc/HalideCMakePackage.md, the
CMakeLists.txt configures itself twice: once as a host build (to compile
the Halide generator, via execute_process() before find_package() needs
it) and once as this Android target build. No manual per-ABI
TARGETS/loop is needed: add_halide_library without TARGETS resolves the
Halide "cmake" meta-target from the ambient CMAKE_SYSTEM_NAME/
CMAKE_SYSTEM_PROCESSOR the NDK toolchain sets per ABI. Targets arm64-v8a
and x86_64.
Also fixes two runtime bugs: CameraActivity never requested the CAMERA
permission at runtime (added the standard ContextCompat/ActivityCompat
check-then-request flow), and hello_wrapper.cpp built its
halide_buffer_t structs without setting `.dimensions`, so every pipeline
call failed a dimension-count check and the app eventually crashed.
Fixes #8727
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Same build-system rot as HelloAndroid: an unbuildable Gradle 2.2/AGP
1.2.2 setup resolving from the dead jcenter(), and a src/res layout
predating Android Studio. Replace it with the standard
app/src/main/{java,res,cpp} module layout and a single
app/src/main/cpp/CMakeLists.txt driven by Gradle's externalNativeBuild,
adapted for this app's two generators (deinterleave, edge_detect) and
four JNI wrapper sources. Targets arm64-v8a and x86_64.
Also fixes three runtime issues: CameraActivity never requested the
CAMERA permission at runtime (added the standard
ContextCompat/ActivityCompat check-then-request flow);
Camera2BasicFragment.java's `switch (view.getId()) { case R.id.toggle:
}` doesn't compile under AGP's non-final resource IDs default, rewritten
as an if; and YuvBufferT's "packed planar" chroma case built a Buffer
for just the U (or V) plane and then called crop(1, 0, height * 2) to
try to extend it to cover both planes -- Buffer::crop() only ever
narrows within a buffer's existing declared bounds, so this crashed
(SIGABRT) the moment the camera produced a frame in this layout. Fixed
by constructing packedPlanarChromaView_ directly from the pointer with
the full doubled-height shape, the same way the sibling "interleaved"
branch already does.
Fixes #8728
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Add an android-apps job (testing-linux.yml) that builds both apps via `./gradlew assembleDebug` and then smoke-tests them (apps/support/android_smoke_test.sh) against a real booted emulator, rather than only checking that they build: (1) fresh install, launch, confirm the OS shows a runtime permission prompt -- catches an app relying on its manifest declaration alone; (2) grant the permission, relaunch, and confirm the process survives with none of a set of known-bad logcat signatures (fatal crash signals, plus each app's own previously-fixed bug's exact error string). Deliberately does not require seeing a "pipeline completed" log line: whether the camera preview surface negotiates a size the pipeline actually runs on is sensitive to the emulator's screen/camera configuration, which varies by environment. Uses reactivecircus/android-emulator-runner with an x86_64 (not arm64-v8a) system image so the emulator gets KVM hardware acceleration on these x86_64 GitHub-hosted runners, and an emulated back/front camera so no physical camera is needed. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alexreinking/wasm-app-build #9286 +/- ##
===============================================================
+ Coverage 70.27% 70.35% +0.07%
===============================================================
Files 258 258
Lines 79160 79160
Branches 18997 18997
===============================================================
+ Hits 55631 55693 +62
+ Misses 17806 17793 -13
+ Partials 5723 5674 -49 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
apps/HelloAndroid had no working build: build.gradle pinned Gradle 2.2 / AGP 1.2.2 and resolved from
jcenter(), which shut down in 2021; jni/Application.mk'sAPP_STL := gnustl_staticis a hard error on current NDKs; and the src/res/AndroidManifest.xml-at-the-root layout predates Android Studio.Replace it with the standard
app/src/main/{java,res,cpp}module layout, current Gradle/AGP, and a singleapp/src/main/cpp/CMakeLists.txtthat Gradle'sexternalNativeBuilddrives directly. The CMakeLists.txt configures itself twice: once as a host build (to compile the Halide generator, viaexecute_process()beforefind_package()needs it) and once as this Android target build. This is admittedly a hack to work around Gradle's lack of a host-targetexternalNativeBuildvariant. Targets arm64-v8a and x86_64, dropping 32-bit Android targets that are obsolete.Also fixes two runtime bugs: CameraActivity never requested the CAMERA permission at runtime (added the standard ContextCompat/ActivityCompat check-then-request flow), and hello_wrapper.cpp built its
halide_buffer_tstructs without setting.dimensions, so every pipeline call failed a dimension-count check and the app eventually crashed.HelloAndroidCamera2 got the same modernization and also fixed three runtime issues: CameraActivity never requested the CAMERA permission at runtime (added the standard ContextCompat/ActivityCompat check-then-request flow); Camera2BasicFragment.java's
switch (view.getId()) { case R.id.toggle: }doesn't compile under AGP's non-final resource IDs default, rewritten as an if; and YuvBufferT's "packed planar" chroma case built a Buffer for just the U (or V) plane and then calledcrop(1, 0, height * 2)to try to extend it to cover both planes --Buffer::crop()only ever narrows within a buffer's existing declared bounds, so this crashed (SIGABRT) the moment the camera produced a frame in this layout. Fixed by constructingpackedPlanarChromaView_directly from the pointer with the full doubled-height shape, the same way the sibling "interleaved" branch already does.Testing
Add an android-apps job (testing-linux.yml) that builds both apps via
./gradlew assembleDebugand then smoke-tests them (apps/support/android_smoke_test.sh) against a real booted emulator, rather than only checking that they build: (1) fresh install, launch, confirm the OS shows a runtime permission prompt -- catches an app relying on its manifest declaration alone; (2) grant the permission, relaunch, and confirm the process survives with none of a set of known-bad logcat signatures (fatal crash signals, plus each app's own previously-fixed bug's exact error string). Deliberately does not require seeing a "pipeline completed" log line: whether the camera preview surface negotiates a size the pipeline actually runs on is sensitive to the emulator's screen/camera configuration, which varies byenvironment.
Uses reactivecircus/android-emulator-runner with an x86_64 (not arm64-v8a) system image so the emulator gets KVM hardware acceleration on these x86_64 GitHub-hosted runners, and an emulated back/front camera so no physical camera is needed.
Fixes #8727
Fixes #8728
Breaking changes
None.
Checklist
Stack created with GitHub Stacks CLI • Give Feedback 💬