Pass checkEdges to avoid recomposition/layout shift in hot code paths#365
Open
dfabulich wants to merge 2 commits intoskiptools:mainfrom
Open
Pass checkEdges to avoid recomposition/layout shift in hot code paths#365dfabulich wants to merge 2 commits intoskiptools:mainfrom
checkEdges to avoid recomposition/layout shift in hot code paths#365dfabulich wants to merge 2 commits intoskiptools:mainfrom
Conversation
This seems to be a common area for recompositions that cause layout shifts. I think the log noise will probably pay off.
IgnoresSafeAreaLayout doesn't expand into its `expandInto` edges on first render unless the caller specifically opts into that behavior by passing the edges with `checkEdges`. Instead, it will eventually get called back in `onGloballyPositionedInWindow`, at which point we know for certain which edges we should expand into. If we don't check the correct edges on first render, the UI may display incorrect layout temporarily and then shift to the correct position (or, at the very least, do an unnecessary recomposition). In my experiments with the Showcase and my own app, `NavigationStack` and `TabView` basically always do expand into their edges; similarly, the `.ignoresSafeArea()` modifier edges are very very likely to be expanded into. Passing the `expandInto` edges in `checkEdges` saves a recomposition (and often a layout shift) in very hot code paths.
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.
IgnoresSafeAreaLayoutdoesn't expand into itsexpandIntoedges on first render unless the caller specifically opts into that behavior by passing the edges withcheckEdges. Instead, it will eventually get called back inonGloballyPositionedInWindow, at which point we know for certain which edges we should expand into.If we don't check the correct edges on first render, the UI may display incorrect layout temporarily and then shift to the correct position (or, at the very least, do an unnecessary recomposition).
In my experiments with the Showcase and my own app,
NavigationStackandTabViewbasically always do expand into their edges; similarly, the.ignoresSafeArea()modifier edges are very very likely to be expanded into.Passing the
expandIntoedges incheckEdgessaves a recomposition (and often a layout shift) in very hot code paths.I've also added a
logTagparameter toIgnoresSafeAreaLayout, and added debug logging, so we can see when/where recompositions happen. (This seems to be a common area for recompositions that cause layout shifts. I think the log noise will probably pay off.)Skip Pull Request Checklist:
swift testI used Cursor to add the
logTagparameter on all of the ISAL callers. Then, I manually tested the Showcase in the following playgrounds, keeping a log of SkipUI.ISAL log entries:Then, I added my changes and redid the testing. I found that my changes reduced recompositions and reduced layout shift in the Showcase app.