Repository navigation
Add front camera screen torch for video capture - #641
soufiane77-eng wants to merge 1 commit into
Conversation
Illuminate the subject during front camera video capture by maximizing the screen brightness and drawing a white frame around the viewfinder. Auto is treated like On, matching the existing front photo screen flash behavior. The brightness is restored once both the photo screen flash and the video screen torch are inactive, including when a photo is taken while recording. Tests cover the derived UI state and the torch frame rendering.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a video screen torch feature for front-facing cameras during video recording, which maximizes screen brightness and displays a white frame around the viewfinder. Feedback on the changes includes a style guide violation regarding the naming convention of the VIDEO_SCREEN_TORCH_OVERLAY test tag, an opportunity to simplify the VideoScreenTorchFrame drawing logic by using Modifier.border instead of a custom Canvas, and suggestions to remove unnecessary runTest coroutine builders from tests that do not perform asynchronous operations.
|
|
||
| const val PREVIEW_DISPLAY = "PreviewDisplay" | ||
| const val SCREEN_FLASH_OVERLAY = "ScreenFlashOverlay" | ||
| const val VIDEO_SCREEN_TORCH_OVERLAY = "VideoScreenTorchOverlay" |
There was a problem hiding this comment.
According to the repository style guide (section 10, Test Tags and Semantics), the string value of a test tag must be a lower_snake_case string following the schema element_purpose_value. Please update the string value of VIDEO_SCREEN_TORCH_OVERLAY to follow this convention (e.g., "overlay_video_screen_torch").
| const val VIDEO_SCREEN_TORCH_OVERLAY = "VideoScreenTorchOverlay" | |
| const val VIDEO_SCREEN_TORCH_OVERLAY = "overlay_video_screen_torch" |
References
- Test Tag Naming Convention: When creating a new test tag, the constant name must be in UPPER_SNAKE_CASE, and the string value must be a lower_snake_case string following a specific schema: element_purpose_value. (link)
| fun VideoScreenTorchFrame(modifier: Modifier = Modifier) { | ||
| Canvas( | ||
| modifier = modifier | ||
| .testTag(VIDEO_SCREEN_TORCH_OVERLAY) | ||
| .fillMaxSize() | ||
| ) { | ||
| val borderWidth = VIDEO_SCREEN_TORCH_BORDER_WIDTH.toPx() | ||
| .coerceAtMost(size.minDimension / 2f) | ||
| // Draw four bars instead of a stroked rectangle so that the frame is fully opaque and the | ||
| // center of the screen stays transparent. | ||
| drawRect( | ||
| color = Color.White, | ||
| size = Size(width = size.width, height = borderWidth) | ||
| ) | ||
| drawRect( | ||
| color = Color.White, | ||
| topLeft = Offset(x = 0f, y = size.height - borderWidth), | ||
| size = Size(width = size.width, height = borderWidth) | ||
| ) | ||
| drawRect( | ||
| color = Color.White, | ||
| size = Size(width = borderWidth, height = size.height) | ||
| ) | ||
| drawRect( | ||
| color = Color.White, | ||
| topLeft = Offset(x = size.width - borderWidth, y = 0f), | ||
| size = Size(width = borderWidth, height = size.height) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of manually drawing four separate rectangles using a custom Canvas to create a border, you can use a standard Box with Modifier.border. This is more idiomatic in Jetpack Compose, simplifies the code, and avoids manual coordinate calculations.\n\nNote that Modifier.border draws the border inside the boundaries of the composable, and keeps the center transparent, which matches your requirements perfectly. Make sure to import androidx.compose.foundation.border.
@Composable\nfun VideoScreenTorchFrame(modifier: Modifier = Modifier) {\n Box(\n modifier = modifier\n .testTag(VIDEO_SCREEN_TORCH_OVERLAY)\n .fillMaxSize()\n .border(width = VIDEO_SCREEN_TORCH_BORDER_WIDTH, color = Color.White)\n )\n}| @Test | ||
| fun videoScreenTorchFrame_sizeFillsMaxSize() = runTest { | ||
| composeTestRule.awaitIdle() | ||
| val rootBounds = composeTestRule.onRoot().getBoundsInRoot() | ||
| composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY)) | ||
| .assertWidthIsAtLeast(rootBounds.width) | ||
| composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY)) | ||
| .assertHeightIsAtLeast(rootBounds.height) | ||
| } |
There was a problem hiding this comment.
The test videoScreenTorchFrame_sizeFillsMaxSize does not perform any asynchronous operations or coroutine-based assertions. Using runTest here is unnecessary and can be simplified to a standard test function.
@Test\n fun videoScreenTorchFrame_sizeFillsMaxSize() {\n composeTestRule.awaitIdle()\n val rootBounds = composeTestRule.onRoot().getBoundsInRoot()\n composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY))\n .assertWidthIsAtLeast(rootBounds.width)\n composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY))\n .assertHeightIsAtLeast(rootBounds.height)\n }| @Test | ||
| fun videoScreenTorchFrame_borderIsWhiteAndCenterIsNotWhite() = runTest { | ||
| composeTestRule.awaitIdle() | ||
| val frameImage = | ||
| composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY)).captureToImage() | ||
|
|
||
| val borderPixel = IntArray(1) | ||
| frameImage.readPixels( | ||
| borderPixel, | ||
| frameImage.width / 2, | ||
| 1, | ||
| 1, | ||
| 1 | ||
| ) | ||
| assertThat(borderPixel[0]).isEqualTo(Color.White.toArgb()) | ||
|
|
||
| val centerPixel = IntArray(1) | ||
| frameImage.readPixels( | ||
| centerPixel, | ||
| frameImage.width / 2, | ||
| frameImage.height / 2, | ||
| 1, | ||
| 1 | ||
| ) | ||
| assertThat(centerPixel[0]).isNotEqualTo(Color.White.toArgb()) | ||
| } |
There was a problem hiding this comment.
The test videoScreenTorchFrame_borderIsWhiteAndCenterIsNotWhite does not use any coroutines or suspend functions. You can remove runTest and define it as a standard test function.
@Test\n fun videoScreenTorchFrame_borderIsWhiteAndCenterIsNotWhite() {\n composeTestRule.awaitIdle()\n val frameImage =\n composeTestRule.onNode(hasTestTag(VIDEO_SCREEN_TORCH_OVERLAY)).captureToImage()\n\n val borderPixel = IntArray(1)\n frameImage.readPixels(\n borderPixel,\n frameImage.width / 2,\n 1,\n 1,\n 1\n )\n assertThat(borderPixel[0]).isEqualTo(Color.White.toArgb())\n\n val centerPixel = IntArray(1)\n frameImage.readPixels(\n centerPixel,\n frameImage.width / 2,\n frameImage.height / 2,\n 1,\n 1\n )\n assertThat(centerPixel[0]).isNotEqualTo(Color.White.toArgb())\n }
Summary
Adds a screen torch for the front-facing camera while recording video, addressing #299.
Most front-facing cameras have no flash unit, so the screen itself now illuminates the subject during video capture when the flash mode is On or Auto:
The photo screen flash is unchanged.
Implementation
isVideoScreenTorchActiveinCaptureUiStateAdapterfrom the existing recording state, lens facing, and flash mode. No camera/event pipeline changes.VideoScreenTorchFrameinside the viewfinder slot so it stays behind the controls.ScreenFlashScreenbrightness management for both the photo flash and the video torch, restoring the original brightness once both are inactive. This also covers taking a photo while the video torch is active.Tests
CaptureUiStateAdapterTest: front + On/Auto + active/paused recording enables the torch; Off, back camera, and inactive recording disable it.VideoScreenTorchFrameTest: the frame fills the viewfinder, has a white border, and leaves the center transparent.Validation note
The local environment has no JDK or Android SDK, so Gradle build, unit tests, lint, and instrumentation tests could not be executed locally (
NOT RUN). The changes were reviewed statically (git diff --check), and CI will run the full validation.