From 8b397cd04eb71fe2aa2b3434d234bc1c55576abd Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Mon, 24 Jul 2017 18:00:43 +0000 Subject: [PATCH] [Layout transition] Invalidate calculated layout if transitioning using the same size range (#464) * Invalidate the calculated layout transitioning using the same size range - It's possible that -transitionLayoutWithSizeRange: can be called with the same size range used for previous layout passes. In that case, it's necessary to invalidate the current layout to avoid it being reused later on in the process. * Fix CHANGELOG --- CHANGELOG.md | 1 + Source/ASDisplayNode+Layout.mm | 40 ++++++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b83bea11e..f7267c240c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 6d44b4a157..31f0739c73 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -185,6 +185,11 @@ ASPrimitiveTraitCollectionDeprecatedImplementation - (ASSizeRange)constrainedSizeForCalculatedLayout { ASDN::MutexLocker l(__instanceLock__); + return [self _locked_constrainedSizeForCalculatedLayout]; +} + +- (ASSizeRange)_locked_constrainedSizeForCalculatedLayout +{ if (_pendingDisplayNodeLayout != nullptr) { return _pendingDisplayNodeLayout->constrainedSize; } @@ -478,6 +483,11 @@ ASPrimitiveTraitCollectionDeprecatedImplementation - (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) { @@ -510,9 +520,6 @@ ASPrimitiveTraitCollectionDeprecatedImplementation measurementCompletion:(void(^)())completion { ASDisplayNodeAssertMainThread(); - - [self setNeedsLayout]; - [self transitionLayoutWithSizeRange:[self _locked_constrainedSizeForLayoutPass] animated:animated shouldMeasureAsync:shouldMeasureAsync @@ -536,17 +543,28 @@ ASPrimitiveTraitCollectionDeprecatedImplementation 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)) { + 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);