diff --git a/feature/camera/src/main/java/org/signal/camera/CameraScreenViewModel.kt b/feature/camera/src/main/java/org/signal/camera/CameraScreenViewModel.kt index 15d15355b9..0cfdab4212 100644 --- a/feature/camera/src/main/java/org/signal/camera/CameraScreenViewModel.kt +++ b/feature/camera/src/main/java/org/signal/camera/CameraScreenViewModel.kt @@ -94,8 +94,11 @@ class CameraScreenViewModel : ViewModel() { /** Requested resolution for the QR analysis stream. */ private val QR_ANALYSIS_RESOLUTION = Size(1280, 720) + /** Where a lens comes up after a bind, and so where the zoom goes back to on every bind. */ + private const val DEFAULT_ZOOM_RATIO = 1f + /** A single point, so a lens that has not reported its range yet reads as one that cannot zoom. */ - private val DEFAULT_ZOOM_RANGE = 1f..1f + private val DEFAULT_ZOOM_RANGE = DEFAULT_ZOOM_RATIO..DEFAULT_ZOOM_RATIO /** How long a zoom animation to a level picked off the zoom bar runs for. */ private const val ZOOM_ANIMATION_DURATION_MS = 250L @@ -123,7 +126,7 @@ class CameraScreenViewModel : ViewModel() { private var brightnessWindow: WeakReference? = null private var deviceTargetRotation: Int = Surface.ROTATION_0 private var surfaceProvider: Preview.SurfaceProvider? = null - private var recordingStartZoomRatio: Float = 1f + private var recordingStartZoomRatio: Float = DEFAULT_ZOOM_RATIO /** The in-flight animation to a level picked off the zoom bar. Anything else that moves the zoom cancels it. */ private var zoomAnimation: Job? = null @@ -135,12 +138,6 @@ class CameraScreenViewModel : ViewModel() { */ private var pendingRecordingLock: Boolean = false - /** - * Set when a lens is bound, so that the first zoom it reports is taken as the current ratio. A rebind comes up at the - * new lens's own zoom rather than carrying the previous one's over. - */ - private var needsZoomResync: Boolean = false - /** Null for a recording that finalizes without being asked to stop, such as one that hits the recorder's own limits. */ private var recordingStopwatch: Stopwatch? = null @@ -168,12 +165,7 @@ class CameraScreenViewModel : ViewModel() { private val zoomRangeObserver = Observer { zoomState -> val zoomRange = zoomState.minZoomRatio..zoomState.maxZoomRatio - if (needsZoomResync) { - needsZoomResync = false - Log.d(TAG, "Bound lens reaches $zoomRange at ${zoomState.zoomRatio}x") - recordingStartZoomRatio = zoomState.zoomRatio - _state.value = _state.value.copy(zoomRange = zoomRange, zoomRatio = zoomState.zoomRatio) - } else if (zoomRange != _state.value.zoomRange) { + if (zoomRange != _state.value.zoomRange) { Log.d(TAG, "Bound lens reaches $zoomRange") _state.value = _state.value.copy(zoomRange = zoomRange) } @@ -909,20 +901,25 @@ class CameraScreenViewModel : ViewModel() { } /** - * Observes what the bound lens can reach. A camera reports its zoom when it is ready rather than by the time it is - * bound, so this observes rather than taking a single reading — a lens whose range arrives late would otherwise look - * like one that cannot zoom. + * Puts the zoom back to [DEFAULT_ZOOM_RATIO] and observes what the bound lens can reach. A camera reports its range + * when it is ready rather than by the time it is bound, so this observes rather than taking a single reading — a lens + * whose range arrives late would otherwise look like one that cannot zoom. */ private fun observeZoomRange() { val zoomState = camera?.cameraInfo?.zoomState + // Every bind here follows an unbind, which drops the lens back to its default zoom. The state follows it back down, + // and an in-flight travel to a level picked before the unbind is left with nowhere to land. + zoomAnimation?.cancel() + recordingStartZoomRatio = DEFAULT_ZOOM_RATIO + _state.value = _state.value.copy(zoomRatio = DEFAULT_ZOOM_RATIO) + if (zoomState === observedZoomState) { return } observedZoomState?.removeObserver(zoomRangeObserver) observedZoomState = zoomState - needsZoomResync = true if (zoomState != null) { zoomState.observeForever(zoomRangeObserver) diff --git a/feature/camera/src/test/java/org/signal/camera/CameraScreenViewModelTest.kt b/feature/camera/src/test/java/org/signal/camera/CameraScreenViewModelTest.kt index 05974ed63d..492f479f18 100644 --- a/feature/camera/src/test/java/org/signal/camera/CameraScreenViewModelTest.kt +++ b/feature/camera/src/test/java/org/signal/camera/CameraScreenViewModelTest.kt @@ -748,17 +748,38 @@ class CameraScreenViewModelTest { assertThat(viewModel.state.value.zoomRange).isEqualTo(1f..3f) } - /** A newly bound lens comes up at its own zoom rather than carrying over whatever the one before it was at. */ + /** + * Coming back into the camera rebinds it, which drops the lens back to its default zoom, so the zoom bar goes back + * with it rather than keeping a level highlighted over a preview that is no longer at it. + */ @Test - fun `Given a lens sitting away from 1x, when bound, then its own zoom is published`() { - setupZoomState(minZoom = 1f, maxZoom = 10f, zoomRatio = 5f) + fun `Given a lens that has been zoomed, when rebound, then the zoom is back at 1x`() { + setupZoomState(minZoom = 1f, maxZoom = 8f) + bindCamera() + viewModel.onEvent(CameraScreenEvents.SetZoomRatio(5f)) + testDispatcher.scheduler.advanceUntilIdle() + assertThat(viewModel.state.value.zoomRatio).isEqualTo(5f) bindCamera() - assertThat(viewModel.state.value.zoomRatio).isEqualTo(5f) + assertThat(viewModel.state.value.zoomRatio).isEqualTo(1f) } - /** The resync happens once per binding, so it cannot pull the lens back from wherever it has since been sent. */ + /** The reset happens on the bind itself, so a lens reporting afterwards cannot pull it back off 1x. */ + @Test + fun `Given a rebound lens, when it reports the zoom it was left at, then the zoom stays at 1x`() { + setupZoomState(minZoom = 1f, maxZoom = 8f) + bindCamera() + viewModel.onEvent(CameraScreenEvents.SetZoomRatio(5f)) + testDispatcher.scheduler.advanceUntilIdle() + + bindCamera() + setupZoomState(minZoom = 1f, maxZoom = 8f, zoomRatio = 5f) + + assertThat(viewModel.state.value.zoomRatio).isEqualTo(1f) + } + + /** The reset happens on the bind, so it cannot pull the lens back from wherever it has since been sent. */ @Test fun `Given a bound lens that has been zoomed, when it reports again, then the zoom it was sent to stands`() { setupZoomState(minZoom = 1f, maxZoom = 10f, zoomRatio = 1f)