[ASDisplayNode+Layout] Ensure a pending layout is applied once #695
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
Before: - Even if a pending layout was applied before, it'll be unnecessarily applied again in next layout passes and cause `-calculatedLayoutDidChange` being called multiple times. After: - If a pending layout was applied, the calculated layout will not be ignored but reused, if possible, in next layout passes.
nguyenhuy
changed the title
Adlai-Holler
approved these changes
Nov 30, 2017
Source/ASDisplayNode+Layout.mm
Outdated
| BOOL calculatedLayoutIsReusable = (_calculatedDisplayNodeLayout->version >= _layoutVersion | ||
| && (_calculatedDisplayNodeLayout->requestedLayoutFromAbove == YES | ||
| || CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, boundsSizeForLayout))); | ||
| if (!pendingLayoutIsPreferred && calculatedLayoutIsReusable) { |
There was a problem hiding this comment.
Nit: let's take the opportunity to remove == YES and maybe add a comment explaining the requestedLayoutFromAbove + sizeCheck logic.
nguyenhuy
added a commit
to nguyenhuy/Texture
that referenced
this pull request
Feb 6, 2018
…ut to not be applied - Since the implementation of layout version (TextureGroup#428), if a node's pending and calculated layouts have the same current version as well as the same constrained size, the 2 layouts are considered equal and can be used interchangeably. A layout version check between the 2 layouts was added in TextureGroup#695. This PR adds a missing constrained size check. - If the pending layout has the same version but a different constrained size compare to the calculated layout's, we can assume that the pending layout is newer and should be preferred over the calculated one. That is because layout operations always register their new layout as pending, which then (immediately or eventually) get applied to the node as calculated layout.
nguyenhuy
added a commit
that referenced
this pull request
Feb 12, 2018
…nding layout to not be applied (#792) * [ASDisplayNode layout] Fix an issue that causes a node's pending layout to not be applied - Since the implementation of layout version (#428), if a node's pending and calculated layouts have the same current version as well as the same constrained size, the 2 layouts are considered equal and can be used interchangeably. A layout version check between the 2 layouts was added in #695. This PR adds a missing constrained size check. - If the pending layout has the same version but a different constrained size compare to the calculated layout's, we can assume that the pending layout is newer and should be preferred over the calculated one. That is because layout operations always register their new layout as pending, which then (immediately or eventually) get applied to the node as calculated layout.
bernieperez
pushed a commit
to AtomTickets/Texture
that referenced
this pull request
Apr 25, 2018
…reGroup#695) Before: - Even if a pending layout was applied before, it'll be unnecessarily applied again in next layout passes and cause `-calculatedLayoutDidChange` being called multiple times. After: - If a pending layout was applied, the calculated layout will not be ignored but reused, if possible, in next layout passes. Test plan: testSetNeedsLayoutAndNormalLayoutPass in TextureGroup#424.
bernieperez
pushed a commit
to AtomTickets/Texture
that referenced
this pull request
Apr 25, 2018
…nding layout to not be applied (TextureGroup#792) * [ASDisplayNode layout] Fix an issue that causes a node's pending layout to not be applied - Since the implementation of layout version (TextureGroup#428), if a node's pending and calculated layouts have the same current version as well as the same constrained size, the 2 layouts are considered equal and can be used interchangeably. A layout version check between the 2 layouts was added in TextureGroup#695. This PR adds a missing constrained size check. - If the pending layout has the same version but a different constrained size compare to the calculated layout's, we can assume that the pending layout is newer and should be preferred over the calculated one. That is because layout operations always register their new layout as pending, which then (immediately or eventually) get applied to the node as calculated layout.
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:
-calculatedLayoutDidChangebeing called multiple times.After:
Test plan:
testSetNeedsLayoutAndNormalLayoutPassin #424.