Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
davidjiagoogle committed Aug 20, 2024
1 parent ecddaee commit 9de91d6
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 81 deletions.
8 changes: 3 additions & 5 deletions app/src/main/java/com/google/jetpackcamera/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class MainActivity : ComponentActivity() {
) {
JcaApp(
previewMode = getPreviewMode(),
isDebugMode = isDebugMode(),
isDebugMode = isDebugMode,
openAppSettings = ::openAppSettings,
onRequestWindowColorMode = { colorMode ->
// Window color mode APIs require API level 26+
Expand All @@ -162,10 +162,8 @@ class MainActivity : ComponentActivity() {
}
}

private fun isDebugMode(): Boolean {
return intent != null && intent.hasExtra(KEY_DEBUG_MODE) &&
intent.getBooleanExtra(KEY_DEBUG_MODE, false)
}
private val isDebugMode: Boolean
get() = intent?.getBooleanExtra(KEY_DEBUG_MODE, false) ?: false

private fun getStandardMode(): PreviewMode.StandardMode {
return PreviewMode.StandardMode { event ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,7 @@ class CameraXCameraUseCaseTest {
Dispatchers.Default,
constraintsRepository
).apply {
initialize(
appSettings,
CameraUseCase.UseCaseMode.STANDARD,
object : CameraUseCase.OnCameraIdChangeListener {
override fun onCameraIdChange(physicalCameraId: String?, logicalCameraId: String) {
}
}
)
initialize(appSettings, CameraUseCase.UseCaseMode.STANDARD)
providePreviewSurface()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ interface CameraUseCase {
*
* @return list of available lenses.
*/
suspend fun initialize(
cameraAppSettings: CameraAppSettings,
useCaseMode: UseCaseMode,
onCameraIdChangeListener: OnCameraIdChangeListener
)
suspend fun initialize(cameraAppSettings: CameraAppSettings, useCaseMode: UseCaseMode)

/**
* Starts the camera.
Expand Down Expand Up @@ -143,14 +139,13 @@ interface CameraUseCase {
IMAGE_ONLY,
VIDEO_ONLY
}

interface OnCameraIdChangeListener {
fun onCameraIdChange(physicalCameraId: String?, logicalCameraId: String)
}
}

data class CameraState(
val zoomScale: Float = 1f,
val sessionFirstFrameTimestamp: Long = 0L,
val torchEnabled: Boolean = false
val torchEnabled: Boolean = false,
val debugInfo: DebugInfo = DebugInfo(null, null)
)

data class DebugInfo(val logicalCameraId: String?, val physicalCameraId: String?)
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,14 @@ constructor(
)
}
val logicalCameraId = session.device.id
if (onCameraIdChangeListener != null &&
(
!physicalCameraId.equals(this.physicalCameraId) ||
logicalCameraId != this.logicalCameraId
)
if (!physicalCameraId.equals(this.physicalCameraId) ||
logicalCameraId != this.logicalCameraId
) {
onCameraIdChangeListener!!.onCameraIdChange(
physicalCameraId,
logicalCameraId
)
_currentCameraState.update { old ->
old.copy(
debugInfo = DebugInfo(logicalCameraId, physicalCameraId)
)
}
}
try {
if (!isFirstFrameTimestampUpdated.value) {
Expand Down Expand Up @@ -213,15 +211,11 @@ constructor(

private val currentSettings = MutableStateFlow<CameraAppSettings?>(null)

private var onCameraIdChangeListener: CameraUseCase.OnCameraIdChangeListener? = null

override suspend fun initialize(
cameraAppSettings: CameraAppSettings,
useCaseMode: CameraUseCase.UseCaseMode,
onCameraIdChangeListener: CameraUseCase.OnCameraIdChangeListener
) {
this.useCaseMode = useCaseMode
this.onCameraIdChangeListener = onCameraIdChangeListener
cameraProvider = ProcessCameraProvider.awaitInstance(application)

// updates values for available cameras
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ class FakeCameraUseCase(

override suspend fun initialize(
cameraAppSettings: CameraAppSettings,
useCaseMode: CameraUseCase.UseCaseMode,
onCameraIdChangeListener: CameraUseCase.OnCameraIdChangeListener
useCaseMode: CameraUseCase.UseCaseMode
) {
initialized = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ class FakeCameraUseCaseTest {
fun canInitialize() = runTest(testDispatcher) {
cameraUseCase.initialize(
cameraAppSettings = DEFAULT_CAMERA_APP_SETTINGS,
useCaseMode = CameraUseCase.UseCaseMode.STANDARD,
onCameraIdChangeListener = object : CameraUseCase.OnCameraIdChangeListener {
override fun onCameraIdChange(physicalCameraId: String?, logicalCameraId: String) {
}
}
useCaseMode = CameraUseCase.UseCaseMode.STANDARD
)
}

Expand Down Expand Up @@ -154,14 +150,7 @@ class FakeCameraUseCaseTest {
backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
cameraUseCase.initialize(
cameraAppSettings = DEFAULT_CAMERA_APP_SETTINGS,
useCaseMode = CameraUseCase.UseCaseMode.STANDARD,
onCameraIdChangeListener = object : CameraUseCase.OnCameraIdChangeListener {
override fun onCameraIdChange(
physicalCameraId: String?,
logicalCameraId: String
) {
}
}
useCaseMode = CameraUseCase.UseCaseMode.STANDARD
)
cameraUseCase.runCamera()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,6 @@ class PreviewViewModel @AssistedInject constructor(
cameraUseCase.initialize(
cameraAppSettings = settingsRepository.defaultCameraAppSettings.first(),
previewMode.toUseCaseMode(),
onCameraIdChangeListener = object : CameraUseCase.OnCameraIdChangeListener {
override fun onCameraIdChange(physicalCameraId: String?, logicalCameraId: String) {
_previewUiState.update { old ->
(old as? PreviewUiState.Ready)?.copy(
currentPhysicalCameraId = physicalCameraId,
currentLogicalCameraId = logicalCameraId
) ?: old
}
}
}
)
}

Expand Down Expand Up @@ -153,7 +143,9 @@ class PreviewViewModel @AssistedInject constructor(
systemConstraints,
cameraAppSettings
),
isDebugMode = isDebugMode
isDebugMode = isDebugMode,
currentLogicalCameraId = cameraState.debugInfo.logicalCameraId,
currentPhysicalCameraId = cameraState.debugInfo.physicalCameraId
)

is PreviewUiState.NotReady ->
Expand All @@ -167,7 +159,9 @@ class PreviewViewModel @AssistedInject constructor(
systemConstraints,
cameraAppSettings
),
isDebugMode = isDebugMode
isDebugMode = isDebugMode,
currentLogicalCameraId = cameraState.debugInfo.logicalCameraId,
currentPhysicalCameraId = cameraState.debugInfo.physicalCameraId
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import androidx.compose.material.icons.filled.Videocam
import androidx.compose.material.icons.outlined.CameraAlt
import androidx.compose.material.icons.outlined.Videocam
import androidx.compose.material3.LocalContentColor
import androidx.compose.material3.LocalTextStyle
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.LaunchedEffect
Expand All @@ -48,6 +49,7 @@ import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.core.util.Preconditions
import com.google.jetpackcamera.feature.preview.CaptureModeToggleUiState
import com.google.jetpackcamera.feature.preview.MultipleEventsCutter
Expand Down Expand Up @@ -234,12 +236,16 @@ private fun ControlsBottom(
onStopVideoRecording: () -> Unit = {}
) {
Column(modifier = modifier, horizontalAlignment = Alignment.CenterHorizontally) {
Row(verticalAlignment = Alignment.CenterVertically) {
if (showZoomLevel) {
ZoomScaleText(zoomLevel)
}
if (previewUiState.isDebugMode) {
CurrentCameraIdText(physicalCameraId, logicalCameraId)
CompositionLocalProvider(
LocalTextStyle provides LocalTextStyle.current.copy(fontSize = 20.sp)
) {
Column(horizontalAlignment = Alignment.CenterHorizontally) {
if (showZoomLevel) {
ZoomScaleText(zoomLevel)
}
if (previewUiState.isDebugMode) {
CurrentCameraIdText(physicalCameraId, logicalCameraId)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,9 @@ fun ZoomScaleText(zoomScale: Float) {
)
Text(
modifier = Modifier
.padding(horizontal = 10.dp)
.alpha(contentAlpha.value)
.testTag(ZOOM_RATIO_TAG),
text = "%.1fx".format(zoomScale),
fontSize = 20.sp
)
}

Expand All @@ -444,8 +442,7 @@ fun CurrentCameraIdText(physicalCameraId: String?, logicalCameraId: String?) {
modifier = Modifier
.testTag(CURRENT_CAMERA_ID_TAG),
text = "physical id: $physicalCameraId, logical id: $logicalCameraId",
fontSize = 20.sp
)
)
}

@Composable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,7 @@ class ScreenFlashTest {
backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
cameraUseCase.initialize(
DEFAULT_CAMERA_APP_SETTINGS,
CameraUseCase.UseCaseMode.STANDARD,
onCameraIdChangeListener = object : CameraUseCase.OnCameraIdChangeListener {
override fun onCameraIdChange(
physicalCameraId: String?,
logicalCameraId: String
) {
}
}
CameraUseCase.UseCaseMode.STANDARD
)
cameraUseCase.runCamera()
}
Expand Down

0 comments on commit 9de91d6

Please sign in to comment.