[ASDisplayNode] Ensure ASHierarchyState propagation does not jump discontinuities in the node hierarchy. (#2263)

Scenario: An ASCollectionNode is a subnode of an ASCellNode.  A layout transition is started, resulting in
the removal of the ASCollectionNode as a subnode.  As it is removed, the hierarchy state is cleared - including
the "range managed" bit - on the ASCollectionNode.  However, the deep recursion traverses the layer hierarchy
too, and clears this bit on the ASCellNodes inside the ASCollectionNode.  A moment later, the collection performs
its final ASRangeController update to mark its cells as invisible and free memory.  Then an assertion is triggered
in ASRangeController, because it is operating on nodes that do not have the "range managed" bit set.

It turns out that ASInterfaceState also propogates in this way, but that behavior is efficient and beneficial in
its current configuration (it assists how multi-dimensional preloading works).  However, hierarchy state should
never need to jump discontinuities in the node hierarchy.  For now, disabling that case and will revisit
other use cases soon.
This commit is contained in:
appleguy
2016-09-19 12:04:39 -07:00
committed by GitHub
parent 48f76c40d5
commit 3e5b8c3096
4 changed files with 29 additions and 29 deletions

View File

@@ -873,13 +873,12 @@ ASLayoutableSizeHelperForwarding
int32_t transitionID = [self _startNewTransition];
// Move all subnodes in a pending state
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodeAssert([node _isTransitionInProgress] == NO, @"Can't start a transition when one of the subnodes is performing one.");
node.hierarchyState |= ASHierarchyStateLayoutPending;
node.pendingTransitionID = transitionID;
});
void (^transitionBlock)(void) = ^{
if ([self _shouldAbortTransitionWithID:transitionID]) {
return;
@@ -925,7 +924,7 @@ ASLayoutableSizeHelperForwarding
[self setCalculatedDisplayNodeLayout:pendingLayout];
// Apply complete layout transitions for all subnodes
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) {
[node _completePendingLayoutTransition];
node.hierarchyState &= (~ASHierarchyStateLayoutPending);
});
@@ -969,7 +968,7 @@ ASLayoutableSizeHelperForwarding
[self _finishOrCancelTransition];
// Tell subnodes to exit layout pending state and clear related properties
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) {
node.hierarchyState &= (~ASHierarchyStateLayoutPending);
});
}
@@ -1305,7 +1304,7 @@ ASLayoutableSizeHelperForwarding
// while the newly materialized subtree finishes rendering. Then destroy placeholderImage to save memory.
[self recursivelyClearContents];
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
if (shouldRasterize) {
[node enterHierarchyState:ASHierarchyStateRasterized];
[node __unloadNode];
@@ -1327,7 +1326,7 @@ ASLayoutableSizeHelperForwarding
[self recursivelyDisplayImmediately];
}
} else {
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
if (shouldRasterize) {
[node enterHierarchyState:ASHierarchyStateRasterized];
} else {
@@ -2233,7 +2232,7 @@ static NSInteger incrementIfFound(NSInteger i) {
_pendingTransitionID = pendingTransitionId;
// Propagate down the new pending transition id
ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) {
node.pendingTransitionID = _pendingTransitionID;
});
}
@@ -2687,7 +2686,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)recursivelyClearContents
{
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode * _Nonnull node) {
[node clearContents];
});
}
@@ -2706,7 +2705,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)recursivelyFetchData
{
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode * _Nonnull node) {
[node fetchData];
});
}
@@ -2718,7 +2717,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)recursivelyClearFetchedData
{
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode * _Nonnull node) {
[node clearFetchedData];
});
}
@@ -2905,7 +2904,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
if (interfaceState == ASInterfaceStateNone) {
return; // This method is a no-op with a 0-bitfield argument, so don't bother recursing.
}
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode *node) {
node.interfaceState |= interfaceState;
});
}
@@ -2916,7 +2915,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
return; // This method is a no-op with a 0-bitfield argument, so don't bother recursing.
}
ASDisplayNodeLogEvent(self, @"%@ %@", NSStringFromSelector(_cmd), NSStringFromASInterfaceState(interfaceState));
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode *node) {
node.interfaceState &= (~interfaceState);
});
}
@@ -2927,7 +2926,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
// setInterfaceState: skips this when handling range-managed nodes (our whole subtree has this set).
// If our range manager intends for us to be displayed right now, and didn't before, get started!
BOOL shouldScheduleDisplay = [self supportsRangeManagedInterfaceState] && [self shouldScheduleDisplayWithNewInterfaceState:newInterfaceState];
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode *node) {
node.interfaceState = newInterfaceState;
});
if (shouldScheduleDisplay) {
@@ -2999,7 +2998,8 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
if (hierarchyState == ASHierarchyStateNormal) {
return; // This method is a no-op with a 0-bitfield argument, so don't bother recursing.
}
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, NO, ^(ASDisplayNode *node) {
node.hierarchyState |= hierarchyState;
});
}
@@ -3009,7 +3009,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
if (hierarchyState == ASHierarchyStateNormal) {
return; // This method is a no-op with a 0-bitfield argument, so don't bother recursing.
}
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, NO, ^(ASDisplayNode *node) {
node.hierarchyState &= (~hierarchyState);
});
}
@@ -3076,7 +3076,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
- (void)recursivelySetNeedsDisplayAtScale:(CGFloat)contentsScale
{
ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) {
ASDisplayNodePerformBlockOnEveryNode(nil, self, YES, ^(ASDisplayNode *node) {
[node setNeedsDisplayAtScale:contentsScale];
});
}

