Skip to content

Commit

Permalink
Improve Ancestry Handling, Avoid Assertion Failure (#246)
Browse files Browse the repository at this point in the history
* Improve our handling of ancestry

* Increase chungalunga
  • Loading branch information
Adlai-Holler committed May 10, 2017
1 parent 19ec044 commit 0143e32
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 39 deletions.
2 changes: 1 addition & 1 deletion AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@
CC57EAF71E3939350034C595 /* ASCollectionView+Undeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = CC2E317F1DAC353700EEE891 /* ASCollectionView+Undeprecated.h */; settings = {ATTRIBUTES = (Private, ); }; };
CC57EAF81E3939450034C595 /* ASTableView+Undeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = CC512B841DAC45C60054848E /* ASTableView+Undeprecated.h */; settings = {ATTRIBUTES = (Private, ); }; };
CC58AA4B1E398E1D002C8CB4 /* ASBlockTypes.h in Headers */ = {isa = PBXBuildFile; fileRef = CC58AA4A1E398E1D002C8CB4 /* ASBlockTypes.h */; settings = {ATTRIBUTES = (Public, ); }; };
CC6AA2DA1E9F03B900978E87 /* ASDisplayNode+Ancestry.h in Headers */ = {isa = PBXBuildFile; fileRef = CC6AA2D81E9F03B900978E87 /* ASDisplayNode+Ancestry.h */; };
CC6AA2DA1E9F03B900978E87 /* ASDisplayNode+Ancestry.h in Headers */ = {isa = PBXBuildFile; fileRef = CC6AA2D81E9F03B900978E87 /* ASDisplayNode+Ancestry.h */; settings = {ATTRIBUTES = (Public, ); }; };
CC6AA2DB1E9F03B900978E87 /* ASDisplayNode+Ancestry.m in Sources */ = {isa = PBXBuildFile; fileRef = CC6AA2D91E9F03B900978E87 /* ASDisplayNode+Ancestry.m */; };
CC7FD9E11BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */; };
CC7FD9E21BB603FF005CCB2B /* ASPhotosFrameworkImageRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- Move more properties from ASTableView, ASCollectionView to their respective node classes. [Adlai Holler](https://github.com/Adlai-Holler)
- Remove finalLayoutElement [Michael Schneider] (https://github.com/maicki)[#96](https://github.com/TextureGroup/Texture/pull/96)
- Add ASPageTable - A map table for fast retrieval of objects within a certain page [Huy Nguyen](https://github.com/nguyenhuy)
- Add new public `-supernodes`, `-supernodesIncludingSelf`, and `-supernodeOfClass:includingSelf:` methods. [Adlai Holler](https://github.com/Adlai-Holler)[#246](https://github.com/TextureGroup/Texture/pull/246)
- Improve our handling supernode traversal to avoid loading layers and fix assertion failures you might hit in debug. [Adlai Holler](https://github.com/Adlai-Holler)[#246](https://github.com/TextureGroup/Texture/pull/246)
- [ASDisplayNode] Pass drawParameter in rendering context callbacks [Michael Schneider](https://github.com/maicki)[#248](https://github.com/TextureGroup/Texture/pull/248)
- [ASTextNode] Move to class method of drawRect:withParameters:isCancelled:isRasterizing: for drawing [Michael Schneider] (https://github.com/maicki)[#232](https://github.com/TextureGroup/Texture/pull/232)
- [ASDisplayNode] Remove instance:-drawRect:withParameters:isCancelled:isRasterizing: (https://github.com/maicki)[#232](https://github.com/TextureGroup/Texture/pull/232)
20 changes: 7 additions & 13 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <AsyncDisplayKit/ASDisplayNodeInternal.h>

#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
#import <AsyncDisplayKit/ASDisplayNode+FrameworkSubclasses.h>
#import <AsyncDisplayKit/ASDisplayNode+Beta.h>
#import <AsyncDisplayKit/ASDisplayNode+Deprecated.h>
Expand Down Expand Up @@ -600,9 +601,9 @@ - (CALayer *)_locked_layerToLoad
return layer;
}

- (void)_locked_loadViewOrLayerIsLayerBacked:(BOOL)isLayerBacked
- (void)_locked_loadViewOrLayer
{
if (isLayerBacked) {
if (_flags.layerBacked) {
TIME_SCOPED(_debugTimeToCreateView);
_layer = [self _locked_layerToLoad];
static int ASLayerDelegateAssociationKey;
Expand Down Expand Up @@ -692,7 +693,7 @@ - (UIView *)view

// Loading a view needs to happen on the main thread
ASDisplayNodeAssertMainThread();
[self _locked_loadViewOrLayerIsLayerBacked:isLayerBacked];
[self _locked_loadViewOrLayer];

// FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
// but automatic subnode management would require us to modify the node tree
Expand Down Expand Up @@ -727,20 +728,13 @@ - (CALayer *)layer
return _layer;
}

BOOL isLayerBacked = _flags.layerBacked;
if (!isLayerBacked) {
// No need for the lock and call the view explicitly in case it needs to be loaded first
ASDN::MutexUnlocker u(__instanceLock__);
return self.view.layer;
}

if (![self _locked_shouldLoadViewOrLayer]) {
return nil;
}

// Loading a layer needs to happen on the main thread
ASDisplayNodeAssertMainThread();
[self _locked_loadViewOrLayerIsLayerBacked:isLayerBacked];
[self _locked_loadViewOrLayer];

// FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
// but automatic subnode management would require us to modify the node tree
Expand Down Expand Up @@ -3909,8 +3903,8 @@ - (ASEventLog *)eventLog
[result addObject:@{ @"frame" : [NSValue valueWithCGRect:_pendingViewState.frame] }];
}

// Check supernode so that if we are cell node we don't find self.
ASCellNode *cellNode = ASDisplayNodeFindFirstSupernodeOfClass(self.supernode, [ASCellNode class]);
// Check supernode so that if we are a cell node we don't find self.
ASCellNode *cellNode = [self supernodeOfClass:[ASCellNode class] includingSelf:NO];
if (cellNode != nil) {
[result addObject:@{ @"cellNode" : ASObjectDescriptionMakeTiny(cellNode) }];
}
Expand Down
4 changes: 2 additions & 2 deletions Source/ASDisplayNodeExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL tr
/**
Given a display node, traverses up the layer tree hierarchy, returning the first display node that passes block.
*/
extern ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernode(ASDisplayNode * _Nullable node, BOOL (^block)(ASDisplayNode *node)) AS_WARN_UNUSED_RESULT;
extern ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernode(ASDisplayNode * _Nullable node, BOOL (^block)(ASDisplayNode *node)) AS_WARN_UNUSED_RESULT ASDISPLAYNODE_DEPRECATED_MSG("Use the `supernodes` property instead.");

