Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Layout transition] Invalidate calculated layout if transitioning using the same size range #464

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- [ASTextNode2] Provide compile flag to globally enable new implementation of ASTextNode: ASTEXTNODE_EXPERIMENT_GLOBAL_ENABLE. [Scott Goodson](https://github.com/appleguy) [#396](https://github.com/TextureGroup/Texture/pull/410)
- Add ASCollectionGalleryLayoutDelegate - an async collection layout that makes same-size collections (e.g photo galleries, pagers, etc) fast and lightweight! [Huy Nguyen](https://github.com/nguyenhuy/) [#76](https://github.com/TextureGroup/Texture/pull/76) [#451](https://github.com/TextureGroup/Texture/pull/451)
- Fix an issue that causes infinite layout loop in ASDisplayNode after [#428](https://github.com/TextureGroup/Texture/pull/428) [Huy Nguyen](https://github.com/nguyenhuy) [#455](https://github.com/TextureGroup/Texture/pull/455)
- Fix an issue in layout transition that causes it to unexpectedly use the old layout [Huy Nguyen](https://github.com/nguyenhuy) [#464](https://github.com/TextureGroup/Texture/pull/464)

##2.3.5
- Fix an issue where inserting/deleting sections could lead to inconsistent supplementary element behavior. [Adlai Holler](https://github.com/Adlai-Holler)
Expand Down
40 changes: 29 additions & 11 deletions Source/ASDisplayNode+Layout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ - (CGSize)calculatedSize
- (ASSizeRange)constrainedSizeForCalculatedLayout
{
ASDN::MutexLocker l(__instanceLock__);
return [self _locked_constrainedSizeForCalculatedLayout];
}

- (ASSizeRange)_locked_constrainedSizeForCalculatedLayout
{
if (_pendingDisplayNodeLayout != nullptr) {
return _pendingDisplayNodeLayout->constrainedSize;
}
Expand Down Expand Up @@ -478,6 +483,11 @@ @implementation ASDisplayNode (ASLayoutTransition)
- (BOOL)_isLayoutTransitionInvalid
{
ASDN::MutexLocker l(__instanceLock__);
return [self _locked_isLayoutTransitionValid];
}

- (BOOL)_locked_isLayoutTransitionValid
{
if (ASHierarchyStateIncludesLayoutPending(_hierarchyState)) {
ASLayoutElementContext *context = ASLayoutElementGetCurrentContext();
if (context == nil || _pendingTransitionID != context.transitionID) {
Expand Down Expand Up @@ -510,9 +520,6 @@ - (void)transitionLayoutWithAnimation:(BOOL)animated
measurementCompletion:(void(^)())completion
{
ASDisplayNodeAssertMainThread();

[self setNeedsLayout];

[self transitionLayoutWithSizeRange:[self _locked_constrainedSizeForLayoutPass]
animated:animated
shouldMeasureAsync:shouldMeasureAsync
Expand All @@ -536,17 +543,28 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize
return;
}

// Check if we are a subnode in a layout transition.
// In this case no measurement is needed as we're part of the layout transition.
if ([self _isLayoutTransitionInvalid]) {
return;
}

BOOL shouldInvalidateLayout = NO;
{
ASDN::MutexLocker l(__instanceLock__);
ASDisplayNodeAssert(ASHierarchyStateIncludesLayoutPending(_hierarchyState) == NO, @"Can't start a transition when one of the supernodes is performing one.");

// Check if we are a subnode in a layout transition.
// In this case no measurement is needed as we're part of the layout transition.
if ([self _locked_isLayoutTransitionValid]) {
return;
}

if (ASHierarchyStateIncludesLayoutPending(_hierarchyState)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there be any problems if we call ASHierarchyStateIncludesLayoutPending while holding the lock?

ASDisplayNodeAssert(NO, @"Can't start a transition when one of the supernodes is performing one.");
return;
}

shouldInvalidateLayout = ASSizeRangeEqualToSizeRange([self _locked_constrainedSizeForCalculatedLayout], constrainedSize);
}


if (shouldInvalidateLayout) {
[self setNeedsLayout];
}

// Every new layout transition has a transition id associated to check in subsequent transitions for cancelling
int32_t transitionID = [self _startNewTransition];
as_log_verbose(ASLayoutLog(), "Transition ID is %d", transitionID);
Expand Down