Repository navigation
Conversation
Replaces the linear-interpolation portrait control placement in CaptureLayout with AdaptivePortraitCaptureLayout, a custom Compose Layout driven by CameraLayoutSolver: - Measures the live control row composables at runtime and reads WindowInsets and LocalMinimumInteractiveComponentSize to construct CameraWindow and CameraLayoutSpec. - Places the Top Bar, Zoom Bar, Elapsed Time, Capture Row, Mode Switcher, Bottom Toolbar, and Viewfinder at the exact vertical bands computed by CameraLayoutSolver. - Provides LocalCameraLayoutSolution so PreviewDisplay positions 3:4, 1:1, and 9:16 viewfinders at their solved bands while keeping the control stack stationary. - Drives ImmersiveNavigationBarEffect when requiresImmersive is true on 16:9 and 17:9 portrait screens.
CameraLayoutSolver.solve was being called from inside the Layout measure lambda whenever a measured row height differed from the design default, which is the normal case on a real device, so the solve ran on every measure pass rather than once per window geometry change. The resulting solution was also written back into snapshot state during the layout phase, which invalidates composition from layout and can oscillate. The solve now runs in a remember keyed on the spec and the window, and the measure pass places each control row into the band the solver already reserved for it. This also restores the ControlRow.height contract: a row reports its reserved height, not its measured content height, so showing or hiding a row's content can no longer move the rows around it.
The search cost was documented but not measured, so there was no way to tell a real regression from a comment that had drifted. Every solve now reports how many candidate layouts it evaluated, and the property tests assert that number instead of only wall clock. Wall clock alone is the wrong assertion. It moves with the machine and the JIT, so a bound loose enough to be reliable is too loose to catch anything. Evaluation count is deterministic and exactly proportional to the work done, so it can be bounded tightly. The timing test is kept, narrowed to what it is actually good for: catching a single evaluation becoming expensive. The measured distribution across the supported sweep of 1,568 windows: evaluations | windows ------------|-------- 1 | 897 2 to 10 | 120 11 to 100 | 545 over 1000 | 6 So 57% of supported windows need no search at all, and only 6 ever pay for a full gap sweep. The worst is 1,055 evaluations, about 1.6 ms, once per window geometry rather than per frame. This was written to justify a branch-and-bound optimisation of the padding sweep. Measurement refuted it: the bound available before evaluating a padding assumes zero frame displacement, but a layout that needed repair has almost always displaced a frame, so the bound never beat the incumbent and never pruned. It changed the in-scope cost by zero evaluations. That work is dropped and only the measurement is kept.
The cost of a solve had only ever been measured on a desktop JVM. That machine is both much faster per core and a different runtime from the one this ships on, so the numbers could not support any claim about device behaviour. This makes the measurement possible on real hardware. Four window geometries are benchmarked rather than one, because how much work a solve does is a property of the window, not of the solver. The designed layout fits outright on 10 of the 22 device configurations in the golden fixture and costs a single candidate evaluation; the worst real device costs 96; the worst supported window anywhere costs 1,055. Benchmarking one geometry would report an arbitrary point on that range. Comparing the first two separates the cost of searching from the fixed cost of a solve, which is the number worth knowing before optimising either. The androidx.benchmark Gradle plugin is deliberately not applied. The version in the catalog reaches for an extension this AGP no longer registers, so applying it fails. It is a convenience wrapper, and the parts that matter here are done explicitly in the build file instead. Note that a microbenchmark reports warm steady-state cost, which is not what the user pays. The solve runs once, cold, during the first composition, and on the desktop JVM that first call was four orders of magnitude more expensive than the warm one. Measuring that needs a trace section and the macrobenchmark module; this does not replace it.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new microbenchmark module to measure the performance of the CameraLayoutSolver on real hardware, and implements an adaptive portrait layout (AdaptivePortraitCaptureLayout) that dynamically solves and positions the camera control rows and viewfinder. It also refactors system bar handling to support immersive navigation bar behavior on shorter screens. The review feedback highlights compilation issues in CaptureLayout.kt due to direct division of Dp operands, identifies several magic numbers that should be extracted to constants, and suggests restricting the visibility of LocalCameraLayoutSolution to internal.
temcguir
marked this pull request as draft
September 24, 2026 22:25
…anchor and full-bleed topStart clamping
temcguir
marked this pull request as ready for review
October 12, 2026 20:30
Kimblebee
requested changes
Oct 17, 2026
- Report requiresImmersive from PreviewLayout through a new onRequiresImmersiveChange callback and apply it in the app-level SystemBarsPolicy (HideStatusAndNavigationBars) so that a single owner controls navigation bar visibility. Remove ImmersiveNavigationBarEffect, which focus changes could override. - Re-assert system bars on window focus changes through LocalWindowInfo.isWindowFocused instead of a ViewTreeObserver listener. - Align CameraSystemBarsEffect with the Dialog-aware implementation so it can be shared with Dialog-hosted capture surfaces. - Let mode switcher content taller than its band, such as a 50dp carousel, extend into the adjacent gaps without moving any row or overlapping the neighboring rows. - Remove TopStartPlacement from PreviewLayout; topStartContent always stays directly below the top bar. - Remove the unreachable portrait branch from VerticalMaterialControls.
Mirror the OverlapAwareStyleProvider wrapping that main added to VerticalMaterialControls so the image well, flip camera button and capture mode toggle use the reskinned backgrounds on portrait handhelds too.
Add viewfinderTopEndContent, anchored to the top-end corner of the viewfinder. While topStartContent is inside the viewfinder, it shares the same vertical origin below the top bar, so both top slots stay aligned. When the viewfinder shifts below topStartContent (for example 1:1), it follows the top edge of the viewfinder while topStartContent stays fixed. While the two slots share rows, its width is limited to the space beside topStartContent so they never overlap. PreviewDisplay fills its slot and offsets its own frame, so it now reports the frame's top edge through the ViewfinderTopLine alignment line, which PreviewLayout reads to place viewfinder-relative content.
Kimblebee
approved these changes
Oct 23, 2026
… into temcguir/adaptive-capture-layout # Conflicts: # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/layout/CameraLayoutTypes.kt
- Remove an unnecessary non-null assertion in PreviewLayout's top slot measurement. - Use the v2 createComposeRule in CaptureLayoutPositionTest.
… into temcguir/adaptive-capture-layout # Conflicts: # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/ui/QuickSettingsComponents.kt
… into temcguir/adaptive-capture-layout # Conflicts: # app/src/main/java/com/google/jetpackcamera/ui/SystemBarsPolicy.kt # app/src/test/java/com/google/jetpackcamera/ui/SystemBarsPolicyTest.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CameraSystemBarsEffect.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt # ui/components/capture/src/test/java/com/google/jetpackcamera/ui/components/capture/CameraSystemBarsEffectTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CaptureLayout.kt,CaptureScreenComponents.kt):PreviewLayoutandPreviewDisplaytoCameraLayoutSolveron handheld portrait windows (AdaptivePortraitCaptureLayout), solving once per window/spec change and placing the viewfinder and control slots (TopBar,ZoomBar,CaptureRow,ModeSwitcher,BottomToolbar) at their solved coordinates.topStartContentat a fixed position directly below the top bar across aspect ratios.viewfinderTopEndContent, anchored to the top-end corner of the viewfinder. WhiletopStartContentis inside the viewfinder, both top slots share the same vertical origin below the top bar; when the viewfinder shifts belowtopStartContent(for example 1:1), it follows the viewfinder's top edge. While the two slots share rows, its width is limited to the space besidetopStartContentso they never overlap.PreviewDisplayreports the top edge of its visible frame through theViewfinderTopLinealignment line, so viewfinder-relative content can track a viewfinder that fills its slot and offsets its own frame.VerticalMaterialControlsas a fallback for squarer windows (landscape, foldable inner displays, or short split-screen panes), compressing its spacing on short windows and reserving acaptureModeCarouselslot.PreviewDisplayto read the solved viewfinder band (top,height) fromLocalCameraLayoutSolution.CameraSystemBarsEffect.kt,SystemBarsPolicy.kt,JcaApp.kt):CameraSystemBarsEffectinto:ui:components:capturewith Activity- and Dialog-window support, re-asserting the bars on lifecycle resume and on window focus changes (LocalWindowInfo.isWindowFocused).PreviewLayoutreportssolution.requiresImmersivethroughonRequiresImmersiveChangeinstead of changing the system bars itself. The app-levelSystemBarsPolicyapplies it on the preview route (HideStatusAndNavigationBars), so a single owner controls navigation bar visibility across resume and focus changes.PreviewPortraitDevices.kt,:microbenchmark):@PreviewPortraitDevicesmulti-preview annotation for portrait screen sizes andCameraLayoutSolverBenchmarkmeasuring solver execution time across standard and compact device configurations.Testing
CaptureLayoutPositionTest: Robolectric Compose tests verifying slot placement, viewfinder alignment, fixedtopStartContent,viewfinderTopEndContentplacement (including with a self-positioning viewfinder) and overlap limits, mode switcher content sizing, and theonRequiresImmersiveChangecontract across device profiles and aspect ratios.CameraSystemBarsEffectTest&SystemBarsPolicyTest: system bar behavior, multi-window handling, focus re-assertion with a hidden navigation bar, and the route-to-policy mapping.