/**
Given a display node, traverses up the layer tree hierarchy, returning the first display node of kind class.
*/
extern __kindof ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c) AS_WARN_UNUSED_RESULT;
extern __kindof ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c) AS_WARN_UNUSED_RESULT ASDISPLAYNODE_DEPRECATED_MSG("Use the `supernodeOfClass:includingSelf:` method instead.");

/**
* Given a layer, find the window it lives in, if any.
Expand Down
20 changes: 9 additions & 11 deletions Source/ASDisplayNodeExtras.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import <AsyncDisplayKit/ASDisplayNodeExtras.h>
#import <AsyncDisplayKit/ASDisplayNodeInternal.h>
#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>

#import <queue>
#import <AsyncDisplayKit/ASRunLoopQueue.h>
Expand Down Expand Up @@ -138,24 +139,21 @@ extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL tr

ASDisplayNode *ASDisplayNodeFindFirstSupernode(ASDisplayNode *node, BOOL (^block)(ASDisplayNode *node))
{
CALayer *layer = node.layer;

while (layer) {
node = ASLayerToDisplayNode(layer);
if (block(node)) {
return node;
// This function has historically started with `self` but the name suggests
// that it wouldn't. Perhaps we should change the behavior.
for (ASDisplayNode *ancestor in node.supernodesIncludingSelf) {
if (block(ancestor)) {
return ancestor;
}
layer = layer.superlayer;
}

return nil;
}

__kindof ASDisplayNode *ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c)
{
return ASDisplayNodeFindFirstSupernode(start, ^(ASDisplayNode *n) {
return [n isKindOfClass:c];
});
// This function has historically started with `self` but the name suggests
// that it wouldn't. Perhaps we should change the behavior.
return [start supernodeOfClass:c includingSelf:YES];
}

static void _ASCollectDisplayNodes(NSMutableArray *array, CALayer *layer)
Expand Down
1 change: 1 addition & 0 deletions Source/AsyncDisplayKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <AsyncDisplayKit/ASAvailability.h>
#import <AsyncDisplayKit/ASDisplayNode.h>
#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
#import <AsyncDisplayKit/ASDisplayNode+Convenience.h>
#import <AsyncDisplayKit/ASDisplayNodeExtras.h>

Expand Down
31 changes: 30 additions & 1 deletion Source/Base/ASDisplayNode+Ancestry.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,36 @@ NS_ASSUME_NONNULL_BEGIN

@interface ASDisplayNode (Ancestry)

- (NSEnumerator *)ancestorEnumeratorWithSelf:(BOOL)includeSelf;
/**
* Returns an object to enumerate the supernode ancestry of this node, starting with its supernode.
*
* For instance, you could write:
* for (ASDisplayNode *node in self.supernodes) {
* if ([node.backgroundColor isEqual:[UIColor blueColor]]) {
* node.hidden = YES;
* }
* }
*
* Note: If this property is read on the main thread, the enumeration will attempt to go up
* the layer hierarchy if it finds a break in the display node hierarchy.
*/
@property (atomic, readonly) id<NSFastEnumeration> supernodes;

/**
* Same as `supernodes` but begins the enumeration with self.
*/
@property (atomic, readonly) id<NSFastEnumeration> supernodesIncludingSelf;

/**
* Searches the supernodes of this node for one matching the given class.
*
* @param supernodeClass The class of node you're looking for.
* @param includeSelf Whether to include self in the search.
* @return A node of the given class that is an ancestor of this node, or nil.
*
* @note See the documentation on `supernodes` for details about the upward traversal.
*/
- (nullable __kindof ASDisplayNode *)supernodeOfClass:(Class)supernodeClass includingSelf:(BOOL)includeSelf;

/**
* e.g. "(<MYTextNode: 0xFFFF>, <MYTextContainingNode: 0xFFFF>, <MYCellNode: 0xFFFF>)"
Expand Down
56 changes: 45 additions & 11 deletions Source/Base/ASDisplayNode+Ancestry.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,80 @@
//

#import "ASDisplayNode+Ancestry.h"
#import <AsyncDisplayKit/ASThread.h>
#import <AsyncDisplayKit/ASDisplayNodeExtras.h>

AS_SUBCLASSING_RESTRICTED
@interface ASNodeAncestryEnumerator : NSEnumerator
@end

@implementation ASNodeAncestryEnumerator {
/// Would be nice to use __unsafe_unretained but nodes can be
/// deallocated on arbitrary threads so nope.
__weak ASDisplayNode * _nextNode;
ASDisplayNode *_lastNode; // This needs to be strong because enumeration will not retain the current batch of objects
BOOL _initialState;
}

- (instancetype)initWithNode:(ASDisplayNode *)node
{
if (self = [super init]) {
_nextNode = node;
_initialState = YES;
_lastNode = node;
}
return self;
}

- (id)nextObject
{
ASDisplayNode *node = _nextNode;
_nextNode = [node supernode];
return node;
if (_initialState) {
_initialState = NO;
return _lastNode;
}

ASDisplayNode *nextNode = _lastNode.supernode;
if (nextNode == nil && ASDisplayNodeThreadIsMain()) {
CALayer *layer = _lastNode.nodeLoaded ? _lastNode.layer.superlayer : nil;
while (layer != nil) {
nextNode = ASLayerToDisplayNode(layer);
if (nextNode != nil) {
break;
}
layer = layer.superlayer;
}
}
_lastNode = nextNode;
return nextNode;
}

@end

@implementation ASDisplayNode (Ancestry)

- (NSEnumerator *)ancestorEnumeratorWithSelf:(BOOL)includeSelf
- (id<NSFastEnumeration>)supernodes
{
NSEnumerator *result = [[ASNodeAncestryEnumerator alloc] initWithNode:self];
[result nextObject]; // discard first object (self)
return result;
}

- (id<NSFastEnumeration>)supernodesIncludingSelf
{
ASDisplayNode *node = includeSelf ? self : self.supernode;
return [[ASNodeAncestryEnumerator alloc] initWithNode:node];
return [[ASNodeAncestryEnumerator alloc] initWithNode:self];
}

- (nullable __kindof ASDisplayNode *)supernodeOfClass:(Class)supernodeClass includingSelf:(BOOL)includeSelf
{
id<NSFastEnumeration> chain = includeSelf ? self.supernodesIncludingSelf : self.supernodes;
for (ASDisplayNode *ancestor in chain) {
if ([ancestor isKindOfClass:supernodeClass]) {
return ancestor;
}
}
return nil;
}

- (NSString *)ancestryDescription
{
NSMutableArray *strings = [NSMutableArray array];
for (ASDisplayNode *node in [self ancestorEnumeratorWithSelf:YES]) {
for (ASDisplayNode *node in self.supernodes) {
[strings addObject:ASObjectDescriptionMakeTiny(node)];
}
return strings.description;
Expand Down

0 comments on commit 0143e32

Please sign in to comment.