Clean up timing of layout tree flattening/ copying of unflattened tree for Weaver #1157
Conversation
This file contains 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
|
|
mikezucc
changed the title
nguyenhuy
approved these changes
Oct 2, 2018
| @@ -993,12 +993,6 @@ - (void)_pendingLayoutTransitionDidComplete | |||
| _pendingLayoutTransition = nil; | |||
| } | |||
|
|
|||
| - (void)_setCalculatedDisplayNodeLayout:(const ASDisplayNodeLayout &)displayNodeLayout | |||
There was a problem hiding this comment.
For the record, this method is not called anywhere.
| if (! [ASDisplayNode shouldStoreUnflattenedLayouts]) { | ||
| layout = [layout filteredNodeLayoutTree]; | ||
| if ([ASDisplayNode shouldStoreUnflattenedLayouts]) { | ||
| _unflattenedLayout = layout; |
There was a problem hiding this comment.
As discussed offline, there is a risk that the layout here will not be used/applied later on, and Weaver (or any other users of this API) will end up with a wrong unflattened layout. But given it's a debugging feature and should not be used in producation, the risk is small and doesn't warrant the complexity of previous implementation.
There was a problem hiding this comment.
Can you add some comment about this fact?
maicki
approved these changes
Oct 3, 2018
| if (! [ASDisplayNode shouldStoreUnflattenedLayouts]) { | ||
| layout = [layout filteredNodeLayoutTree]; | ||
| if ([ASDisplayNode shouldStoreUnflattenedLayouts]) { | ||
| _unflattenedLayout = layout; |
There was a problem hiding this comment.
Can you add some comment about this fact?
|
I will rebase with the correct author, no worry |
mikezucc
added a commit
to mikezucc/Texture
that referenced
this pull request
Nov 7, 2018
…e for Weaver (TextureGroup#1157) * Simpler Huy fix for more efficient delayed flattening of the layout tree * Nit * Remove pbx changes * Update CHANGELOG.md * Add note about change in timing of _flattenedLayout capture
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.
Before display, a node's layout tree is flattened in order to calculate display information such as position. In almost all cases, this happens early in the layout transition lifecycle (during
calculateLayoutThatFits:). However, there are special cases where this layout tree is not flattened until the very last opportunity so that debugging tools (Weaver) can inspect the layout tree.With the move to use const references instead of shared_ptr's for the pending layout tree used in the layout transition, the flattened layout tree was accessible to changes in the pending stage of the transition, causing layout specs to sneak into the final step of the layout transition: moving subnodes.
Huy simplified this diff to just a few lines.