Use camera API in AndroidPicturesService#444
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Android PicturesService implementation by replacing the heavyweight external camera Intent flow with an in-app CameraX overlay, reducing app background/foreground churn and enabling richer camera interactions.
Changes:
- Replaces
MediaStore.ACTION_IMAGE_CAPTUREwith an in-app CameraX-based overlay (preview, capture, flip camera, pinch-to-zoom, back/cancel handling). - Refactors post-capture image scaling/rotation/compression into a standalone
ImagePreprocessorand adds a native “cancelled” callback path. - Updates the dalvik AAR build inputs to include CameraX dependencies and new resources, and removes the now-unneeded FileProvider setup.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/pictures/src/main/resources/META-INF/substrate/dalvik/res/xml/file_provider_paths.xml | Removes FileProvider paths config (no longer using FileProvider-based capture). |
| modules/pictures/src/main/resources/META-INF/substrate/dalvik/res/drawable/ic_flip_camera.xml | Adds flip-camera vector icon for the overlay UI. |
| modules/pictures/src/main/resources/META-INF/substrate/dalvik/build.gradle | Adds an Android library build file with CameraX dependencies for the dalvik AAR build. |
| modules/pictures/src/main/resources/META-INF/substrate/dalvik/AndroidManifest.xml | Removes FileProvider declaration; keeps/adjusts permissions and queries for the dalvik AAR. |
| modules/pictures/src/main/resources/META-INF/substrate/dalvik/android-dependencies.txt | Declares CameraX/Guava dependencies to be carried alongside the produced AAR. |
| modules/pictures/src/main/native/android/dalvik/PreviewStreamCoverController.java | Adds logic to fade out a startup cover when the preview stream starts. |
| modules/pictures/src/main/native/android/dalvik/ImagePreprocessor.java | Centralizes image downscaling/rotation and JPEG rewrite behavior. |
| modules/pictures/src/main/native/android/dalvik/DalvikPicturesService.java | Switches capture flow to CameraController and routes results/cancel back to native layer. |
| modules/pictures/src/main/native/android/dalvik/CameraRotationController.java | Adds rotation/orientation tracking and debounce for stable camera target rotation updates. |
| modules/pictures/src/main/native/android/dalvik/CameraOverlayView.java | Builds the overlay UI (preview + controls) and handles inset-aware layout. |
| modules/pictures/src/main/native/android/dalvik/CameraController.java | Orchestrates CameraX lifecycle/binding, capture, zoom gesture handling, and overlay teardown. |
| modules/pictures/src/main/native/android/dalvik/CameraBackHandler.java | Implements back handling for the overlay (system back + key listener). |
| modules/pictures/src/main/native/android/c/pictures.c | Adds native sendCancelled JNI entry point; fixes local-ref cleanup. |
| modules/pictures/src/main/java/com/gluonhq/attach/pictures/PicturesService.java | Updates Android permission documentation to reflect the new approach. |
| modules/pictures/src/main/java/com/gluonhq/attach/pictures/impl/AndroidPicturesService.java | Handles cancellation results (null paths) and updates docs. |
| gradle/native-build.gradle | Adjusts dalvik AAR packaging to copy the entire res/ directory (for new drawable resources). |
Comments suppressed due to low confidence (2)
modules/pictures/src/main/native/android/dalvik/DalvikPicturesService.java:106
DalvikPicturesService.verifyPermissions()requestsREAD_EXTERNAL_STORAGE/WRITE_EXTERNAL_STORAGEfor Android < 13, but this module AndroidManifest.xml currently doesn't declare those permissions. On Android 12 and below, runtime permission requests for undeclared permissions will fail, which will effectively disable both camera and gallery features. Either declare the legacy permissions in the manifest (typically withandroid:maxSdkVersion="32") or adjust the permission checks to match the manifest/scoped-storage approach.
private boolean verifyPermissions() {
if (!verified) {
if (Build.VERSION.SDK_INT >= 33) {
verified = Util.verifyPermissions(Manifest.permission.CAMERA,
Manifest.permission.READ_MEDIA_AUDIO,
Manifest.permission.READ_MEDIA_IMAGES,
Manifest.permission.READ_MEDIA_VIDEO);
} else {
verified = Util.verifyPermissions(Manifest.permission.CAMERA,
Manifest.permission.READ_EXTERNAL_STORAGE,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
}
modules/pictures/src/main/native/android/dalvik/DalvikPicturesService.java:102
- For Android 13+, the code requests
READ_MEDIA_AUDIOandREAD_MEDIA_VIDEOeven though the gallery intent is restricted toimage/*. This is a broader permission request than needed (principle of least privilege) and may cause unnecessary permission prompts / policy friction. Consider requesting onlyREAD_MEDIA_IMAGESfor gallery access.
if (Build.VERSION.SDK_INT >= 33) {
verified = Util.verifyPermissions(Manifest.permission.CAMERA,
Manifest.permission.READ_MEDIA_AUDIO,
Manifest.permission.READ_MEDIA_IMAGES,
Manifest.permission.READ_MEDIA_VIDEO);
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+132
to
+137
| private File createCaptureFile(boolean savePhoto) { | ||
| String timeStamp = new SimpleDateFormat("yyyyMMdd_HHmmss").format(new Date()); | ||
| // Create the file where the photo should go | ||
| try { | ||
| File photo = savePhoto ? new File(Environment.getExternalStoragePublicDirectory( | ||
| Environment.DIRECTORY_PICTURES), "IMG_"+ timeStamp + ".jpg") : | ||
| File.createTempFile("IMG_"+ timeStamp, ".jpg", activity.getCacheDir()); | ||
| File photo = savePhoto | ||
| ? new File(Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_PICTURES), "IMG_" + timeStamp + ".jpg") | ||
| : File.createTempFile("IMG_" + timeStamp, ".jpg", activity.getCacheDir()); |
Comment on lines
4
to
7
| <uses-permission android:name="android.permission.CAMERA" /> | ||
| <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" /> | ||
| <uses-permission android:name="android.permission.READ_MEDIA_AUDIO" /> | ||
| <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" /> |
Comment on lines
+84
to
+85
| * <li><code>android.permission.READ_MEDIA_AUDIO</code> — for reading audio files from the gallery (Android 13+)</li> | ||
| * <li><code>android.permission.READ_MEDIA_VIDEO</code> — for reading video files from the gallery (Android 13+)</li> |
Comment on lines
+49
to
+55
| * <p>Required permissions are automatically declared in AndroidManifest.xml:</p> | ||
| * <ul> | ||
| * <li><code>android.permission.CAMERA</code></li> | ||
| * <li><code>android.permission.READ_MEDIA_IMAGES</code> (Android 13+)</li> | ||
| * <li><code>android.permission.READ_MEDIA_AUDIO</code> (Android 13+)</li> | ||
| * <li><code>android.permission.READ_MEDIA_VIDEO</code> (Android 13+)</li> | ||
| * <li><code>android.permission.READ_EXTERNAL_STORAGE</code> (Android 12 and below)</li> |
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.
Fixes #443