-
Notifications
You must be signed in to change notification settings - Fork 348
[CameraX β’ Compose] Migrate CameraXBasic Samples to Compose #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @madebymozart, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request migrates the CameraX samples to Jetpack Compose. It includes changes to the app's build file to include the CameraX library, adds a new API surface for CameraX, and updates the sample demos to include CameraX samples. The pull request also adds new files for the CameraX samples, including basic image capture, video capture, extensions, and MLKit integration. The UI layer is re-architected in Jetpack Compose, simplifying the underlying logic where appropriate. The primary goal of this PR was the conversion to Jetpack Compose, and any pre-existing bugs or limitations present in the original View-based samples may have been carried over during this migration.
Highlights
- Compose Migration: Migrates CameraX samples to Jetpack Compose for a cleaner and more maintainable architecture.
- New CameraX Samples: Adds four new CameraX samples: Basic Image Capture, Basic Video Capture, Extensions, and MLKit Sample.
- Dependency Updates: Includes necessary dependency updates in
gradle/libs.versions.toml
andapp/build.gradle.kts
to support CameraX and MLKit features.
Changelog
Click here to see the changelog
- app/build.gradle.kts
- Added implementation dependency for the CameraX sample module.
- app/src/main/java/com/example/platform/app/ApiSurface.kt
- Added a new
ApiSurface
entry for CameraX to register it as a platform API. - Added CameraX to the list of API surfaces.
- Added a new
- app/src/main/java/com/example/platform/app/SampleDemo.kt
- Imported CameraX sample composables.
- Registered CameraX samples (Basic Image Capture, Video Capture, Extensions, MLKit) as composable demos.
- gradle/libs.versions.toml
- Added CameraX dependencies (core, camera2, lifecycle, video, view, mlkit-vision, extensions).
- Added barcode scanning dependencies (barcode-scanning-common, play-services-mlkit-barcode-scanning).
- samples/camera/camerax/README.md
- Added a placeholder README file.
- samples/camera/camerax/build.gradle.kts
- Created a new build file for the CameraX sample module, including dependencies for CameraX, Compose, Coil, and MLKit.
- samples/camera/camerax/src/main/AndroidManifest.xml
- Created a new AndroidManifest file for the CameraX sample module, including camera permission and a file provider.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXBasic.kt
- Created a composable for basic CameraX image capture functionality.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXExtensions.kt
- Created a composable for CameraX extensions functionality.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
- Created a composable for CameraX MLKit integration, specifically for QR code scanning.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXVideo.kt
- Created a composable for basic CameraX video capture functionality.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/mlkit/QrCodeDrawable.kt
- Created a custom Drawable for displaying QR code information on the camera preview.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/mlkit/QrCodeViewModel.kt
- Created a ViewModel for handling QR code data and touch events.
- samples/camera/camerax/src/main/java/com/example/platform/camerax/viewmodels/CameraExtensionsViewModel.kt
- Created a ViewModel for CameraX extensions, managing camera provider, extensions manager, and UI state.
- settings.gradle.kts
- Included the new CameraX sample module in the project.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with π and π on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Compose now sings a camera's tune,
Where views once reigned, a brighter moon.
Jetpack's embrace,
A modern grace,
In samples new, a welcome boon.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. β©
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request migrates CameraX samples to Jetpack Compose, which is a great step towards modernizing the codebase. The changes seem well-structured, and the new Compose UIs should improve maintainability. However, there are a few areas that could be improved, particularly regarding documentation and code clarity.
Summary of Findings
- Missing Descriptions: Several CameraX sample descriptions are marked as "TBD". Providing meaningful descriptions would greatly enhance the discoverability and understanding of these samples.
- Duplicated Dependency: The
build.gradle.kts
file for the CameraX sample contains a duplicated dependency forandroidx.camerax.mlkit.vision
. This should be resolved to avoid potential conflicts.
Merge Readiness
The pull request is a good step forward, but the missing descriptions and duplicated dependency should be addressed before merging. I am unable to directly approve the pull request, and that users should have others review and approve this code before merging.
samples/camera/camerax/src/main/java/com/example/platform/camerax/mlkit/QrCodeViewModel.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXBasic.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXBasic.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXBasic.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXMlKitSample.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/CameraXVideo.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/basic/CameraXBasic.kt
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/basic/CameraXBasic.kt
Outdated
Show resolved
Hide resolved
samples/camera/camerax/src/main/java/com/example/platform/camerax/basic/CameraXBasic.kt
Show resolved
Hide resolved
β¦es using CameraXViewFinder
7bde909
to
62d7324
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for post-merge comments but noticed a few issues.
} | ||
} | ||
|
||
Box(modifier = Modifier.fillMaxSize()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build on the provided modifier
param i.e. Box(modifier = modifier.fillMaxSize()) {
onShowCapturedImageChange = { showCapturedImage = it }, | ||
viewModel = viewModel, | ||
imageCaptureCallbackExecutor = imageCaptureCallbackExecutor, | ||
modifier = modifier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forward the modifier param onward.
var showCapturedImage by remember { mutableStateOf<Uri?>(null) } | ||
val cameraPermissionState = rememberPermissionState(Manifest.permission.CAMERA) | ||
val imageCaptureCallbackExecutor: ExecutorService = remember { Executors.newSingleThreadExecutor() } | ||
val viewModel = remember { CameraXBasicViewModel() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use the viewModels
function to construct? https://developer.android.com/reference/kotlin/androidx/lifecycle/viewmodel/compose/package-summary#viewModel(androidx.lifecycle.ViewModelStoreOwner,kotlin.String,kotlin.Function1)
when (cameraPermissionState.status) { | ||
is PermissionStatus.Granted -> { | ||
if (showCapturedImage != null) { | ||
CapturedImageView(uri = showCapturedImage) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this should accept the modifier param if it takes this branch.
} | ||
} | ||
|
||
is PermissionStatus.Denied -> CameraPermissionDeniedView( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: again the modifier should be passed here if taking this branch.
|
||
surfaceRequest?.let { request -> | ||
val coordinateTransformer = remember { MutableCoordinateTransformer() } | ||
Box(modifier = Modifier.fillMaxSize()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should build on the passed modifier.
} | ||
|
||
// State for managing the autofocus indicator | ||
var autofocusRequest by remember { mutableStateOf(randomUUID() to Offset.Unspecified) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the pair needed here? It seems like this could just be
var autofocusLocation = remember { mutableStateOf { Offset.Unspecified } {
?
Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed to remember the state of the autofocus point to adjust when the tap-to-focus actually happened
We do the same here: https://gist.github.com/JolandaVerhoef/74d4696b804736c698450bd34b5c9ff8#file-2_1_tap_to_focus-kt-L130
.size(48.dp), | ||
) | ||
} | ||
Column( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Column
isn't needed. The Button
could use Modifier.wrapContentSize(<alignment>)
to position itself.
* (e.g., clicks the back button). | ||
*/ | ||
@Composable | ||
fun CapturedImageView(uri: Uri, onDismiss: () -> Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should accept a modifier.
π Jetpack Compose Migration for Camera Samples (CameraXBasic)
π Description
This pull request continues our initiative to modernize the
camera-samples
repository by converting existing View-based examples to Jetpack Compose.In this iteration, one key sample has been successfully migrated from their original implementations in the android/camera-samples repository. The migration involved:
β How Has This Been Tested?
Manual testing has been performed on the following configuration to ensure the core functionality of the migrated samples remains intact:
Test Configuration #1
Next Steps: