Repository navigation
Implement phone landscape mode with window counter-rotation and upright icon animations - #619
davidjiagoogle wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive device and window rotation handling for the camera capture screen, including UI element counter-rotation, landscape layout adjustments, and orientation locking during video recording. The review feedback highlights a potential layout flicker issue in PreviewScreen.kt due to redundant orientation resets, which can be resolved by separating lifecycle cleanup from active state updates. Additionally, several visibility modifiers in RotationModifiers.kt should be restricted to internal to comply with the repository style guide, and the DeviceRotationTest class requires a @RunWith annotation.
185d5ca to
cd67e1d
Compare
cd67e1d to
19ed975
Compare
| Build.VERSION.SDK_INT > 24 -> ImplementationMode.EXTERNAL | ||
| else -> ImplementationMode.EMBEDDED | ||
| } | ||
| val implementationMode = ImplementationMode.EMBEDDED |
There was a problem hiding this comment.
Switching this to ImplementationMode.EMBEDDED forces TextureView instead of SurfaceView, which causes a few problems:
When you first launch the app in STANDARD mode (Preview + ImageCapture + VideoCapture), CameraX uses StreamSharing and pre-rotates the buffers in OpenGL (hasCameraTransform = false / TransformationMode.PRE_APPLIED), so Viewfinder skips getTextureViewCorrectionMatrix and counterRotateToPortrait rotates the preview as expected. Once you switch modes (like switching to video mode or photo-only mode), StreamSharing isn't used anymore (hasCameraTransform = true / TransformationMode.DEFERRED), and the viewfinder gets stretched and mis-rotated in landscape because Viewfinder applies getTextureViewCorrectionMatrix(displayRotationDegrees) for the landscape window on top of counterRotateToPortrait while sourceRotation stays at ROTATION_0.
Using EMBEDDED also drops the SurfaceView hardware overlay path, and it breaks 10-bit HDR preview because DetectWindowColorModeChanges right above this (L732) checks implMode == ImplementationMode.EXTERNAL before setting COLOR_MODE_HDR (and TextureView can't render 10-bit HDR).
In #162, Preview.targetRotation was pinned to Surface.ROTATION_0 (72fa358c4c) because updating Preview.targetRotation dynamically on window rotation is asynchronous and lags behind the Compose configuration change. Back then, #162 kept PreviewDisplay outside rotatedLayout (counterRotateToPortrait), stayed on ImplementationMode.EXTERNAL, and adjusted transformationInfo.sourceRotation synchronously during composition using LocalView.current.display.rotation (73dcf435ea, when JCA had its own wrapper around Viewfinder).
Now that we use androidx.camera.compose.CameraXViewfinder, we don't have a way to intercept TransformationInfo in JCA, and Viewfinder / StreamSharing in CameraX don't currently handle dynamic display rotation cleanly across both PRE_APPLIED (StreamSharing) and DEFERRED (direct camera) streams without either stretching the preview, lagging on window rotation, or breaking ImplementationMode.EXTERNAL (and 10-bit HDR). I am going to file bugs against CameraX to get this fixed upstream first, since we'll need CameraX support before we can properly support this flow.
There was a problem hiding this comment.
Is there anything in this PR that should be done regarding this? Or just wait for the CameraX fix?
There was a problem hiding this comment.
I'm looking into how easy this would be to fix in CameraX. I'll let you know.
Summary
Implements phone landscape mode (
Rotated90andRotated270) for Jetpack Camera App using root counter-rotation and upright icon animations while preserving JCA's UDF state pipeline:Manifest & Window Configuration (
AndroidManifest.xml,MainActivity.kt):android:screenOrientation="fullUser"), addsorientation|screenSize|screenLayout|smallestScreenSizetoandroid:configChangesto avoidCameraSessionre-binding on rotation, and setsrotationAnimation = ROTATION_ANIMATION_JUMPCUTfor zero-frame-drop OS rotation transitions.Configuration.orientationinMainActivity(isSystemInLandscape) to feed intoCaptureUiState.Sensor Hysteresis & Shortest-Path Rotation (
DeviceRotation.kt,RotationModifiers.kt,CameraSession.kt):HYSTERESIS_DEGREES = 20) around 45°/135°/225°/315° thresholds inDeviceRotation.fromSensorDegrees(orientation, previousRotation)to prevent boundary flicker.Modifier.counterRotateWithSystem(deviceRotation, isSystemInLandscape)to counter-rotate the root container back to the portrait coordinate frame when the OS rotates to landscape, keeping controls anchored by the physical charging port and top bezel.Modifier.animateToUpright(deviceRotation)andModifier.rotateLayout90()with shortest-path angle unwrapping (targetShortestAngle) so 270° ↔ 0° transitions smoothly animate across 90° instead of spinning 270°.Capture & Quick Settings UI Adaptation (
CaptureLayout.kt,CaptureScreenComponents.kt,QuickSettingsScreen.kt, etc.):PreviewLayoutand rotates individual icons/buttons (FlipCameraButton,ImageWell,QuickSettingsDropDown,FlashModeIndicator,HdrIndicator, zoom preset text labels, elapsed timer, snackbars,CameraErrorDialog,ToggleSwitchicons, andtopStartContent) upright in place.topStartContenttoAlignment.TopEndin standard landscape (DeviceRotation.Rotated270) so it stays at the visual top-left of the screen.QuickSettingsScreenOverlayinto a 410dp landscape side sheet (AnimatedVisibilityhorizontal slide +rotateLayout90) in landscape orientations.Testing
DeviceRotationTest.ktandCaptureUiStateAdapterTest.kt.Natural), Standard Landscape (Rotated270), Reverse Landscape (Rotated90), and Landscape Quick Settings side sheet on Cuttlefish emulator.