Fix wrong removal timing of screen flash compose and remove redundant…

… clean ups
google · Dec 29, 2023 · dcf7be8 · dcf7be8
1 parent f041d6c
commit dcf7be8
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 38 deletions.
diff --git a/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraUseCase.kt b/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraUseCase.kt
@@ -59,8 +59,6 @@ interface CameraUseCase {
 
     fun isScreenFlashEnabled(): Boolean
 
-    fun clearScreenFlash()
-
     suspend fun setAspectRatio(aspectRatio: SettingsAspectRatio, isFrontFacing: Boolean)
 
     suspend fun flipCamera(isFrontFacing: Boolean, flashMode: SettingsFlashMode)

diff --git a/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt b/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt
@@ -299,14 +299,6 @@ constructor(
         imageCaptureUseCase.flashMode == ImageCapture.FLASH_MODE_SCREEN &&
             imageCaptureUseCase.screenFlash != null
 
-    override fun clearScreenFlash() {
-        // In case a screen flash capture is already ongoing, first set screenFlashUiControl to null
-        // to stop receiving new events and then send a clearScreenFlashUi event in case an apply
-        // event already came through for the capture.
-        val prevScreenFlashUiControl = imageCaptureUseCase.screenFlash
-        prevScreenFlashUiControl?.clear()
-    }
-
     override suspend fun setAspectRatio(aspectRatio: AspectRatio, isFrontFacing: Boolean) {
         this.aspectRatio = aspectRatio
         updateUseCaseGroup()

diff --git a/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/test/FakeCameraUseCase.kt b/domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/test/FakeCameraUseCase.kt
@@ -125,10 +125,6 @@ class FakeCameraUseCase(
 
     override fun isScreenFlashEnabled() = isScreenFlash
 
-    override fun clearScreenFlash() {
-        isScreenFlash = false
-    }
-
     override suspend fun setAspectRatio(aspectRatio: AspectRatio, isFrontFacing: Boolean) {
         this.aspectRatio = aspectRatio
     }

diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt
@@ -55,7 +55,6 @@ import androidx.compose.ui.unit.dp
 import androidx.hilt.navigation.compose.hiltViewModel
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.repeatOnLifecycle
-import androidx.lifecycle.viewModelScope
 import com.google.jetpackcamera.feature.preview.ui.CaptureButton
 import com.google.jetpackcamera.feature.preview.ui.FlipCameraButton
 import com.google.jetpackcamera.feature.preview.ui.PreviewDisplay
@@ -69,7 +68,6 @@ import com.google.jetpackcamera.feature.quicksettings.ui.QuickSettingsIndicators
 import com.google.jetpackcamera.settings.model.CaptureMode
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.awaitCancellation
-import kotlinx.coroutines.launch
 
 private const val TAG = "PreviewScreen"
 private const val ZOOM_SCALE_SHOW_TIMEOUT_MS = 3000L
@@ -266,16 +264,13 @@ fun PreviewScreen(
         }
 
         // Screen flash overlay that stays on top of everything but invisible normally. This should
-        // not be enabled based on whether screen flash is enabled so that disabling is properly
-        // handled (e.g. brightness restored). Compose should be able to optimize this if the
-        // screenFlashUiState is no longer changing.
+        // not be enabled based on whether screen flash is enabled because a previous image capture
+        // may still be running after flash mode change and clear actions (e.g. brightness restore)
+        // may need to be handled later. Compose smart recomposition should be able to optimize this
+        // if the relevant states are no longer changing.
         ScreenFlashScreen(
             screenFlashUiState = screenFlashUiState,
-            onInitialBrightnessCalculated = { value ->
-                viewModel.viewModelScope.launch {
-                    viewModel.screenFlash.setClearUiScreenBrightness(value)
-                }
-            }
+            onInitialBrightnessCalculated = viewModel.screenFlash::setClearUiScreenBrightness
         )
     }
 }
diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt
@@ -88,7 +88,7 @@ class PreviewViewModel @Inject constructor(
 
     fun runCamera(surfaceProvider: SurfaceProvider) {
         Log.d(TAG, "runCamera")
-        stopCameraJob()
+        stopCamera()
         runningCameraJob = viewModelScope.launch {
             // TODO(yasith): Handle Exceptions from binding use cases
             cameraUseCase.runCamera(
@@ -100,12 +100,6 @@ class PreviewViewModel @Inject constructor(
 
     fun stopCamera() {
         Log.d(TAG, "stopCamera")
-        stopCameraJob()
-        cameraUseCase.clearScreenFlash()
-    }
-
-    private fun stopCameraJob() {
-        Log.d(TAG, "stopCameraJob")
         runningCameraJob?.apply {
             if (isActive) {
                 cancel()
@@ -132,7 +126,7 @@ class PreviewViewModel @Inject constructor(
     }
 
     fun setAspectRatio(aspectRatio: AspectRatio) {
-        stopCameraJob()
+        stopCamera()
         runningCameraJob = viewModelScope.launch {
             _previewUiState.emit(
                 previewUiState.value.copy(
@@ -164,7 +158,7 @@ class PreviewViewModel @Inject constructor(
             CaptureMode.SINGLE_STREAM -> CaptureMode.MULTI_STREAM
         }
 
-        stopCameraJob()
+        stopCamera()
         runningCameraJob = viewModelScope.launch {
             _previewUiState.emit(
                 previewUiState.value.copy(
@@ -185,7 +179,7 @@ class PreviewViewModel @Inject constructor(
         if (previewUiState.value.currentCameraSettings.isBackCameraAvailable &&
             previewUiState.value.currentCameraSettings.isFrontCameraAvailable
         ) {
-            stopCameraJob()
+            stopCamera()
             runningCameraJob = viewModelScope.launch {
                 _previewUiState.emit(
                     previewUiState.value.copy(

diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ScreenFlash.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ScreenFlash.kt
@@ -77,9 +77,11 @@ class ScreenFlash(
      * Sets the screenBrightness value to the value right before APPLY_UI event for the next
      * CLEAR_UI event, will be set to unknown (null) again after CLEAR_UI event is completed.
      */
-    suspend fun setClearUiScreenBrightness(brightness: Float) {
-        _screenFlashUiState.emit(
-            screenFlashUiState.value.copy(screenBrightnessToRestore = brightness)
-        )
+    fun setClearUiScreenBrightness(brightness: Float) {
+        scope.launch {
+            _screenFlashUiState.emit(
+                screenFlashUiState.value.copy(screenBrightnessToRestore = brightness)
+            )
+        }
     }
 }