Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions AsyncDisplayKit/ASDisplayNode+Subclasses.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@


/**
* Called just before the view is added to a superview.
* Called just before the view is added to a window.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly I think the behavior of this method is subtle. It actually varies a bit for layer-backed nodes, which rely on kCAOrderIn (CAAction) rather than the UIView-unique hierarchy notifications. This needs better documentation, and in fact, I'm pretty sure a Github issue is filed for this purpose.

*/
- (void)willEnterHierarchy ASDISPLAYNODE_REQUIRES_SUPER;

Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we walk the layer hierarchy instead, or does that not achieve the same result?


// The two methods below will eventually be exposed, but their names are subject to change.
/**
Expand Down
20 changes: 15 additions & 5 deletions AsyncDisplayKit/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,28 @@ 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a bit better :)

/** 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,

/**
* 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,
};



/**
* 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.
Expand Down
28 changes: 27 additions & 1 deletion AsyncDisplayKit/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#import "ASInternalHelpers.h"
#import "ASLayout.h"
#import "ASLayoutSpec.h"
#import "ASCellNode.h"

@interface ASDisplayNode () <UIGestureRecognizerDelegate>

Expand Down Expand Up @@ -1581,13 +1582,21 @@ - (void)willEnterHierarchy
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
{
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
Expand Down Expand Up @@ -1635,6 +1644,20 @@ - (void)recursivelyClearFetchedData
[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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good as a prototype implementation, but we indeed need to improve efficiency with something like ASHierarchyState to avoid the repeated recursion or measurable performance impact will occur. I think we'll land as-is once I am able to start on that change, but it would be important to fix before releasing (or even having in master very long, definitely increases deadlock probability)

return ([self isKindOfClass:ASCellNode.class]
|| [self _supernodeWithClass:ASCellNode.class checkViewHierarchy:NO] != nil);
}

- (ASInterfaceState)interfaceState
{
ASDN::MutexLocker l(_propertyLock);
Expand Down Expand Up @@ -1871,14 +1894,17 @@ - (void)_applyPendingStateToViewOrLayer

// 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) {
if ([supernode isKindOfClass:supernodeClass])
return supernode;
supernode = supernode.supernode;
}
if (!checkViewHierarchy) {
return nil;
}

UIView *view = self.view.superview;
while (view) {
Expand Down
59 changes: 59 additions & 0 deletions AsyncDisplayKitTests/ASDisplayNodeTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,11 +77,15 @@ @interface ASDisplayNode (HackForTests)
+ (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
Expand All @@ -93,6 +98,18 @@ - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
return _calculateSizeBlock ? _calculateSizeBlock(self, constrainedSize) : CGSizeZero;
}

- (void)fetchData
{
[super fetchData];
self.hasFetchedData = YES;
}

- (void)clearFetchedData
{
[super clearFetchedData];
self.hasFetchedData = NO;
}

- (void)dealloc
{
if (_willDeallocBlock) {
Expand Down Expand Up @@ -1663,6 +1680,48 @@ - (void)testBackgroundColorOpaqueRelationshipNoLayer
[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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!!! Thanks for testing!

{
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]];
Expand Down