Refactor PreviewScreen to use lambdas for deferred state reading#529
Refactor PreviewScreen to use lambdas for deferred state reading#529temcguir wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors PreviewScreen.kt to optimize recompositions by introducing deferred state providers and wrapping slot lambdas in remember blocks. The review feedback highlights a critical potential NullPointerException in Compose Previews due to a non-null assertion on zoomController, a performance issue in elapsedTimeDisplayLambda causing excessive recompositions, and outdated KDoc documentation for ElapsedTimeText.
|
@gemini-code-assist The suggested fixes for the NullPointerException in the ZoomStateManager preview fallback, the deferred state read leak in the elapsedTimeDisplayLambda, and the updated KDoc for ElapsedTimeText have all been implemented. PTAL! |
There was a problem hiding this comment.
Code Review
This pull request refactors PreviewScreen and its sub-composables to use deferred state reads via lambda providers (e.g., captureUiStateProvider), isolating recompositions and improving performance. Feedback on these changes highlights a potential bug where capturing the provider directly inside derivedStateOf blocks can lead to stale lambda references; using rememberUpdatedState is recommended to resolve this. Additionally, the reviewer noted a need to update the KDoc parameter name in CaptureScreenComponents.kt and to add missing KDoc documentation for the FlashModeIndicator composable in QuickSettingsComponents.kt to comply with the repository's style guide.
1b43b88 to
2a3ee6a
Compare
This PR refactors
PreviewScreento use lambda providers (() -> T) instead of passing raw UI state values down the composable tree.It also utilizes
derivedStateOfto tightly scope high-frequency state reads (like elapsed recording time and low-light boost active state) to the specific child composables that need them. This dramatically reduces unnecessary recompositions of the entirePreviewScreenwrapper, significantly improving UI performance during capture.Note: This PR is stacked on top of PR 519 (
temcguir/disable_animations).