From fb92b448e06979c4121256de68824f5b2455b48e Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 28 Oct 2016 15:39:03 -0700 Subject: [PATCH] [ASDisplayNode] Proper handling of `constrainedSize` (#2505) * Check in ASLayout if size is valid for sizing instead of valid for layout * Return constrainedSize from calculateSizeThatFits: Remove invalid constrainedSize check within ASNetworkImageNode. Furthermore as ASDisplayNode does not return CGSizeZero anymore we have to give the display nodes we use in tests and are involving a stack spec an intrinsic content size. * Remove extra constrainedSize handling in ASNetworkImageNode handling * Change test to use FLT_MAX --- AsyncDisplayKit/ASDisplayNode.mm | 2 +- AsyncDisplayKit/ASImageNode.mm | 2 +- AsyncDisplayKit/ASNetworkImageNode.mm | 17 ----------------- AsyncDisplayKit/Layout/ASLayout.mm | 2 +- .../ASDisplayNodeImplicitHierarchyTests.m | 17 +++++++++++++++++ AsyncDisplayKitTests/ASDisplayNodeTests.m | 17 +++++++++++++++++ 6 files changed, 37 insertions(+), 20 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 7b18bc5fd5..a20bbaa9d9 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -2491,7 +2491,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) { __ASDisplayNodeCheckForLayoutMethodOverrides; - return CGSizeZero; + return constrainedSize; } - (id)_layoutElementThatFits:(ASSizeRange)constrainedSize diff --git a/AsyncDisplayKit/ASImageNode.mm b/AsyncDisplayKit/ASImageNode.mm index 1aa0c22587..bdc2180db1 100644 --- a/AsyncDisplayKit/ASImageNode.mm +++ b/AsyncDisplayKit/ASImageNode.mm @@ -190,7 +190,7 @@ struct ASImageNodeDrawParameters { ASDN::MutexLocker l(__instanceLock__); if (_image == nil) { - return constrainedSize; + return [super calculateSizeThatFits:constrainedSize]; } return _image.size; diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 73ccbebc03..8a989c5c8d 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -331,23 +331,6 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; } } -#pragma mark - Layout and Sizing - -- (CGSize)calculateSizeThatFits:(CGSize)constrainedSize -{ - ASDN::MutexLocker l(__instanceLock__); - - // If the image node is currently in the loading process and no valid size is applied return CGSizeZero for the time - // being. - // TODO: After 2.0 is stable we should remove this behavior as a ASNetworkImageNode is a replaced element and the - // client code should set the size of an image or it's container it's embedded in - if (ASIsCGSizeValidForSize(constrainedSize) == NO && _URL != nil && self.image == nil) { - return CGSizeZero; - } - - return [super calculateSizeThatFits:constrainedSize]; -} - #pragma mark - Private methods, safe to call without lock - (void)handleProgressImage:(UIImage *)progressImage progress:(CGFloat)progress downloadIdentifier:(nullable id)downloadIdentifier diff --git a/AsyncDisplayKit/Layout/ASLayout.mm b/AsyncDisplayKit/Layout/ASLayout.mm index b2f992fe11..b9967f6199 100644 --- a/AsyncDisplayKit/Layout/ASLayout.mm +++ b/AsyncDisplayKit/Layout/ASLayout.mm @@ -72,7 +72,7 @@ static inline NSString * descriptionIndents(NSUInteger indents) // Read this now to avoid @c weak overhead later. _layoutElementType = layoutElement.layoutElementType; - if (!ASIsCGSizeValidForLayout(size)) { + if (!ASIsCGSizeValidForSize(size)) { ASDisplayNodeAssert(NO, @"layoutSize is invalid and unsafe to provide to Core Animation! Release configurations will force to 0, 0. Size = %@, node = %@", NSStringFromCGSize(size), layoutElement); size = CGSizeZero; } else { diff --git a/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m b/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m index 51f8c26ee6..6eac4cd6c0 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m @@ -59,11 +59,21 @@ - (void)testInitialNodeInsertionWithOrdering { + static CGSize kSize = {100, 100}; + ASDisplayNode *node1 = [[ASDisplayNode alloc] init]; ASDisplayNode *node2 = [[ASDisplayNode alloc] init]; ASDisplayNode *node3 = [[ASDisplayNode alloc] init]; ASDisplayNode *node4 = [[ASDisplayNode alloc] init]; ASDisplayNode *node5 = [[ASDisplayNode alloc] init]; + + + // As we will involve a stack spec we have to give the nodes an intrinsic content size + node1.style.preferredSize = kSize; + node2.style.preferredSize = kSize; + node3.style.preferredSize = kSize; + node4.style.preferredSize = kSize; + node5.style.preferredSize = kSize; ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; node.automaticallyManagesSubnodes = YES; @@ -88,10 +98,17 @@ - (void)testCalculatedLayoutHierarchyTransitions { + static CGSize kSize = {100, 100}; + ASDisplayNode *node1 = [[ASDisplayNode alloc] init]; ASDisplayNode *node2 = [[ASDisplayNode alloc] init]; ASDisplayNode *node3 = [[ASDisplayNode alloc] init]; + // As we will involve a stack spec we have to give the nodes an intrinsic content size + node1.style.preferredSize = kSize; + node2.style.preferredSize = kSize; + node3.style.preferredSize = kSize; + ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; node.automaticallyManagesSubnodes = YES; node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize){ diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index 47fed66bba..a74f324b56 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -2151,4 +2151,21 @@ static bool stringContainsPointer(NSString *description, id p) { XCTAssertThrowsSpecificNamed([node calculateLayoutThatFits:ASSizeRangeMake(CGSizeMake(100, 100))], NSException, NSInternalInconsistencyException); } +- (void)testThatLayoutWithInvalidSizeCausesException +{ + ASDisplayNode *displayNode = [[ASDisplayNode alloc] init]; + ASDisplayNode *node = [[ASDisplayNode alloc] init]; + node.layoutSpecBlock = ^ASLayoutSpec *(ASDisplayNode *node, ASSizeRange constrainedSize) { + return [ASWrapperLayoutSpec wrapperWithLayoutElement:displayNode]; + }; + + XCTAssertThrows([node layoutThatFits:ASSizeRangeMake(CGSizeMake(0, FLT_MAX))]); + + // This dance is necessary as we would assert in case we create an ASDimension that is not real numbers + ASDimension width = displayNode.style.width; + width.value = INFINITY; + displayNode.style.width = width; + XCTAssertThrows([node layoutThatFits:ASSizeRangeMake(CGSizeMake(0, 0), CGSizeMake(INFINITY, INFINITY))]); +} + @end