From 04d93532bc68356ed5071709d4bf4a7d753ac923 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 30 Nov 2015 17:20:32 -0800 Subject: [PATCH 01/10] Only require node sizes once per run loop, and only if a node's size has changed --- AsyncDisplayKit/ASCellNode.h | 5 +++-- AsyncDisplayKit/ASCellNode.m | 5 +++-- AsyncDisplayKit/ASCollectionView.mm | 21 +++++++++++++++++++-- AsyncDisplayKit/ASTableView.mm | 21 +++++++++++++++++++-- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/ASCellNode.h b/AsyncDisplayKit/ASCellNode.h index 16fc60d1f3..6b038ee853 100644 --- a/AsyncDisplayKit/ASCellNode.h +++ b/AsyncDisplayKit/ASCellNode.h @@ -15,12 +15,13 @@ typedef NSUInteger ASCellNodeAnimation; @protocol ASCellNodeLayoutDelegate /** - * Notifies the delegate that the specified cell node has done a relayout. + * Notifies the delegate that the specified cell node has done a relayout + * that resulted in a change of `calculatedSize`. * The notification is done on main thread. * * @param node A node informing the delegate about the relayout. */ -- (void)nodeDidRelayout:(ASCellNode *)node; +- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node; @end /** diff --git a/AsyncDisplayKit/ASCellNode.m b/AsyncDisplayKit/ASCellNode.m index ef6652b700..029de4b8a4 100644 --- a/AsyncDisplayKit/ASCellNode.m +++ b/AsyncDisplayKit/ASCellNode.m @@ -53,11 +53,12 @@ - (void)setNeedsLayout { ASDisplayNodeAssertThreadAffinity(self); + CGSize oldSize = self.calculatedSize; [super setNeedsLayout]; - if (_layoutDelegate != nil) { + if (_layoutDelegate != nil && !CGSizeEqualToSize(oldSize, self.calculatedSize)) { ASPerformBlockOnMainThread(^{ - [_layoutDelegate nodeDidRelayout:self]; + [_layoutDelegate nodeDidRelayoutWithSizeChange:self]; }); } } diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index a139bdecec..0c1f7e9961 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -155,6 +155,7 @@ static BOOL _isInterceptedSelector(SEL sel) BOOL _asyncDelegateImplementsInsetSection; BOOL _collectionViewLayoutImplementsInsetSection; BOOL _asyncDataSourceImplementsConstrainedSizeForNode; + BOOL _queuedNodeSizeUpdate; ASBatchContext *_batchContext; @@ -912,10 +913,26 @@ static BOOL _isInterceptedSelector(SEL sel) #pragma mark - ASCellNodeDelegate -- (void)nodeDidRelayout:(ASCellNode *)node +- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node { ASDisplayNodeAssertMainThread(); - // Cause UICollectionView to requery for the new height of this node + + if (_queuedNodeSizeUpdate) { + return; + } + + _queuedNodeSizeUpdate = YES; + [self performSelector:@selector(requeryNodeSizes) + withObject:nil + afterDelay:0 + inModes:@[ NSRunLoopCommonModes ]]; +} + +// Cause UICollectionView to requery for the new size of all nodes +- (void)requeryNodeSizes +{ + _queuedNodeSizeUpdate = NO; + [super performBatchUpdates:^{} completion:nil]; } diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 7354bdcf55..295e592a7e 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -179,6 +179,7 @@ static BOOL _isInterceptedSelector(SEL sel) CGFloat _nodesConstrainedWidth; BOOL _ignoreNodesConstrainedWidthChange; + BOOL _queuedNodeHeightUpdate; } @property (atomic, assign) BOOL asyncDataSourceLocked; @@ -909,10 +910,26 @@ static BOOL _isInterceptedSelector(SEL sel) #pragma mark - ASCellNodeLayoutDelegate -- (void)nodeDidRelayout:(ASCellNode *)node +- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node { ASDisplayNodeAssertMainThread(); - // Cause UITableView to requery for the new height of this node + + if (_queuedNodeHeightUpdate) { + return; + } + + _queuedNodeHeightUpdate = YES; + [self performSelector:@selector(requeryNodeHeights) + withObject:nil + afterDelay:0 + inModes:@[ NSRunLoopCommonModes ]]; +} + +// Cause UITableView to requery for the new height of this node +- (void)requeryNodeHeights +{ + _queuedNodeHeightUpdate = NO; + [super beginUpdates]; [super endUpdates]; } From 0ee1fd82dc51ba587a07a6b4b41382ea124d6171 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 30 Nov 2015 19:29:27 -0800 Subject: [PATCH 02/10] Refactor nodeDidRelayoutWithSizeChange: -> nodeDidRelayout:sizeChanged: --- AsyncDisplayKit/ASCellNode.h | 6 +++--- AsyncDisplayKit/ASCellNode.m | 7 ++++--- AsyncDisplayKit/ASCollectionView.mm | 4 ++-- AsyncDisplayKit/ASTableView.mm | 4 ++-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/AsyncDisplayKit/ASCellNode.h b/AsyncDisplayKit/ASCellNode.h index 6b038ee853..78821ef2d8 100644 --- a/AsyncDisplayKit/ASCellNode.h +++ b/AsyncDisplayKit/ASCellNode.h @@ -15,13 +15,13 @@ typedef NSUInteger ASCellNodeAnimation; @protocol ASCellNodeLayoutDelegate /** - * Notifies the delegate that the specified cell node has done a relayout - * that resulted in a change of `calculatedSize`. + * Notifies the delegate that the specified cell node has done a relayout. * The notification is done on main thread. * * @param node A node informing the delegate about the relayout. + * @param sizeChanged `YES` if the node's `calculatedSize` changed during the relayout, `NO` otherwise. */ -- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node; +- (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged; @end /** diff --git a/AsyncDisplayKit/ASCellNode.m b/AsyncDisplayKit/ASCellNode.m index 029de4b8a4..50ad035702 100644 --- a/AsyncDisplayKit/ASCellNode.m +++ b/AsyncDisplayKit/ASCellNode.m @@ -55,10 +55,11 @@ ASDisplayNodeAssertThreadAffinity(self); CGSize oldSize = self.calculatedSize; [super setNeedsLayout]; - - if (_layoutDelegate != nil && !CGSizeEqualToSize(oldSize, self.calculatedSize)) { + + if (_layoutDelegate != nil) { + BOOL sizeChanged = !CGSizeEqualToSize(oldSize, self.calculatedSize); ASPerformBlockOnMainThread(^{ - [_layoutDelegate nodeDidRelayoutWithSizeChange:self]; + [_layoutDelegate nodeDidRelayout:self sizeChanged:sizeChanged]; }); } } diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 0c1f7e9961..57b37104df 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -913,11 +913,11 @@ static BOOL _isInterceptedSelector(SEL sel) #pragma mark - ASCellNodeDelegate -- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node +- (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged { ASDisplayNodeAssertMainThread(); - if (_queuedNodeSizeUpdate) { + if (!sizeChanged || _queuedNodeSizeUpdate) { return; } diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 295e592a7e..4bd5ef2639 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -910,11 +910,11 @@ static BOOL _isInterceptedSelector(SEL sel) #pragma mark - ASCellNodeLayoutDelegate -- (void)nodeDidRelayoutWithSizeChange:(ASCellNode *)node +- (void)nodeDidRelayout:(ASCellNode *)node sizeChanged:(BOOL)sizeChanged { ASDisplayNodeAssertMainThread(); - if (_queuedNodeHeightUpdate) { + if (!sizeChanged || _queuedNodeHeightUpdate) { return; } From f723452756aa9580683efc61f1fbb14c09457a7d Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Thu, 3 Dec 2015 09:25:37 -0800 Subject: [PATCH 03/10] Allow link taps to continue if touches move within the same link --- AsyncDisplayKit/ASTextNode.mm | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 6c797fbaba..17d9e78b74 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -778,11 +778,7 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation ASDisplayNodeAssertMainThread(); - UITouch *touch = [touches anyObject]; - - UIView *view = touch.view; - CGPoint point = [touch locationInView:view]; - point = [self.view convertPoint:point fromView:view]; + CGPoint point = [[touches anyObject] locationInView:self.view]; NSRange range = NSMakeRange(0, 0); NSString *linkAttributeName = nil; @@ -835,7 +831,16 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation { [super touchesMoved:touches withEvent:event]; - [self _clearHighlightIfNecessary]; + CGPoint point = [[touches anyObject] locationInView:self.view]; + NSRange range = NSMakeRange(0, 0); + [self _linkAttributeValueAtPoint:point + attributeName:NULL + range:&range + inAdditionalTruncationMessage:NULL]; + + if (!NSEqualRanges(_highlightRange, range)) { + [self _clearHighlightIfNecessary]; + } } - (void)_handleLongPress:(UILongPressGestureRecognizer *)longPressRecognizer From f39105a8f492627e47f77416f3d552f53a01bc51 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Thu, 3 Dec 2015 09:45:45 -0800 Subject: [PATCH 04/10] Optimize --- AsyncDisplayKit/ASTextNode.mm | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 17d9e78b74..531cc228d1 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -831,15 +831,18 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation { [super touchesMoved:touches withEvent:event]; - CGPoint point = [[touches anyObject] locationInView:self.view]; - NSRange range = NSMakeRange(0, 0); - [self _linkAttributeValueAtPoint:point - attributeName:NULL - range:&range - inAdditionalTruncationMessage:NULL]; + // If touch has moved out of the current highlight range, clear the highlight. + if (_highlightRange.length > 0) { + NSRange range = NSMakeRange(0, 0); + CGPoint point = [[touches anyObject] locationInView:self.view]; + [self _linkAttributeValueAtPoint:point + attributeName:NULL + range:&range + inAdditionalTruncationMessage:NULL]; - if (!NSEqualRanges(_highlightRange, range)) { - [self _clearHighlightIfNecessary]; + if (!NSEqualRanges(_highlightRange, range)) { + [self _clearHighlightIfNecessary]; + } } } From 835acd6a25cd474cc791c6793fcaa33304c01388 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Thu, 3 Dec 2015 20:52:02 -0800 Subject: [PATCH 05/10] Change assumed value for text node highlighting delegate method to YES and don't consult it unless highlighting. --- AsyncDisplayKit/ASTextNode.h | 2 +- AsyncDisplayKit/ASTextNode.mm | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/AsyncDisplayKit/ASTextNode.h b/AsyncDisplayKit/ASTextNode.h index 130cc16b22..db66b09e13 100644 --- a/AsyncDisplayKit/ASTextNode.h +++ b/AsyncDisplayKit/ASTextNode.h @@ -245,7 +245,7 @@ typedef NS_ENUM(NSUInteger, ASTextNodeHighlightStyle) { @param attribute The attribute that was tapped. Will not be nil. @param value The value of the tapped attribute. @param point The point within textNode, in textNode's coordinate system, that was touched to trigger a highlight. - @discussion If not implemented, the default value is NO. + @discussion If not implemented, the default value is YES. @return YES if the entity attribute should be a link, NO otherwise. */ - (BOOL)textNode:(ASTextNode *)textNode shouldHighlightLinkAttribute:(NSString *)attribute value:(id)value atPoint:(CGPoint)point; diff --git a/AsyncDisplayKit/ASTextNode.mm b/AsyncDisplayKit/ASTextNode.mm index 531cc228d1..f5d3b0381c 100644 --- a/AsyncDisplayKit/ASTextNode.mm +++ b/AsyncDisplayKit/ASTextNode.mm @@ -394,13 +394,15 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation return [self _linkAttributeValueAtPoint:point attributeName:attributeNameOut range:rangeOut - inAdditionalTruncationMessage:NULL]; + inAdditionalTruncationMessage:NULL + forHighlighting:NO]; } - (id)_linkAttributeValueAtPoint:(CGPoint)point attributeName:(out NSString **)attributeNameOut range:(out NSRange *)rangeOut inAdditionalTruncationMessage:(out BOOL *)inAdditionalTruncationMessageOut + forHighlighting:(BOOL)highlighting { ASTextKitRenderer *renderer = [self _renderer]; NSRange visibleRange = renderer.visibleRanges[0]; @@ -453,10 +455,10 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation continue; } - // Check if delegate implements optional method, if not assume NO. - // Should the text be highlightable/touchable? - if (![_delegate respondsToSelector:@selector(textNode:shouldHighlightLinkAttribute:value:atPoint:)] || - ![_delegate textNode:self shouldHighlightLinkAttribute:name value:value atPoint:point]) { + // If highlighting, check with delegate first. If not implemented, assume YES. + if (highlighting + && [_delegate respondsToSelector:@selector(textNode:shouldHighlightLinkAttribute:value:atPoint:)] + && ![_delegate textNode:self shouldHighlightLinkAttribute:name value:value atPoint:point]) { value = nil; name = nil; } @@ -758,7 +760,8 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation id linkAttributeValue = [self _linkAttributeValueAtPoint:point attributeName:&linkAttributeName range:&range - inAdditionalTruncationMessage:&inAdditionalTruncationMessage]; + inAdditionalTruncationMessage:&inAdditionalTruncationMessage + forHighlighting:YES]; NSUInteger lastCharIndex = NSIntegerMax; BOOL linkCrossesVisibleRange = (lastCharIndex > range.location) && (lastCharIndex < NSMaxRange(range) - 1); @@ -787,7 +790,8 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation id linkAttributeValue = [self _linkAttributeValueAtPoint:point attributeName:&linkAttributeName range:&range - inAdditionalTruncationMessage:&inAdditionalTruncationMessage]; + inAdditionalTruncationMessage:&inAdditionalTruncationMessage + forHighlighting:YES]; NSUInteger lastCharIndex = NSIntegerMax; BOOL linkCrossesVisibleRange = (lastCharIndex > range.location) && (lastCharIndex < NSMaxRange(range) - 1); @@ -838,7 +842,8 @@ static NSString *ASTextNodeTruncationTokenAttributeName = @"ASTextNodeTruncation [self _linkAttributeValueAtPoint:point attributeName:NULL range:&range - inAdditionalTruncationMessage:NULL]; + inAdditionalTruncationMessage:NULL + forHighlighting:YES]; if (!NSEqualRanges(_highlightRange, range)) { [self _clearHighlightIfNecessary]; From 05ff0e40ec132db508acee4503d715945357f150 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 4 Dec 2015 12:46:34 -0800 Subject: [PATCH 06/10] Add test case for non-cell node interface state --- AsyncDisplayKitTests/ASDisplayNodeTests.m | 59 +++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index 44d63392e6..8e39f43565 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -15,6 +15,7 @@ #import "ASDisplayNode+Subclasses.h" #import "ASDisplayNodeTestsHelper.h" #import "UIView+ASConvenience.h" +#import "ASCellNode.h" // Conveniences for making nodes named a certain way #define DeclareNodeNamed(n) ASDisplayNode *n = [[ASDisplayNode alloc] init]; n.name = @#n @@ -76,11 +77,15 @@ for (ASDisplayNode *n in @[ nodes ]) {\ + (dispatch_queue_t)asyncSizingQueue; - (id)initWithViewClass:(Class)viewClass; - (id)initWithLayerClass:(Class)layerClass; + +// FIXME: Importing ASDisplayNodeInternal.h causes a heap of problems. +- (void)enterInterfaceState:(ASInterfaceState)interfaceState; @end @interface ASTestDisplayNode : ASDisplayNode @property (atomic, copy) void (^willDeallocBlock)(ASTestDisplayNode *node); @property (atomic, copy) CGSize(^calculateSizeBlock)(ASTestDisplayNode *node, CGSize size); +@property (atomic) BOOL hasFetchedData; @end @interface ASTestResponderNode : ASTestDisplayNode @@ -93,6 +98,18 @@ for (ASDisplayNode *n in @[ nodes ]) {\ return _calculateSizeBlock ? _calculateSizeBlock(self, constrainedSize) : CGSizeZero; } +- (void)fetchData +{ + [super fetchData]; + self.hasFetchedData = YES; +} + +- (void)clearFetchedData +{ + [super clearFetchedData]; + self.hasFetchedData = NO; +} + - (void)dealloc { if (_willDeallocBlock) { @@ -1663,6 +1680,48 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point [self checkBackgroundColorOpaqueRelationshipWithViewLoaded:NO layerBacked:YES]; } +// Check that nodes who have no cell node (no range controller) +// do get their `fetchData` called, and they do report +// the fetch data interface state. +- (void)testInterfaceStateForNonCellNode +{ + ASTestWindow *window = [ASTestWindow new]; + ASTestDisplayNode *node = [ASTestDisplayNode new]; + XCTAssert(node.interfaceState == ASInterfaceStateNone); + XCTAssert(!node.hasFetchedData); + + [window addSubview:node.view]; + XCTAssert(node.hasFetchedData); + XCTAssert(node.interfaceState == ASInterfaceStateInHierarchy); + + [node.view removeFromSuperview]; + XCTAssert(node.hasFetchedData); + XCTAssert(node.interfaceState == ASInterfaceStateNone); +} + +// Check that nodes who have no cell node (no range controller) +// do get their `fetchData` called, and they do report +// the fetch data interface state. +- (void)testInterfaceStateForCellNode +{ + ASCellNode *cellNode = [ASCellNode new]; + ASTestDisplayNode *node = [ASTestDisplayNode new]; + XCTAssert(node.interfaceState == ASInterfaceStateNone); + XCTAssert(!node.hasFetchedData); + + // Simulate range handler updating cell node. + [cellNode addSubnode:node]; + [cellNode enterInterfaceState:ASInterfaceStateFetchData]; + XCTAssert(node.hasFetchedData); + XCTAssert(node.interfaceState == ASInterfaceStateFetchData); + + // If the node goes into a view it should not adopt the `InHierarchy` state. + ASTestWindow *window = [ASTestWindow new]; + [window addSubview:cellNode.view]; + XCTAssert(node.hasFetchedData); + XCTAssert(node.interfaceState == ASInterfaceStateFetchData); +} + - (void)testInitWithViewClass { ASDisplayNode *scrollNode = [[ASDisplayNode alloc] initWithViewClass:[UIScrollView class]]; From 9579420cd78cc452446adfa25dd183f37a4412a1 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 4 Dec 2015 12:47:36 -0800 Subject: [PATCH 07/10] Add static constant ASInterfaceStateInHierarchy which we'll return if the node is not in a cell but its view is in a window --- AsyncDisplayKit/ASDisplayNode+Subclasses.h | 12 ++++++---- AsyncDisplayKit/ASDisplayNode.h | 24 +++++++++++++++---- AsyncDisplayKit/ASDisplayNode.mm | 28 +++++++++++++++++++++- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode+Subclasses.h b/AsyncDisplayKit/ASDisplayNode+Subclasses.h index 6d853721ca..3af5c45254 100644 --- a/AsyncDisplayKit/ASDisplayNode+Subclasses.h +++ b/AsyncDisplayKit/ASDisplayNode+Subclasses.h @@ -366,7 +366,7 @@ /** - * Called just before the view is added to a superview. + * Called just before the view is added to a window. */ - (void)willEnterHierarchy ASDISPLAYNODE_REQUIRES_SUPER; @@ -426,9 +426,13 @@ @end @interface ASDisplayNode (ASDisplayNodePrivate) -// This method has proven helpful in a few rare scenarios, similar to a category extension on UIView, -// but it's considered private API for now and its use should not be encouraged. -- (ASDisplayNode *)_supernodeWithClass:(Class)supernodeClass; +/** + * This method has proven helpful in a few rare scenarios, similar to a category extension on UIView, + * but it's considered private API for now and its use should not be encouraged. + * @param checkViewHierarchy If YES, and no supernode can be found, method will walk up from `self.view` to find a supernode. + * If YES, this method must be called on the main thread and the node must not be layer-backed. + */ +- (ASDisplayNode *)_supernodeWithClass:(Class)supernodeClass checkViewHierarchy:(BOOL)checkViewHierarchy; // The two methods below will eventually be exposed, but their names are subject to change. /** diff --git a/AsyncDisplayKit/ASDisplayNode.h b/AsyncDisplayKit/ASDisplayNode.h index 80ee51aa6f..b10db67a5c 100644 --- a/AsyncDisplayKit/ASDisplayNode.h +++ b/AsyncDisplayKit/ASDisplayNode.h @@ -41,18 +41,32 @@ typedef void (^ASDisplayNodeDidLoadBlock)(ASDisplayNode *node); typedef NS_OPTIONS(NSUInteger, ASInterfaceState) { /** The element is not predicted to be onscreen soon and preloading should not be performed */ - ASInterfaceStateNone = 1 << 0, + ASInterfaceStateNone = 0, /** The element may be added to a view soon that could become visible. Measure the layout, including size calculation. */ - ASInterfaceStateMeasureLayout = 1 << 1, + ASInterfaceStateMeasureLayout = 1 << 0, /** The element is likely enough to come onscreen that disk and/or network data required for display should be fetched. */ - ASInterfaceStateFetchData = 1 << 2, + ASInterfaceStateFetchData = 1 << 1, /** The element is very likely to become visible, and concurrent rendering should be executed for any -setNeedsDisplay. */ - ASInterfaceStateDisplay = 1 << 3, + ASInterfaceStateDisplay = 1 << 2, /** The element is physically onscreen by at least 1 pixel. In practice, all other bit fields should also be set when this flag is set. */ - ASInterfaceStateVisible = 1 << 4, + ASInterfaceStateVisible = 1 << 3, }; +/** + * Currently we only set `interfaceState` for + * nodes contained in table views or collection views. + + * Nodes that aren't contained in cells will be in this state when + * they are in the view hierarchy, and `ASInterfaceStateNone` when + * they aren't. + */ +static const ASInterfaceState ASInterfaceStateInHierarchy = + ASInterfaceStateMeasureLayout + | ASInterfaceStateFetchData + | ASInterfaceStateDisplay + | ASInterfaceStateVisible; + /** * An `ASDisplayNode` is an abstraction over `UIView` and `CALayer` that allows you to perform calculations about a view * hierarchy off the main thread, and could do rendering off the main thread as well. diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 9c52f08c21..cc88b754ba 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -24,6 +24,7 @@ #import "ASInternalHelpers.h" #import "ASLayout.h" #import "ASLayoutSpec.h" +#import "ASCellNode.h" @interface ASDisplayNode () @@ -1581,6 +1582,10 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(_flags.isEnteringHierarchy, @"You should never call -willEnterHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isExitingHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); + + if (![self supportsInterfaceState]) { + self.interfaceState = ASInterfaceStateInHierarchy; + } } - (void)didExitHierarchy @@ -1588,6 +1593,10 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) ASDisplayNodeAssertMainThread(); ASDisplayNodeAssert(_flags.isExitingHierarchy, @"You should never call -didExitHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); + + if (![self supportsInterfaceState]) { + self.interfaceState = ASInterfaceStateNone; + } } - (void)clearContents @@ -1635,6 +1644,20 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) [self clearFetchedData]; } +/** + * We currently only set interface state on nodes + * in table/collection views. For other nodes, if they are + * in the hierarchy we return `Unknown`, otherwise we return `None`. + * + * TODO: Avoid traversing up node hierarchy due to possible deadlock. + * @see https://github.com/facebook/AsyncDisplayKit/issues/900 + * Possible solution is to push `isInCellNode` state downward on `addSubnode`/`removeFromSupernode`. + */ +- (BOOL)supportsInterfaceState { + return ([self isKindOfClass:ASCellNode.class] + || [self _supernodeWithClass:ASCellNode.class checkViewHierarchy:NO] != nil); +} + - (ASInterfaceState)interfaceState { ASDN::MutexLocker l(_propertyLock); @@ -1871,7 +1894,7 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) // This method has proved helpful in a few rare scenarios, similar to a category extension on UIView, but assumes knowledge of _ASDisplayView. // It's considered private API for now and its use should not be encouraged. -- (ASDisplayNode *)_supernodeWithClass:(Class)supernodeClass +- (ASDisplayNode *)_supernodeWithClass:(Class)supernodeClass checkViewHierarchy:(BOOL)checkViewHierarchy { ASDisplayNode *supernode = self.supernode; while (supernode) { @@ -1879,6 +1902,9 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) return supernode; supernode = supernode.supernode; } + if (!checkViewHierarchy) { + return nil; + } UIView *view = self.view.superview; while (view) { From 0bfb5a040111d75eeb28cf3143abe80d9294b26f Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 4 Dec 2015 12:49:38 -0800 Subject: [PATCH 08/10] Make new interface state a first-class value --- AsyncDisplayKit/ASDisplayNode.h | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.h b/AsyncDisplayKit/ASDisplayNode.h index b10db67a5c..23eb84a6b5 100644 --- a/AsyncDisplayKit/ASDisplayNode.h +++ b/AsyncDisplayKit/ASDisplayNode.h @@ -51,21 +51,17 @@ typedef NS_OPTIONS(NSUInteger, ASInterfaceState) /** The element is physically onscreen by at least 1 pixel. In practice, all other bit fields should also be set when this flag is set. */ ASInterfaceStateVisible = 1 << 3, + + /** + * The node is not contained in a cell but it is in a window. + * + * Currently we only set `interfaceState` to other values for + * nodes contained in table views or collection views. + */ + ASInterfaceStateInHierarchy = ASInterfaceStateMeasureLayout | ASInterfaceStateFetchData | ASInterfaceStateDisplay | ASInterfaceStateVisible, }; -/** - * Currently we only set `interfaceState` for - * nodes contained in table views or collection views. - - * Nodes that aren't contained in cells will be in this state when - * they are in the view hierarchy, and `ASInterfaceStateNone` when - * they aren't. - */ -static const ASInterfaceState ASInterfaceStateInHierarchy = - ASInterfaceStateMeasureLayout - | ASInterfaceStateFetchData - | ASInterfaceStateDisplay - | ASInterfaceStateVisible; + /** * An `ASDisplayNode` is an abstraction over `UIView` and `CALayer` that allows you to perform calculations about a view From 34c487de5b6302397230f5e081d9c13fd4a7133e Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 4 Dec 2015 13:01:15 -0800 Subject: [PATCH 09/10] Fix test --- AsyncDisplayKitTests/ASDisplayNodeTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index 8e39f43565..ae37bc8765 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -1695,7 +1695,7 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssert(node.interfaceState == ASInterfaceStateInHierarchy); [node.view removeFromSuperview]; - XCTAssert(node.hasFetchedData); + XCTAssert(!node.hasFetchedData); XCTAssert(node.interfaceState == ASInterfaceStateNone); } From 4284b6e23acd3bdfd3d7b6005376a1747dee7f1f Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sat, 5 Dec 2015 00:20:44 -0800 Subject: [PATCH 10/10] Set _interfaceState before calling fetchData et al. --- AsyncDisplayKit/ASDisplayNode.mm | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 9c52f08c21..a9172f7c04 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -1643,14 +1643,20 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) - (void)setInterfaceState:(ASInterfaceState)interfaceState { - ASDN::MutexLocker l(_propertyLock); - if (interfaceState != _interfaceState) { - if ((interfaceState & ASInterfaceStateMeasureLayout) != (_interfaceState & ASInterfaceStateMeasureLayout)) { + ASInterfaceState oldValue; + { + ASDN::MutexLocker l(_propertyLock); + oldValue = _interfaceState; + _interfaceState = interfaceState; + } + + if (interfaceState != oldValue) { + if ((interfaceState & ASInterfaceStateMeasureLayout) != (oldValue & ASInterfaceStateMeasureLayout)) { // Trigger asynchronous measurement if it is not already cached or being calculated. } // Entered or exited data loading state. - if ((interfaceState & ASInterfaceStateFetchData) != (_interfaceState & ASInterfaceStateFetchData)) { + if ((interfaceState & ASInterfaceStateFetchData) != (oldValue & ASInterfaceStateFetchData)) { if (interfaceState & ASInterfaceStateFetchData) { [self fetchData]; } else { @@ -1659,7 +1665,7 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) } // Entered or exited contents rendering state. - if ((interfaceState & ASInterfaceStateDisplay) != (_interfaceState & ASInterfaceStateDisplay)) { + if ((interfaceState & ASInterfaceStateDisplay) != (oldValue & ASInterfaceStateDisplay)) { if (interfaceState & ASInterfaceStateDisplay) { // Once the working window is eliminated (ASRangeHandlerRender), trigger display directly here. [self setDisplaySuspended:NO]; @@ -1670,15 +1676,14 @@ void recursivelyEnsureDisplayForLayer(CALayer *layer) } // Entered or exited data loading state. - if ((interfaceState & ASInterfaceStateVisible) != (_interfaceState & ASInterfaceStateVisible)) { + if ((interfaceState & ASInterfaceStateVisible) != (oldValue & ASInterfaceStateVisible)) { if (interfaceState & ASInterfaceStateVisible) { // Consider providing a -didBecomeVisible. } else { // Consider providing a -didBecomeInvisible. } } - - _interfaceState = interfaceState; + } }