View File

@@ -82,16 +82,16 @@ extern ASDisplayNode * _Nullable ASViewToDisplayNode(UIView * _Nullable view);
extern ASDisplayNode *ASDisplayNodeUltimateParentOfNode(ASDisplayNode *node);
/**
This function will walk the layer hierarchy, spanning discontinuous sections of the node hierarchy (e.g. the layers
of UIKit intermediate views in UIViewControllers, UITableView, UICollectionView).
If traverseSublayers == YES, this function will walk the layer hierarchy, spanning discontinuous sections of the node hierarchy\
(e.g. the layers of UIKit intermediate views in UIViewControllers, UITableView, UICollectionView).
In the event that a node's backing layer is not created yet, the function will only walk the direct subnodes instead
of forcing the layer hierarchy to be created.
*/
extern void ASDisplayNodePerformBlockOnEveryNode(CALayer * _Nullable layer, ASDisplayNode * _Nullable node, void(^block)(ASDisplayNode *node));
extern void ASDisplayNodePerformBlockOnEveryNode(CALayer * _Nullable layer, ASDisplayNode * _Nullable node, BOOL traverseSublayers, void(^block)(ASDisplayNode *node));
/**
This function will walk the node hierarchy in a breadth first fashion. It does run the block on the node provided
directly to the function call.
directly to the function call. It does NOT traverse sublayers.
*/
extern void ASDisplayNodePerformBlockOnEveryNodeBFS(ASDisplayNode *node, void(^block)(ASDisplayNode *node));
@@ -99,7 +99,7 @@ extern void ASDisplayNodePerformBlockOnEveryNodeBFS(ASDisplayNode *node, void(^b
Identical to ASDisplayNodePerformBlockOnEveryNode, except it does not run the block on the
node provided directly to the function call - only on all descendants.
*/
extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, void(^block)(ASDisplayNode *node));
extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL traverseSublayers, void(^block)(ASDisplayNode *node));
/**
Given a display node, traverses up the layer tree hierarchy, returning the first display node that passes block.

View File

@@ -39,7 +39,7 @@ extern ASDisplayNode *ASViewToDisplayNode(UIView *view)
return view.asyncdisplaykit_node;
}
extern void ASDisplayNodePerformBlockOnEveryNode(CALayer *layer, ASDisplayNode *node, void(^block)(ASDisplayNode *node))
extern void ASDisplayNodePerformBlockOnEveryNode(CALayer * _Nullable layer, ASDisplayNode * _Nullable node, BOOL traverseSublayers, void(^block)(ASDisplayNode *node))
{
if (!node) {
ASDisplayNodeCAssertNotNil(layer, @"Cannot recursively perform with nil node and nil layer");
@@ -50,19 +50,19 @@ extern void ASDisplayNodePerformBlockOnEveryNode(CALayer *layer, ASDisplayNode *
if (node) {
block(node);
}
if (!layer && [node isNodeLoaded] && ASDisplayNodeThreadIsMain()) {
if (traverseSublayers && !layer && [node isNodeLoaded] && ASDisplayNodeThreadIsMain()) {
layer = node.layer;
}
if (layer && node.shouldRasterizeDescendants == NO) {
if (traverseSublayers && layer && node.shouldRasterizeDescendants == NO) {
/// NOTE: The docs say `sublayers` returns a copy, but it does not.
/// See: http://stackoverflow.com/questions/14854480/collection-calayerarray-0x1ed8faa0-was-mutated-while-being-enumerated
for (CALayer *sublayer in [[layer sublayers] copy]) {
ASDisplayNodePerformBlockOnEveryNode(sublayer, nil, block);
ASDisplayNodePerformBlockOnEveryNode(sublayer, nil, traverseSublayers, block);
}
} else if (node) {
for (ASDisplayNode *subnode in [node subnodes]) {
ASDisplayNodePerformBlockOnEveryNode(nil, subnode, block);
ASDisplayNodePerformBlockOnEveryNode(nil, subnode, traverseSublayers, block);
}
}
}
@@ -86,10 +86,10 @@ extern void ASDisplayNodePerformBlockOnEveryNodeBFS(ASDisplayNode *node, void(^b
}
}
extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, void(^block)(ASDisplayNode *node))
extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL traverseSublayers, void(^block)(ASDisplayNode *node))
{
for (ASDisplayNode *subnode in node.subnodes) {
ASDisplayNodePerformBlockOnEveryNode(nil, subnode, block);
ASDisplayNodePerformBlockOnEveryNode(nil, subnode, YES, block);
}
}

View File

@@ -19,7 +19,7 @@
{
ASDisplayNodeAssertNotNil(node.calculatedLayout, @"Node %@ must be measured before it is rendered.", node);
node.bounds = (CGRect) { .size = node.calculatedSize };
ASDisplayNodePerformBlockOnEveryNode(nil, node, ^(ASDisplayNode * _Nonnull node) {
ASDisplayNodePerformBlockOnEveryNode(nil, node, YES, ^(ASDisplayNode * _Nonnull node) {
[node.layer setNeedsDisplay];
});
[node recursivelyEnsureDisplaySynchronously:YES];