diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md index 649159a2f..c7e9abe8e 100644 --- a/.gemini/styleguide.md +++ b/.gemini/styleguide.md @@ -70,6 +70,7 @@ When reviewing a pull request, focus on the following key areas: * **Scrutinize Debug Logs:** Question the use of `Log.d`, `Log.v`, and especially `println()`. These are often remnants of debugging and should be removed before merging unless they provide essential, long-term value. Calls to `println()` should always be replaced with a proper `Log` method. * **KDoc for Complexity:** For new or significantly modified functions that are complex, have non-obvious logic, or a large number of parameters, suggest adding KDoc comments. Good documentation should explain the function's purpose, its parameters, and what it returns. * **Keep KDoc Synchronized:** If a PR modifies a function with existing KDocs, verify that the comments are still accurate. Outdated documentation can be more misleading than no documentation at all. + * **Trace Naming Scheme:** Standardize on `ComponentName.actionName` or `ClassName.methodName` for custom trace sections to make them easily searchable in Perfetto. [Introduced in PR #518] 11. **KDoc Documentation Standards** * **Document all non-private members:** All non-private classes, functions, and composables must have KDoc documentation. diff --git a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt index 09c6f0472..5925dc9bb 100644 --- a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt +++ b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt @@ -42,6 +42,7 @@ import androidx.compose.ui.test.performTouchInput import androidx.compose.ui.test.printToString import androidx.test.core.app.ApplicationProvider import androidx.test.espresso.action.ViewActions.swipeDown +import androidx.tracing.trace import com.google.common.truth.Truth.assertThat import com.google.errorprone.annotations.CanIgnoreReturnValue import com.google.jetpackcamera.model.CaptureMode @@ -249,60 +250,87 @@ fun ComposeTestRule.pressAndDragToLockVideoRecording( onNodeWithTag(VIDEO_CAPTURE_FAILURE_TAG).assertIsNotDisplayed() } ) { - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - down(center) + trace("pressAndDragToLockVideoRecording") { + onNodeWithTag(CAPTURE_BUTTON).assertExists().performTouchInput { + trace("down(center)") { down(center) } + trace("wait for long press and trigger onDragStart") { + advanceEventTime(viewConfiguration.longPressTimeoutMillis + 100) + moveBy(delta = Offset(1f, 0f)) + } } - waitUntil(timeoutMillis = ELAPSED_TIME_TEXT_TIMEOUT_MILLIS) { - checkWhileWaiting() - onNodeWithTag(ELAPSED_TIME_TAG).isDisplayed() - } - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - moveBy(delta = Offset(-400f, 0f)) - up() + + // Wait for recording to start (timer displayed) + trace("wait for ELAPSED_TIME_TAG") { + waitUntil(timeoutMillis = ELAPSED_TIME_TEXT_TIMEOUT_MILLIS) { + onNodeWithTag(ELAPSED_TIME_TAG).isDisplayed() + } } - waitUntilVideoRecordingDurationAtLeast(durationMillis, checkWhileWaiting) + + onNodeWithTag(CAPTURE_BUTTON).performTouchInput { + trace("drag to lock") { + val steps = 10 + val deltaX = -400f / steps + for (i in 1..steps) { + moveBy(delta = Offset(deltaX, 0f)) + } + } + } + + onNodeWithTag(CAPTURE_BUTTON).performTouchInput { trace("up()") { up() } } + + // Wait for the recording to reach desired duration + trace("waitUntilVideoRecordingDurationAtLeast") { + waitUntilVideoRecordingDurationAtLeast(durationMillis, checkWhileWaiting) + } + } } fun ComposeTestRule.longClickForVideoRecordingCheckingElapsedTime( durationMillis: Long = VIDEO_DURATION_MILLIS, checkWhileWaiting: () -> Unit = { // If the video capture fails, there is no point to continue waiting. Assert. - onNodeWithTag(VIDEO_CAPTURE_FAILURE_TAG).assertIsNotDisplayed() + if (onAllNodesWithTag(VIDEO_CAPTURE_FAILURE_TAG).fetchSemanticsNodes().isNotEmpty()) { + throw AssertionError("Video capture failed!") + } } ) { - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - down(center) + trace("longClickForVideoRecordingCheckingElapsedTime") { + onNodeWithTag(CAPTURE_BUTTON).assertExists().performTouchInput { + trace("down(center)") { down(center) } + + trace("wait for long press and trigger onDragStart") { + advanceEventTime(viewConfiguration.longPressTimeoutMillis + 100) + moveBy(delta = Offset(1f, 0f)) + } } - waitUntil(timeoutMillis = ELAPSED_TIME_TEXT_TIMEOUT_MILLIS) { - checkWhileWaiting() - onNodeWithTag(ELAPSED_TIME_TAG).isDisplayed() - } - waitUntilVideoRecordingDurationAtLeast(durationMillis, checkWhileWaiting) - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - up() + + // Wait for recording to start (timer displayed) + trace("wait for ELAPSED_TIME_TAG") { + waitUntil(timeoutMillis = ELAPSED_TIME_TEXT_TIMEOUT_MILLIS) { + onAllNodesWithTag(ELAPSED_TIME_TAG).fetchSemanticsNodes().isNotEmpty() + } } + + // Wait for the desired duration outside performTouchInput + waitUntilVideoRecordingDurationAtLeast(durationMillis, checkWhileWaiting) + + // Complete the gesture (release touch) + trace("up() after long click") { onNodeWithTag(CAPTURE_BUTTON).performTouchInput { up() } } + } } fun ComposeTestRule.longClickForVideoRecording(durationMillis: Long = VIDEO_DURATION_MILLIS) { - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - down(center) - } - idleForVideoDuration(durationMillis) - onNodeWithTag(CAPTURE_BUTTON) - .assertExists() - .performTouchInput { - up() + trace("longClickForVideoRecording") { + onNodeWithTag(CAPTURE_BUTTON).assertExists().performTouchInput { + trace("down(center)") { down(center) } + trace("wait for long press and trigger onDragStart") { + advanceEventTime(viewConfiguration.longPressTimeoutMillis + 100) + moveBy(delta = Offset(1f, 0f)) + } + trace("wait for duration") { advanceEventTime(durationMillis) } + trace("up()") { up() } } + } } fun ComposeTestRule.tapStartLockedVideoRecording() { diff --git a/ui/components/capture/build.gradle.kts b/ui/components/capture/build.gradle.kts index 70395892c..d5f35327b 100644 --- a/ui/components/capture/build.gradle.kts +++ b/ui/components/capture/build.gradle.kts @@ -81,6 +81,9 @@ dependencies { implementation(libs.camera.core) implementation(libs.camera.compose) + // Tracing + implementation(libs.androidx.tracing) + // Compose - Testing androidTestImplementation(libs.compose.junit) debugImplementation(libs.compose.test.manifest) diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonComponents.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonComponents.kt index f9a1e5f14..eb24ff095 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonComponents.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonComponents.kt @@ -72,6 +72,7 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.core.view.ViewCompat +import androidx.tracing.trace import com.google.jetpackcamera.model.CaptureMode import com.google.jetpackcamera.ui.uistate.capture.CaptureButtonUiState import kotlinx.coroutines.Job @@ -211,22 +212,24 @@ internal fun CaptureButton( } } fun onLongPress() { - if (!isLongPressing.value) { - when (val current = currentUiState.value) { - is CaptureButtonUiState.Enabled.Idle -> when (current.captureMode) { - CaptureMode.STANDARD, - CaptureMode.VIDEO_ONLY -> { - isLongPressing.value = true - Log.d(TAG, "Starting recording") - onStartRecording() - } + trace("CaptureButton.onLongPress") { + if (!isLongPressing.value) { + when (val current = currentUiState.value) { + is CaptureButtonUiState.Enabled.Idle -> when (current.captureMode) { + CaptureMode.STANDARD, + CaptureMode.VIDEO_ONLY -> { + isLongPressing.value = true + Log.d(TAG, "Starting recording") + onStartRecording() + } - CaptureMode.IMAGE_ONLY -> { - isLongPressing.value = true + CaptureMode.IMAGE_ONLY -> { + isLongPressing.value = true + } } - } - else -> {} + else -> {} + } } } } @@ -339,7 +342,7 @@ private fun CaptureButton( captureButtonSize: Float = DEFAULT_CAPTURE_BUTTON_SIZE ) { // todo: explore MutableInteractionSource - var isCaptureButtonPressed by remember { + var isTapping by remember { mutableStateOf(false) } @@ -389,7 +392,7 @@ private fun CaptureButton( fun toggleSwitchPosition() = if (shouldBeLocked()) { switchPosition = LOCK_SWITCH_POSITION_OFF } else { - if (!isCaptureButtonPressed) { + if (!isTapping) { onLockVideoRecording(true) } else { switchPosition = @@ -404,10 +407,15 @@ private fun CaptureButton( // touch is dragged off the component onLongPress = {}, onPress = { - isCaptureButtonPressed = true - onPress() - awaitRelease() - isCaptureButtonPressed = false + isTapping = true + try { + trace("CaptureButton.onPress") { + onPress() + } + awaitRelease() + } finally { + isTapping = false + } if (shouldBeLocked()) { onLockVideoRecording(true) onRelease(true) @@ -421,35 +429,45 @@ private fun CaptureButton( .pointerInput(Unit) { detectDragGesturesAfterLongPress( onDragStart = {}, - onDragEnd = {}, - onDragCancel = {}, + onDragEnd = { + trace("CaptureButton.onDragEnd") { + onRelease(shouldBeLocked()) + } + }, + onDragCancel = { + trace("CaptureButton.onDragCancel") { + onRelease(false) + } + }, onDrag = { change, deltaOffset -> - if (currentUiState.value == - CaptureButtonUiState.Enabled.Recording.PressedRecording - ) { - val newPoint = change.position - - // update position of lock switch - setLockSwitchPosition(newPoint.x, deltaOffset.x) - - // update zoom - val previousPoint = change.position - deltaOffset - val positiveDistance = - if (newPoint.y >= 0 && previousPoint.y >= 0) { - // 0 if both points are within bounds - 0f - } else if (newPoint.y < 0 && previousPoint.y < 0) { - deltaOffset.y - } else if (newPoint.y <= 0) { - newPoint.y - } else { - previousPoint.y + trace("CaptureButton.onDrag") { + if (currentUiState.value == + CaptureButtonUiState.Enabled.Recording.PressedRecording + ) { + val newPoint = change.position + + // update position of lock switch + setLockSwitchPosition(newPoint.x, deltaOffset.x) + + // update zoom + val previousPoint = change.position - deltaOffset + val positiveDistance = + if (newPoint.y >= 0 && previousPoint.y >= 0) { + // 0 if both points are within bounds + 0f + } else if (newPoint.y < 0 && previousPoint.y < 0) { + deltaOffset.y + } else if (newPoint.y <= 0) { + newPoint.y + } else { + previousPoint.y + } + + if (!positiveDistance.isNaN()) { + // todo(kc): should check the tuning of this. + val zoom = positiveDistance * -0.01f // Adjust sensitivity + onDragZoom(zoom) } - - if (!positiveDistance.isNaN()) { - // todo(kc): should check the tuning of this. - val zoom = positiveDistance * -0.01f // Adjust sensitivity - onDragZoom(zoom) } } } @@ -485,7 +503,7 @@ private fun CaptureButton( } else { CaptureButtonNucleus( captureButtonUiState = captureButtonUiState, - isPressed = isCaptureButtonPressed, + isTapping = isTapping, captureButtonSize = captureButtonSize ) } @@ -572,7 +590,7 @@ private fun LockSwitchCaptureButtonNucleus( captureButtonSize = captureButtonSize, captureButtonUiState = captureButtonUiState, pressedVideoCaptureScale = LOCK_SWITCH_PRESSED_NUCLEUS_SCALE, - isPressed = false + isTapping = false ) // locked icon, matches cylinder offset @@ -617,7 +635,7 @@ private fun LockSwitchCaptureButtonNucleus( private fun CaptureButtonNucleus( modifier: Modifier = Modifier, captureButtonUiState: CaptureButtonUiState, - isPressed: Boolean, + isTapping: Boolean, captureButtonSize: Float, offsetX: Dp = 0.dp, recordingColor: Color = Color.Red, @@ -685,7 +703,7 @@ private fun CaptureButtonNucleus( .size(centerShapeSize) .clip(CircleShape) .alpha( - if (isPressed && + if (isTapping && currentUiState.value == CaptureButtonUiState.Enabled.Idle(CaptureMode.IMAGE_ONLY) ) { @@ -736,7 +754,7 @@ private fun IdleStandardCaptureButtonPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.STANDARD), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -748,7 +766,7 @@ private fun IdleImageCaptureButtonPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.IMAGE_ONLY), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -760,7 +778,7 @@ private fun IdleVideoOnlyCaptureButtonPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.VIDEO_ONLY), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -775,7 +793,7 @@ private fun IdleStandardCaptureButtonDisabledPreview() { CaptureMode.STANDARD, isEnabled = false ), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -790,7 +808,7 @@ private fun IdleImageCaptureButtonDisabledPreview() { CaptureMode.IMAGE_ONLY, isEnabled = false ), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -805,7 +823,7 @@ private fun IdleVideoOnlyCaptureButtonDisabledPreview() { CaptureMode.VIDEO_ONLY, isEnabled = false ), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -817,7 +835,7 @@ private fun PressedImageCaptureButtonPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.IMAGE_ONLY), - isPressed = true, + isTapping = true, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -829,7 +847,7 @@ private fun IdleRecordingCaptureButtonPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Idle(CaptureMode.VIDEO_ONLY), - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -841,7 +859,7 @@ private fun SimpleNucleusPressedRecordingPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Recording.PressedRecording, - isPressed = true, + isTapping = true, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) } @@ -853,7 +871,7 @@ private fun LockedRecordingPreview() { CaptureButtonRing(captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE, color = Color.White) { CaptureButtonNucleus( captureButtonUiState = CaptureButtonUiState.Enabled.Recording.LockedRecording, - isPressed = false, + isTapping = false, captureButtonSize = DEFAULT_CAPTURE_BUTTON_SIZE ) }